Changeset 284397 in webkit

Timestamp:

Oct 18, 2021 1:50:56 PM (9 months ago)

Author:

svillar@igalia.com

Message:

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

Reviewed by Manuel Rego Casasnovas.

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.

This is a reland of r279271 which was pretty similar but caused a visual regression in
GMail. After r284359 this is no longer a problem of this patch.

This fixes 3 WPT flexbox tests.

• rendering/RenderFlexibleBox.cpp:

(WebCore::ScopedUnboundedBoxWithFlexBasisAsChildMainSize::ScopedUnboundedBoxWithFlexBasisAsChildMainSize):
renamed from ScopedFlexBasisAsChildMainSize. Resets min|max-size to their initial values.
(WebCore::ScopedUnboundedBoxWithFlexBasisAsChildMainSize::~ScopedUnboundedBoxWithFlexBasisAsChildMainSize):
ditto. Restores min|max-size original values.
(WebCore::RenderFlexibleBox::computeFlexBaseSizeForChild):
(WebCore::ScopedFlexBasisAsChildMainSize::ScopedFlexBasisAsChildMainSize): Deleted.
(WebCore::ScopedFlexBasisAsChildMainSize::~ScopedFlexBasisAsChildMainSize): Deleted.

LayoutTests:

• TestExpectations: Unskipped 3 tests that are now passing.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderFlexibleBox.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-10-15  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
+        <rdar://problem/78100329>
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        * TestExpectations: Unskipped 3 tests that are now passing.
+
 2021-10-18  Tyler Wilcock  <tyler_w@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -4163 +4163 @@
 webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/vertical-alignment-srl-040.xht [ ImageOnlyFailure ]
 
-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-012.html [ ImageOnlyFailure ]
 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/aspect-ratio-intrinsic-size-005.html [ ImageOnlyFailure ]
@@ -4191 +4189 @@
 webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-inflexible-in-column-2.html [ ImageOnlyFailure ]
 webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-inflexible-in-row-2.html [ ImageOnlyFailure ]
-webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-min-height-1.html [ ImageOnlyFailure ]
 
 # SVGs as flex items.

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-10-15  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
+        <rdar://problem/78100329>
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        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.
+
+        This is a reland of r279271 which was pretty similar but caused a visual regression in
+        GMail. After r284359 this is no longer a problem of this patch.
+
+        This fixes 3 WPT flexbox tests.
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::ScopedUnboundedBoxWithFlexBasisAsChildMainSize::ScopedUnboundedBoxWithFlexBasisAsChildMainSize):
+        renamed from ScopedFlexBasisAsChildMainSize. Resets min|max-size to their initial values.
+        (WebCore::ScopedUnboundedBoxWithFlexBasisAsChildMainSize::~ScopedUnboundedBoxWithFlexBasisAsChildMainSize):
+        ditto. Restores min|max-size original values.
+        (WebCore::RenderFlexibleBox::computeFlexBaseSizeForChild):
+        (WebCore::ScopedFlexBasisAsChildMainSize::ScopedFlexBasisAsChildMainSize): Deleted.
+        (WebCore::ScopedFlexBasisAsChildMainSize::~ScopedFlexBasisAsChildMainSize): Deleted.
+
 2021-10-18  Tyler Wilcock  <tyler_w@apple.com>
 

trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -997 +997 @@
 }
 
-// This is a RAII class that is used to temporarily set the flex basis as the child size in the main axis.
-class ScopedFlexBasisAsChildMainSize {
+// This is a RAII class that is used to temporarily set the flex basis as the child size in the main axis. Apart from that
+// the min|max-size restrictions are ignored for this computation. See https://drafts.csswg.org/css-flexbox/#algo-main-item
+// for further details.
+class ScopedUnboundedBoxWithFlexBasisAsChildMainSize {
 public:
-    ScopedFlexBasisAsChildMainSize(RenderBox& child, Length flexBasis, bool mainAxisIsInlineAxis)
+    ScopedUnboundedBoxWithFlexBasisAsChildMainSize(RenderBox& child, Length flexBasis, bool mainAxisIsInlineAxis)
         : m_child(child)
+        , m_isHorizontalWritingModeInParallelFlow(mainAxisIsInlineAxis == child.isHorizontalWritingMode())
         , m_mainAxisIsInlineAxis(mainAxisIsInlineAxis)
     {
+        if (m_isHorizontalWritingModeInParallelFlow) {
+            m_originalMinSize = m_child.style().minWidth();
+            m_originalMaxSize = m_child.style().maxWidth();
+            m_child.mutableStyle().setMinWidth(RenderStyle::initialMinSize());
+            m_child.mutableStyle().setMaxWidth(RenderStyle::initialMaxSize());
+        } else {
+            m_originalMinSize = m_child.style().minHeight();
+            m_originalMaxSize = m_child.style().maxHeight();
+            m_child.mutableStyle().setMinHeight(RenderStyle::initialMinSize());
+            m_child.mutableStyle().setMaxHeight(RenderStyle::initialMaxSize());
+        }
+
         if (m_mainAxisIsInlineAxis) {
-            m_originalLength = m_child.style().logicalWidth();
+            m_originalSize = m_child.style().logicalWidth();
             m_child.mutableStyle().setLogicalWidth(Length(flexBasis));
             return;
         }
-        m_originalLength = m_child.style().logicalHeight();
+        m_originalSize = m_child.style().logicalHeight();
         m_child.mutableStyle().setLogicalHeight(Length(flexBasis));
     }
-    ~ScopedFlexBasisAsChildMainSize()
+    ~ScopedUnboundedBoxWithFlexBasisAsChildMainSize()
     {
+        if (m_isHorizontalWritingModeInParallelFlow) {
+            m_child.mutableStyle().setMinWidth(Length(m_originalMinSize));
+            m_child.mutableStyle().setMaxWidth(Length(m_originalMaxSize));
+        } else {
+            m_child.mutableStyle().setMinHeight(Length(m_originalMinSize));
+            m_child.mutableStyle().setMaxHeight(Length(m_originalMaxSize));
+        }
+
         if (m_mainAxisIsInlineAxis)
-            m_child.mutableStyle().setLogicalWidth(Length(m_originalLength));
+            m_child.mutableStyle().setLogicalWidth(Length(m_originalSize));
         else
-            m_child.mutableStyle().setLogicalHeight(Length(m_originalLength));
+            m_child.mutableStyle().setLogicalHeight(Length(m_originalSize));
     }
 private:
     RenderBox& m_child;
+    bool m_isHorizontalWritingModeInParallelFlow;
     bool m_mainAxisIsInlineAxis;
-    Length m_originalLength;
+    Length m_originalSize;
+    Length m_originalMinSize;
+    Length m_originalMaxSize;
 };
 
@@ -1044 +1070 @@
     // 9.3.2 E. Otherwise, size the item into the available space using its used flex basis in place of its main size.
     {
-        ScopedFlexBasisAsChildMainSize flexBasisScope(child, flexBasis, mainAxisIsChildInlineAxis(child));
+        ScopedUnboundedBoxWithFlexBasisAsChildMainSize flexBasisScope(child, flexBasis, mainAxisIsChildInlineAxis(child));
         if (mainAxisIsChildInlineAxis(child))
             return child.maxPreferredLogicalWidth() - mainAxisBorderAndPadding;