- Revision
- 277371
- Author
- [email protected]
- Date
- 2021-05-12 08:59:48 -0700 (Wed, 12 May 2021)
Log Message
[css-flexbox] Do not use margins when computing aspect ratio cross sizes
https://bugs.webkit.org/show_bug.cgi?id=221210
LayoutTests/imported/w3c:
<rdar://problem/74097534>
Reviewed by Javier Fernandez.
* web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013-expected.txt:
Replaced FAIL by PASS expectations + new expectations.
* web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html: Imported
latest changes from upstream WPT.
Source/WebCore:
Reviewed by Javier Fernandez.
In r270578 we implemented section 9.8.1 of the flexbox specs which allowed us to compute child's
indefinite cross sizes as definite when the flexbox container had a definite cross size and a couple
of other conditions. However we did not take into account that the child might have some margins in
in the cross direction. Aspect ratio computations must use the content box and thus, we need to
substract the margin extent before trying to compute a cross size based on an aspect ratio.
Note that when computeMainSizeFromAspectRatioUsing() is called the child might not have been ever
laid out. This means that the margins wouldn't have been computed and thus marginXXX() would always
return 0. That's why crossAxisMarginExtentForChild() was also modified so that it actually computes
the margins in case the child needs to be laid out.
* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::crossAxisMarginExtentForChild const):
(WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const):
LayoutTests:
Reviewed by Javier Fernandez.
* TestExpectations: Unskipped flex-aspect-ratio-img-row-013.html which is now passing.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (277370 => 277371)
--- trunk/LayoutTests/ChangeLog 2021-05-12 15:48:26 UTC (rev 277370)
+++ trunk/LayoutTests/ChangeLog 2021-05-12 15:59:48 UTC (rev 277371)
@@ -1,3 +1,12 @@
+2021-05-12 Sergio Villar Senin <[email protected]>
+
+ [css-flexbox] Do not use margins when computing aspect ratio cross sizes
+ https://bugs.webkit.org/show_bug.cgi?id=221210
+
+ Reviewed by Javier Fernandez.
+
+ * TestExpectations: Unskipped flex-aspect-ratio-img-row-013.html which is now passing.
+
2021-05-12 Diego Pino Garcia <[email protected]>
[GLIB] Unreviewed test gardening. media/track/in-band/track-in-band-srt-mkv-kind.html is a flaky crash.
Modified: trunk/LayoutTests/TestExpectations (277370 => 277371)
--- trunk/LayoutTests/TestExpectations 2021-05-12 15:48:26 UTC (rev 277370)
+++ trunk/LayoutTests/TestExpectations 2021-05-12 15:59:48 UTC (rev 277371)
@@ -3869,7 +3869,6 @@
webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-015.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-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/aspect-ratio-intrinsic-size-001.html [ ImageOnlyFailure ]
webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-002.html [ ImageOnlyFailure ]
webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-003.html [ ImageOnlyFailure ]
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (277370 => 277371)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2021-05-12 15:48:26 UTC (rev 277370)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2021-05-12 15:59:48 UTC (rev 277371)
@@ -1,3 +1,16 @@
+2021-05-12 Sergio Villar Senin <[email protected]>
+
+ [css-flexbox] Do not use margins when computing aspect ratio cross sizes
+ https://bugs.webkit.org/show_bug.cgi?id=221210
+ <rdar://problem/74097534>
+
+ Reviewed by Javier Fernandez.
+
+ * web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013-expected.txt:
+ Replaced FAIL by PASS expectations + new expectations.
+ * web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html: Imported
+ latest changes from upstream WPT.
+
2021-05-11 Cathie Chen <[email protected]>
[CSS contain] Support contain:size
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013-expected.txt (277370 => 277371)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013-expected.txt 2021-05-12 15:48:26 UTC (rev 277370)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013-expected.txt 2021-05-12 15:59:48 UTC (rev 277371)
@@ -1,4 +1,4 @@
-Pass condition is 3 green 100x100 squares.
+Pass condition is 4 green 100x100 squares and 1 0x0 square.
Firefox 84a1 passes. Chrome 87 fails them all by making the green rectangles be 200x100.
@@ -8,13 +8,19 @@
Have to subtract the margin from the stretched height to get the transferred size suggestion:
+Have to subtract a margin larger than the stretched height to get 0px transferred size suggestion:
+
+
+Have to subtract the margin from the stretched height (ignoring the presence of a border) to get the transferred size suggestion:
+
+
Stretched transferred size suggestion has to obey min-height:
PASS img 1
-FAIL img 2 assert_equals:
-<img src="" style="margin-bottom: 20px" data-expected-height="100" data-expected-width="100">
-width expected 100 but got 120
+PASS img 2
PASS img 3
+PASS img 4
+PASS img 5
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html (277370 => 277371)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html 2021-05-12 15:48:26 UTC (rev 277370)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html 2021-05-12 15:59:48 UTC (rev 277371)
@@ -10,7 +10,7 @@
<body _onload_="checkLayout('img')">
<p>
-Pass condition is 3 green 100x100 squares.
+Pass condition is 4 green 100x100 squares and 1 0x0 square.
</p>
<p>
@@ -27,6 +27,16 @@
<img src="" style="margin-bottom: 20px" data-expected-height=100 data-expected-width=100>
</div>
+<p>Have to subtract a margin larger than the stretched height to get 0px transferred size suggestion:</p>
+<div style="display: flex; width: 0; height: 120px;">
+ <img src="" style="margin-bottom: 160px" data-expected-height=0 data-expected-width=0>
+</div>
+
+<p>Have to subtract the margin from the stretched height (ignoring the presence of a border) to get the transferred size suggestion:</p>
+<div style="display: flex; width: 0; height: 120px;">
+ <img src="" style="border-right: 10px solid black; margin-bottom: 20px" data-expected-height=100 data-expected-width=110>
+</div>
+
<p>Stretched transferred size suggestion has to obey min-height:</p>
<div style="display: flex; width: 0; height: 50px;">
<img src="" style="min-height: 100px;" data-expected-height=100 data-expected-width=100>
Modified: trunk/Source/WebCore/ChangeLog (277370 => 277371)
--- trunk/Source/WebCore/ChangeLog 2021-05-12 15:48:26 UTC (rev 277370)
+++ trunk/Source/WebCore/ChangeLog 2021-05-12 15:59:48 UTC (rev 277371)
@@ -1,3 +1,25 @@
+2021-05-12 Sergio Villar Senin <[email protected]>
+
+ [css-flexbox] Do not use margins when computing aspect ratio cross sizes
+ https://bugs.webkit.org/show_bug.cgi?id=221210
+
+ Reviewed by Javier Fernandez.
+
+ In r270578 we implemented section 9.8.1 of the flexbox specs which allowed us to compute child's
+ indefinite cross sizes as definite when the flexbox container had a definite cross size and a couple
+ of other conditions. However we did not take into account that the child might have some margins in
+ in the cross direction. Aspect ratio computations must use the content box and thus, we need to
+ substract the margin extent before trying to compute a cross size based on an aspect ratio.
+
+ Note that when computeMainSizeFromAspectRatioUsing() is called the child might not have been ever
+ laid out. This means that the margins wouldn't have been computed and thus marginXXX() would always
+ return 0. That's why crossAxisMarginExtentForChild() was also modified so that it actually computes
+ the margins in case the child needs to be laid out.
+
+ * rendering/RenderFlexibleBox.cpp:
+ (WebCore::RenderFlexibleBox::crossAxisMarginExtentForChild const):
+ (WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const):
+
2021-05-12 Sam Weinig <[email protected]>
Factor copyImagePixels pixel conversion code into its own file
Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (277370 => 277371)
--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2021-05-12 15:48:26 UTC (rev 277370)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2021-05-12 15:59:48 UTC (rev 277371)
@@ -741,7 +741,16 @@
LayoutUnit RenderFlexibleBox::crossAxisMarginExtentForChild(const RenderBox& child) const
{
- return isHorizontalFlow() ? child.verticalMarginExtent() : child.horizontalMarginExtent();
+ if (!child.needsLayout())
+ return isHorizontalFlow() ? child.verticalMarginExtent() : child.horizontalMarginExtent();
+
+ LayoutUnit marginStart;
+ LayoutUnit marginEnd;
+ if (isHorizontalFlow())
+ child.computeBlockDirectionMargins(*this, marginStart, marginEnd);
+ else
+ child.computeInlineDirectionMargins(*this, child.containingBlockLogicalWidthForContentInFragment(nullptr), child.logicalWidth(), marginStart, marginEnd);
+ return marginStart + marginEnd;
}
LayoutUnit RenderFlexibleBox::crossAxisScrollbarExtent() const
@@ -818,7 +827,7 @@
auto containerCrossSizeLength = isHorizontalFlow() ? style().height() : style().width();
// Keep this sync'ed with childCrossSizeShouldUseContainerCrossSize().
ASSERT(containerCrossSizeLength.isFixed());
- crossSize = valueForLength(containerCrossSizeLength, -1_lu);
+ crossSize = std::max(0_lu, valueForLength(containerCrossSizeLength, -1_lu) - crossAxisMarginExtentForChild(child));
} else {
ASSERT(crossSizeLength.isPercentOrCalculated());
crossSize = mainAxisIsChildInlineAxis(child) ? child.computePercentageLogicalHeight(crossSizeLength) : adjustBorderBoxLogicalWidthForBoxSizing(valueForLength(crossSizeLength, contentWidth()), crossSizeLength.type());
