<!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>[279271] 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/279271">279271</a></dd>
<dt>Author</dt> <dd>svillar@igalia.com</dd>
<dt>Date</dt> <dd>2021-06-25 01:59:13 -0700 (Fri, 25 Jun 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>[css-flexbox] Do not clamp flex base size with {min|max}-{height|width}
https://bugs.webkit.org/show_bug.cgi?id=225590

Reviewed by Alan Bujtas.

Source/WebCore:

When computing flex base size we should not clamp it with neither min-{height|width}
nor max-{height|width}. That would be done later and will be called the hypothetical
main size.

For most of the cases we were already doing it, however there are some particular cases
in which we have to call the generic layout code which does clamp the sizes using min
and max sizes. In order to make those code paths work, we reset the min|max values to
their initial ones (so they effectively become inactive) and then we set the original
values back after computing the base size.

* rendering/RenderFlexibleBox.cpp:
(WebCore::ScopedUnboundedBox::ScopedUnboundedBox): New scoped class that sets min|max sizes
to their initial values on instantiation and restores the original values on destruction.
(WebCore::ScopedUnboundedBox::~ScopedUnboundedBox):
(WebCore::RenderFlexibleBox::constructFlexItems): Wrap the base size computation with a
ScopedUnboundedBox.

LayoutTests:

The patch allows us to pass 3 new tests. We're adding percentage-max-height-003.html to
the list of expected failures because these changes make it fail. This is not really a
regression however because although the size of the deepest flex item was correct (and thus
allowed us to pass the test) the other sizes were completely wrong. So it's more an issue
of the test not being complete enough and passing artificially than anything else.

* TestExpectations: Unskipped 3 tests & skipped 1.</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="#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 (279270 => 279271)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog      2021-06-25 08:43:33 UTC (rev 279270)
+++ trunk/LayoutTests/ChangeLog 2021-06-25 08:59:13 UTC (rev 279271)
</span><span class="lines">@@ -1,3 +1,18 @@
</span><ins>+2021-06-14  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-flexbox] Do not clamp flex base size with {min|max}-{height|width}
+        https://bugs.webkit.org/show_bug.cgi?id=225590
+
+        Reviewed by Alan Bujtas.
+
+        The patch allows us to pass 3 new tests. We're adding percentage-max-height-003.html to
+        the list of expected failures because these changes make it fail. This is not really a
+        regression however because although the size of the deepest flex item was correct (and thus
+        allowed us to pass the test) the other sizes were completely wrong. So it's more an issue
+        of the test not being complete enough and passing artificially than anything else.
+
+        * TestExpectations: Unskipped 3 tests & skipped 1.
+
</ins><span class="cx"> 2021-06-25  Arcady Goldmints-Orlov  <agoldmints@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         [GTK] Unreviewed test gardening, mark more WebXR tests as unsupported
</span></span></pre></div>
<a id="trunkLayoutTestsTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/TestExpectations (279270 => 279271)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/TestExpectations       2021-06-25 08:43:33 UTC (rev 279270)
+++ trunk/LayoutTests/TestExpectations  2021-06-25 08:59:13 UTC (rev 279271)
</span><span class="lines">@@ -1217,6 +1217,7 @@
</span><span class="cx"> webkit.org/b/136754 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-015.html [ ImageOnlyFailure ]
</span><span class="cx"> webkit.org/b/136754 imported/w3c/web-platform-tests/css/css-flexbox/flexbox-safe-overflow-position-001.html [ ImageOnlyFailure ]
</span><span class="cx"> webkit.org/b/136754 imported/w3c/web-platform-tests/css/css-flexbox/percentage-max-height-002.html [ ImageOnlyFailure ]
</span><ins>+webkit.org/b/136754 imported/w3c/web-platform-tests/css/css-flexbox/percentage-max-height-003.html [ ImageOnlyFailure ]
</ins><span class="cx"> webkit.org/b/210243 imported/w3c/web-platform-tests/css/css-flexbox/percentage-size-quirks-002.html [ Failure ]
</span><span class="cx"> webkit.org/b/136754 imported/w3c/web-platform-tests/css/css-flexbox/scrollbars-no-margin.html [ ImageOnlyFailure ]
</span><span class="cx"> 
</span><span class="lines">@@ -3949,9 +3950,7 @@
</span><span class="cx"> webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/vertical-alignment-srl-034.xht [ ImageOnlyFailure ]
</span><span class="cx"> webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/vertical-alignment-srl-040.xht [ ImageOnlyFailure ]
</span><span class="cx"> 
</span><del>-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-012.html [ ImageOnlyFailure ]
</del><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-015.html [ ImageOnlyFailure ]
</span><del>-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-007.html [ ImageOnlyFailure ]
</del><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-010.html [ ImageOnlyFailure ]
</span><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="lines">@@ -4023,7 +4022,6 @@
</span><span class="cx"> webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-inflexible-in-column-1.html [ ImageOnlyFailure ]
</span><span class="cx"> webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-inflexible-in-column-2.html [ ImageOnlyFailure ]
</span><span class="cx"> webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-inflexible-in-row-2.html [ ImageOnlyFailure ]
</span><del>-webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-min-height-1.html [ ImageOnlyFailure ]
</del><span class="cx"> 
</span><span class="cx"> # SVGs as flex items.
</span><span class="cx"> webkit.org/b/221474 imported/w3c/web-platform-tests/css/css-flexbox/svg-root-as-flex-item-002.html [ ImageOnlyFailure ]
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (279270 => 279271)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog   2021-06-25 08:43:33 UTC (rev 279270)
+++ trunk/Source/WebCore/ChangeLog      2021-06-25 08:59:13 UTC (rev 279271)
</span><span class="lines">@@ -1,3 +1,27 @@
</span><ins>+2021-06-14  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-flexbox] Do not clamp flex base size with {min|max}-{height|width}
+        https://bugs.webkit.org/show_bug.cgi?id=225590
+
+        Reviewed by Alan Bujtas.
+
+        When computing flex base size we should not clamp it with neither min-{height|width}
+        nor max-{height|width}. That would be done later and will be called the hypothetical
+        main size.
+
+        For most of the cases we were already doing it, however there are some particular cases
+        in which we have to call the generic layout code which does clamp the sizes using min
+        and max sizes. In order to make those code paths work, we reset the min|max values to
+        their initial ones (so they effectively become inactive) and then we set the original
+        values back after computing the base size.
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::ScopedUnboundedBox::ScopedUnboundedBox): New scoped class that sets min|max sizes
+        to their initial values on instantiation and restores the original values on destruction.
+        (WebCore::ScopedUnboundedBox::~ScopedUnboundedBox):
+        (WebCore::RenderFlexibleBox::constructFlexItems): Wrap the base size computation with a
+        ScopedUnboundedBox.
+
</ins><span class="cx"> 2021-06-09  Sergio Villar Senin  <svillar@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         [css-flexbox] Move flex item preferred width computation specifics to RenderFlexibleBox class
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderFlexibleBoxcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (279270 => 279271)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp     2021-06-25 08:43:33 UTC (rev 279270)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp        2021-06-25 08:59:13 UTC (rev 279271)
</span><span class="lines">@@ -1370,34 +1370,80 @@
</span><span class="cx">     return childSize;
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+// A scoped class that resets either min-width & max-width or min-height & max-height (depending on the
+// flex container main size) to their initial values on instantiation. It later resets them back to their
+// original values on destruction. It's used when computing the flex base sizes of flex items as "when determining 
+// the flex base size, the item’s min and max main sizes are ignored (no clamping occurs)". See
+// https://drafts.csswg.org/css-flexbox/#line-sizing
+class ScopedUnboundedBox {
+WTF_MAKE_NONCOPYABLE(ScopedUnboundedBox);
+public:
+    ScopedUnboundedBox(RenderBox& box, bool mainAxisIsChildInlineAxis)
+        : m_box(box)
+        , m_isHorizontalWritingModeInParallelFlow(mainAxisIsChildInlineAxis == box.isHorizontalWritingMode())
+    {
+        m_originalMin = m_isHorizontalWritingModeInParallelFlow ? m_box.style().minWidth() : m_box.style().minHeight();
+        m_originalMax = m_isHorizontalWritingModeInParallelFlow ? m_box.style().maxWidth() : m_box.style().maxHeight();
+        if (m_isHorizontalWritingModeInParallelFlow) {
+            m_box.mutableStyle().setMinWidth(RenderStyle::initialMinSize());
+            m_box.mutableStyle().setMaxWidth(RenderStyle::initialMaxSize());
+            return;
+        }
+        m_box.mutableStyle().setMinHeight(RenderStyle::initialMinSize());
+        m_box.mutableStyle().setMaxHeight(RenderStyle::initialMaxSize());
+    }
+
+
+    ~ScopedUnboundedBox()
+    {
+        if (m_isHorizontalWritingModeInParallelFlow) {
+            m_box.mutableStyle().setMinWidth(WTFMove(m_originalMin));
+            m_box.mutableStyle().setMaxWidth(WTFMove(m_originalMax));
+            return;
+        }
+        m_box.mutableStyle().setMinHeight(WTFMove(m_originalMin));
+        m_box.mutableStyle().setMaxHeight(WTFMove(m_originalMax));
+    }
+
+private:
+    RenderBox& m_box;
+    bool m_isHorizontalWritingModeInParallelFlow;
+    Length m_originalMin;
+    Length m_originalMax;
+};
+
</ins><span class="cx"> FlexItem RenderFlexibleBox::constructFlexItem(RenderBox& child, bool relayoutChildren)
</span><span class="cx"> {
</span><span class="cx">     auto childHadLayout = child.everHadLayout();
</span><del>-    child.clearOverridingContentSize();
-    if (childHasIntrinsicMainAxisSize(child)) {
-        // If this condition is true, then computeMainAxisExtentForChild will call
-        // child.intrinsicContentLogicalHeight() and child.scrollbarLogicalHeight(),
-        // so if the child has intrinsic min/max/preferred size, run layout on it now to make sure
-        // its logical height and scroll bars are up to date.
-        updateBlockChildDirtyBitsBeforeLayout(relayoutChildren, child);
-        // Don't resolve percentages in children. This is especially important for the min-height calculation,
-        // where we want percentages to be treated as auto. For flex-basis itself, this is not a problem because
-        // by definition we have an indefinite flex basis here and thus percentages should not resolve.
-        if (child.needsLayout() || !m_intrinsicSizeAlongMainAxis.contains(&child)) {
-            if (isHorizontalWritingMode() == child.isHorizontalWritingMode())
-                child.setOverridingContainingBlockContentLogicalHeight(std::nullopt);
-            else
-                child.setOverridingContainingBlockContentLogicalWidth(std::nullopt);
-            child.clearOverridingContentSize();
-            child.setChildNeedsLayout(MarkOnlyThis);
-            child.layoutIfNeeded();
-            cacheChildMainSize(child);
-            child.clearOverridingContainingBlockContentSize();
</del><ins>+    LayoutUnit borderAndPadding = isHorizontalFlow() ? child.horizontalBorderAndPaddingExtent() : child.verticalBorderAndPaddingExtent();
+    LayoutUnit childInnerFlexBaseSize;
+    {
+        ScopedUnboundedBox unbounded(child, mainAxisIsChildInlineAxis(child));
+        child.clearOverridingContentSize();
+        if (childHasIntrinsicMainAxisSize(child)) {
+            // If this condition is true, then computeMainAxisExtentForChild will call
+            // child.intrinsicContentLogicalHeight() and child.scrollbarLogicalHeight(),
+            // so if the child has intrinsic min/max/preferred size, run layout on it now to make sure
+            // its logical height and scroll bars are up to date.
+            updateBlockChildDirtyBitsBeforeLayout(relayoutChildren, child);
+            // Don't resolve percentages in children. This is especially important for the min-height calculation,
+            // where we want percentages to be treated as auto. For flex-basis itself, this is not a problem because
+            // by definition we have an indefinite flex basis here and thus percentages should not resolve.
+            if (child.needsLayout() || !m_intrinsicSizeAlongMainAxis.contains(&child)) {
+                if (isHorizontalWritingMode() == child.isHorizontalWritingMode())
+                    child.setOverridingContainingBlockContentLogicalHeight(std::nullopt);
+                else
+                    child.setOverridingContainingBlockContentLogicalWidth(std::nullopt);
+                child.clearOverridingContentSize();
+                child.setChildNeedsLayout(MarkOnlyThis);
+                child.layoutIfNeeded();
+                cacheChildMainSize(child);
+                child.clearOverridingContainingBlockContentSize();
+            }
</ins><span class="cx">         }
</span><ins>+        childInnerFlexBaseSize = computeInnerFlexBaseSizeForChild(child, borderAndPadding);
</ins><span class="cx">     }
</span><span class="cx">     
</span><del>-    LayoutUnit borderAndPadding = isHorizontalFlow() ? child.horizontalBorderAndPaddingExtent() : child.verticalBorderAndPaddingExtent();
-    LayoutUnit childInnerFlexBaseSize = computeInnerFlexBaseSizeForChild(child, borderAndPadding);
</del><span class="cx">     LayoutUnit margin = isHorizontalFlow() ? child.horizontalMarginExtent() : child.verticalMarginExtent();
</span><span class="cx">     return FlexItem(child, childInnerFlexBaseSize, borderAndPadding, margin, computeFlexItemMinMaxSizes(child), childHadLayout);
</span><span class="cx"> }
</span></span></pre>
</div>
</div>

</body>
</html>