<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[277371] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/277371">277371</a></dd>
<dt>Author</dt> <dd>svillar@igalia.com</dd>
<dt>Date</dt> <dd>2021-05-12 08:59:48 -0700 (Wed, 12 May 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>[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 <a href="http://trac.webkit.org/projects/webkit/changeset/270578">r270578</a> 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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsTestExpectations">trunk/LayoutTests/TestExpectations</a></li>
<li><a href="#trunkLayoutTestsimportedw3cChangeLog">trunk/LayoutTests/imported/w3c/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsimportedw3cwebplatformtestscsscssflexboxflexaspectratioimgrow013expectedtxt">trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013-expected.txt</a></li>
<li><a href="#trunkLayoutTestsimportedw3cwebplatformtestscsscssflexboxflexaspectratioimgrow013html">trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderFlexibleBoxcpp">trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (277370 => 277371)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog      2021-05-12 15:48:26 UTC (rev 277370)
+++ trunk/LayoutTests/ChangeLog 2021-05-12 15:59:48 UTC (rev 277371)
</span><span class="lines">@@ -1,3 +1,12 @@
</span><ins>+2021-05-12  Sergio Villar Senin  <svillar@igalia.com>
+
+        [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.
+
</ins><span class="cx"> 2021-05-12  Diego Pino Garcia  <dpino@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         [GLIB] Unreviewed test gardening. media/track/in-band/track-in-band-srt-mkv-kind.html is a flaky crash.
</span></span></pre></div>
<a id="trunkLayoutTestsTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/TestExpectations (277370 => 277371)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/TestExpectations       2021-05-12 15:48:26 UTC (rev 277370)
+++ trunk/LayoutTests/TestExpectations  2021-05-12 15:59:48 UTC (rev 277371)
</span><span class="lines">@@ -3869,7 +3869,6 @@
</span><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-015.html [ ImageOnlyFailure ]
</span><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-007.html [ ImageOnlyFailure ]
</span><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-010.html [ ImageOnlyFailure ]
</span><del>-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html [ ImageOnlyFailure ]
</del><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-001.html [ ImageOnlyFailure ]
</span><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-002.html [ ImageOnlyFailure ]
</span><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-003.html [ ImageOnlyFailure ]
</span></span></pre></div>
<a id="trunkLayoutTestsimportedw3cChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/imported/w3c/ChangeLog (277370 => 277371)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2021-05-12  Sergio Villar Senin  <svillar@igalia.com>
+
+        [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.
+
</ins><span class="cx"> 2021-05-11  Cathie Chen  <cathiechen@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         [CSS contain] Support contain:size
</span></span></pre></div>
<a id="trunkLayoutTestsimportedw3cwebplatformtestscsscssflexboxflexaspectratioimgrow013expectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013-expected.txt (277370 => 277371)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,4 +1,4 @@
</span><del>-Pass condition is 3 green 100x100 squares.
</del><ins>+Pass condition is 4 green 100x100 squares and 1 0x0 square.
</ins><span class="cx"> 
</span><span class="cx"> Firefox 84a1 passes. Chrome 87 fails them all by making the green rectangles be 200x100.
</span><span class="cx"> 
</span><span class="lines">@@ -8,13 +8,19 @@
</span><span class="cx"> Have to subtract the margin from the stretched height to get the transferred size suggestion:
</span><span class="cx"> 
</span><span class="cx"> 
</span><ins>+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:
+
+
</ins><span class="cx"> Stretched transferred size suggestion has to obey min-height:
</span><span class="cx"> 
</span><span class="cx"> 
</span><span class="cx"> 
</span><span class="cx"> PASS img 1
</span><del>-FAIL img 2 assert_equals:
-<img src="support/200x200-green.png" style="margin-bottom: 20px" data-expected-height="100" data-expected-width="100">
-width expected 100 but got 120
</del><ins>+PASS img 2
</ins><span class="cx"> PASS img 3
</span><ins>+PASS img 4
+PASS img 5
</ins><span class="cx"> 
</span></span></pre></div>
<a id="trunkLayoutTestsimportedw3cwebplatformtestscsscssflexboxflexaspectratioimgrow013html"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html (277370 => 277371)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -10,7 +10,7 @@
</span><span class="cx"> 
</span><span class="cx"> <body onload="checkLayout('img')">
</span><span class="cx"> <p>
</span><del>-Pass condition is 3 green 100x100 squares.
</del><ins>+Pass condition is 4 green 100x100 squares and 1 0x0 square.
</ins><span class="cx"> </p>
</span><span class="cx"> 
</span><span class="cx"> <p>
</span><span class="lines">@@ -27,6 +27,16 @@
</span><span class="cx">   <img src="support/200x200-green.png" style="margin-bottom: 20px" data-expected-height=100 data-expected-width=100>
</span><span class="cx"> </div>
</span><span class="cx"> 
</span><ins>+<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="support/200x200-green.png" 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="support/200x200-green.png" style="border-right: 10px solid black; margin-bottom: 20px" data-expected-height=100 data-expected-width=110>
+</div>
+
</ins><span class="cx"> <p>Stretched transferred size suggestion has to obey min-height:</p>
</span><span class="cx"> <div style="display: flex; width: 0; height: 50px;">
</span><span class="cx">   <img src="support/200x200-green.png" style="min-height: 100px;" data-expected-height=100 data-expected-width=100>
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (277370 => 277371)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2021-05-12  Sergio Villar Senin  <svillar@igalia.com>
+
+        [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):
+
</ins><span class="cx"> 2021-05-12  Sam Weinig  <weinig@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Factor copyImagePixels pixel conversion code into its own file
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderFlexibleBoxcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (277370 => 277371)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -741,7 +741,16 @@
</span><span class="cx"> 
</span><span class="cx"> LayoutUnit RenderFlexibleBox::crossAxisMarginExtentForChild(const RenderBox& child) const
</span><span class="cx"> {
</span><del>-    return isHorizontalFlow() ? child.verticalMarginExtent() : child.horizontalMarginExtent();
</del><ins>+    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;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> LayoutUnit RenderFlexibleBox::crossAxisScrollbarExtent() const
</span><span class="lines">@@ -818,7 +827,7 @@
</span><span class="cx">         auto containerCrossSizeLength = isHorizontalFlow() ? style().height() : style().width();
</span><span class="cx">         // Keep this sync'ed with childCrossSizeShouldUseContainerCrossSize().
</span><span class="cx">         ASSERT(containerCrossSizeLength.isFixed());
</span><del>-        crossSize = valueForLength(containerCrossSizeLength, -1_lu);
</del><ins>+        crossSize = std::max(0_lu, valueForLength(containerCrossSizeLength, -1_lu) - crossAxisMarginExtentForChild(child));
</ins><span class="cx">     } else {
</span><span class="cx">         ASSERT(crossSizeLength.isPercentOrCalculated());
</span><span class="cx">         crossSize = mainAxisIsChildInlineAxis(child) ? child.computePercentageLogicalHeight(crossSizeLength) : adjustBorderBoxLogicalWidthForBoxSizing(valueForLength(crossSizeLength, contentWidth()), crossSizeLength.type());
</span></span></pre>
</div>
</div>

</body>
</html>