Title: [279271] trunk
- Revision
- 279271
- Author
- [email protected]
- Date
- 2021-06-25 01:59:13 -0700 (Fri, 25 Jun 2021)
Log Message
[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.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (279270 => 279271)
--- trunk/LayoutTests/ChangeLog 2021-06-25 08:43:33 UTC (rev 279270)
+++ trunk/LayoutTests/ChangeLog 2021-06-25 08:59:13 UTC (rev 279271)
@@ -1,3 +1,18 @@
+2021-06-14 Sergio Villar Senin <[email protected]>
+
+ [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.
+
2021-06-25 Arcady Goldmints-Orlov <[email protected]>
[GTK] Unreviewed test gardening, mark more WebXR tests as unsupported
Modified: trunk/LayoutTests/TestExpectations (279270 => 279271)
--- trunk/LayoutTests/TestExpectations 2021-06-25 08:43:33 UTC (rev 279270)
+++ trunk/LayoutTests/TestExpectations 2021-06-25 08:59:13 UTC (rev 279271)
@@ -1217,6 +1217,7 @@
webkit.org/b/136754 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-015.html [ ImageOnlyFailure ]
webkit.org/b/136754 imported/w3c/web-platform-tests/css/css-flexbox/flexbox-safe-overflow-position-001.html [ ImageOnlyFailure ]
webkit.org/b/136754 imported/w3c/web-platform-tests/css/css-flexbox/percentage-max-height-002.html [ ImageOnlyFailure ]
+webkit.org/b/136754 imported/w3c/web-platform-tests/css/css-flexbox/percentage-max-height-003.html [ ImageOnlyFailure ]
webkit.org/b/210243 imported/w3c/web-platform-tests/css/css-flexbox/percentage-size-quirks-002.html [ Failure ]
webkit.org/b/136754 imported/w3c/web-platform-tests/css/css-flexbox/scrollbars-no-margin.html [ ImageOnlyFailure ]
@@ -3949,9 +3950,7 @@
webkit.org/b/209080 imported/w3c/web-platform-tests/css/css-writing-modes/vertical-alignment-srl-034.xht [ ImageOnlyFailure ]
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-001.html [ ImageOnlyFailure ]
webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-002.html [ ImageOnlyFailure ]
@@ -4023,7 +4022,6 @@
webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-inflexible-in-column-1.html [ ImageOnlyFailure ]
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.
webkit.org/b/221474 imported/w3c/web-platform-tests/css/css-flexbox/svg-root-as-flex-item-002.html [ ImageOnlyFailure ]
Modified: trunk/Source/WebCore/ChangeLog (279270 => 279271)
--- 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)
@@ -1,3 +1,27 @@
+2021-06-14 Sergio Villar Senin <[email protected]>
+
+ [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.
+
2021-06-09 Sergio Villar Senin <[email protected]>
[css-flexbox] Move flex item preferred width computation specifics to RenderFlexibleBox class
Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (279270 => 279271)
--- 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)
@@ -1370,34 +1370,80 @@
return childSize;
}
+// 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;
+};
+
FlexItem RenderFlexibleBox::constructFlexItem(RenderBox& child, bool relayoutChildren)
{
auto childHadLayout = child.everHadLayout();
- 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();
+ 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();
+ }
}
+ childInnerFlexBaseSize = computeInnerFlexBaseSizeForChild(child, borderAndPadding);
}
- LayoutUnit borderAndPadding = isHorizontalFlow() ? child.horizontalBorderAndPaddingExtent() : child.verticalBorderAndPaddingExtent();
- LayoutUnit childInnerFlexBaseSize = computeInnerFlexBaseSizeForChild(child, borderAndPadding);
LayoutUnit margin = isHorizontalFlow() ? child.horizontalMarginExtent() : child.verticalMarginExtent();
return FlexItem(child, childInnerFlexBaseSize, borderAndPadding, margin, computeFlexItemMinMaxSizes(child), childHadLayout);
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
