[css-flexbox] WebKit doesn't preserve aspect ratio when computing cro…

…ss size of flexed images in auto-height flex container https://bugs.webkit.org/show_bug.cgi?id=209983 <rdar://problem/61288094> Reviewed by Darin Adler. LayoutTests/imported/w3c: * web-platform-tests/css/css-flexbox/flexitem-stretch-image-expected.txt: Replace FAIL by PASS expectations. Source/WebCore: Aspect ratio was not preserved in the cross axis because WebKit was stretching the items (as they're auto sized) without considering the aspect ratio. Instead of letting flexbox code deal with that we basically don't stretch them and let RenderReplaced compute the cross size based on the aspect ratio as it does with other elements that are not flex items. This allows us to pass 3 tests from the WPT test suite. * rendering/RenderFlexibleBox.cpp: (WebCore::RenderFlexibleBox::needToStretchChildLogicalHeight const): Return false for replaced elements with aspect ratio. LayoutTests: * TestExpectations: Unskipped 2 tests that are working fine now * css3/flexbox/flexitem.html: Updated expectations. Aspect ratio must be preserved. Canonical link: https://commits.webkit.org/231990@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@270288 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Dec 1, 2020 · 1c7aa20 · 1c7aa20
1 parent a2d86b2
commit 1c7aa20
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 10 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2020-12-01  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-flexbox] WebKit doesn't preserve aspect ratio when computing cross size of flexed images in auto-height flex container
+        https://bugs.webkit.org/show_bug.cgi?id=209983
+        <rdar://problem/61288094>
+
+        Reviewed by Darin Adler.
+
+        * TestExpectations: Unskipped 2 tests that are working fine now
+        * css3/flexbox/flexitem.html: Updated expectations. Aspect ratio must be preserved.
+
 2020-12-01  Diego Pino Garcia  <dpino@igalia.com>
 
         [WPE] Unreviewed test gardening. Update WPE baselines of baselines updated r238731.

diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -3912,11 +3912,9 @@ webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-017.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-006.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-007.html [ ImageOnlyFailure ]
-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-008.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-009.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-010.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html [ ImageOnlyFailure ]
-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-014.html [ ImageOnlyFailure ]
 
 webkit.org/b/145176 imported/w3c/web-platform-tests/css/css-flexbox/flexbox_align-items-stretch-3.html [ ImageOnlyFailure ]
 webkit.org/b/210093 imported/w3c/web-platform-tests/css/css-flexbox/select-element-zero-height-001.html [ ImageOnlyFailure ]

diff --git a/LayoutTests/css3/flexbox/flexitem.html b/LayoutTests/css3/flexbox/flexitem.html
@@ -47,8 +47,8 @@
 
 <div class="flexbox">
   <img data-expected-display="block" data-expected-width="200" style="flex: 1 0 auto;" src="../images/resources/blue-100.png">
-  <img data-expected-display="block" data-expected-width="200" data-expected-height="100" style="flex: 2 0 0;" src="doesnotexist.png">
-  <img data-expected-display="block" data-expected-width="200" data-expected-height="100" style="flex: 2 0 0;" src="doesnotexist.png" alt="placeholder text">
+  <img data-expected-display="block" data-expected-width="200" data-expected-height="200" style="flex: 2 0 0;" src="doesnotexist.png">
+  <img data-expected-display="block" data-expected-width="200" data-expected-height="200" style="flex: 2 0 0;" src="doesnotexist.png" alt="placeholder text">
 </div>
 
 <div class="flexbox">

diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
@@ -1,3 +1,13 @@
+2020-12-01  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-flexbox] WebKit doesn't preserve aspect ratio when computing cross size of flexed images in auto-height flex container
+        https://bugs.webkit.org/show_bug.cgi?id=209983
+        <rdar://problem/61288094>
+
+        Reviewed by Darin Adler.
+
+        * web-platform-tests/css/css-flexbox/flexitem-stretch-image-expected.txt: Replace FAIL by PASS expectations.
+
 2020-11-30  Alex Christensen  <achristensen@webkit.org>
 
         Allow blob URLs with fragments

diff --git a/...Tests/imported/w3c/web-platform-tests/css/css-flexbox/flexitem-stretch-image-expected.txt b/...Tests/imported/w3c/web-platform-tests/css/css-flexbox/flexitem-stretch-image-expected.txt
@@ -1,8 +1,3 @@
 
-FAIL .flexbox 1 assert_equals:
-<div class="flexbox">
-  <img data-expected-display="block" data-expected-width="345" data-expected-height="345" style="flex: 1 0 auto;" src="support/100x100-blue.png">
-  <img data-expected-display="block" data-expected-width="255" data-expected-height="345" style="flex: 1 0 auto;" src="support/10x10-green.png">
-</div>
-height expected 345 but got 100
+PASS .flexbox 1
 
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,20 @@
+2020-12-01  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-flexbox] WebKit doesn't preserve aspect ratio when computing cross size of flexed images in auto-height flex container
+        https://bugs.webkit.org/show_bug.cgi?id=209983
+        <rdar://problem/61288094>
+
+        Reviewed by Darin Adler.
+
+        Aspect ratio was not preserved in the cross axis because WebKit was stretching the items (as they're auto sized) without considering
+        the aspect ratio. Instead of letting flexbox code deal with that we basically don't stretch them and let RenderReplaced compute the
+        cross size based on the aspect ratio as it does with other elements that are not flex items.
+
+        This allows us to pass 3 tests from the WPT test suite.
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::needToStretchChildLogicalHeight const): Return false for replaced elements with aspect ratio.
+
 2020-11-30  Andres Gonzalez  <andresg_22@apple.com>
 
         [WebAccessibilityObjectWrapper doAXAttributedStringForRange] needs to run on the main thread.

diff --git a/Source/WebCore/rendering/RenderFlexibleBox.cpp b/Source/WebCore/rendering/RenderFlexibleBox.cpp
@@ -1550,6 +1550,10 @@ bool RenderFlexibleBox::needToStretchChildLogicalHeight(const RenderBox& child)
     if (isHorizontalFlow() != child.style().isHorizontalWritingMode())
         return false;
 
+    // Aspect ratio is properly handled by RenderReplaced during layout.
+    if (child.isRenderReplaced() && child.hasAspectRatio())
+        return false;
+
     return child.style().logicalHeight().isAuto();
 }