Diff
Modified: trunk/LayoutTests/ChangeLog (266694 => 266695)
--- trunk/LayoutTests/ChangeLog 2020-09-07 08:28:41 UTC (rev 266694)
+++ trunk/LayoutTests/ChangeLog 2020-09-07 09:20:46 UTC (rev 266695)
@@ -1,3 +1,20 @@
+2020-09-02 Sergio Villar Senin <[email protected]>
+
+ [css-flexbox] min-height: auto not applied to nested flexboxes.
+ https://bugs.webkit.org/show_bug.cgi?id=210089
+
+ Reviewed by Daniel Bates.
+
+ Apart from enabling some tests we're removing a test which is now invalid as it was added
+ under the condition that we were not matching the specs wrt percentage height computation in
+ column flexboxes.
+
+ * TestExpectations: Removed two test cases that are passing now.
+ * fast/flexbox/nested-column-intrinsic-min-disabled-expected.html: Removed.
+ * fast/flexbox/nested-column-intrinsic-min-disabled.html: Removed.
+ * platform/ios/fast/forms/auto-fill-button/input-strong-password-auto-fill-button-expected.txt: Updated.
+ * platform/mac/fast/forms/auto-fill-button/input-strong-password-auto-fill-button-expected.txt: Ditto.
+
2020-09-06 Chris Dumez <[email protected]>
ConvolverNode incorrectly outputs silence because m_reverb is null
Modified: trunk/LayoutTests/TestExpectations (266694 => 266695)
--- trunk/LayoutTests/TestExpectations 2020-09-07 08:28:41 UTC (rev 266694)
+++ trunk/LayoutTests/TestExpectations 2020-09-07 09:20:46 UTC (rev 266695)
@@ -3935,8 +3935,6 @@
webkit.org/b/206767 imported/w3c/web-platform-tests/css/css-flexbox/gap-016.html [ ImageOnlyFailure ]
webkit.org/b/209649 imported/w3c/web-platform-tests/css/css-flexbox/padding-overflow-crash.html [ ImageOnlyFailure ]
webkit.org/b/210077 imported/w3c/web-platform-tests/css/css-flexbox/flex-basis-010.html [ ImageOnlyFailure ]
-webkit.org/b/210089 imported/w3c/web-platform-tests/css/css-flexbox/flex-minimum-height-flex-items-011.xht [ ImageOnlyFailure ]
-webkit.org/b/210089 imported/w3c/web-platform-tests/css/css-flexbox/flex-minimum-height-flex-items-013.html [ ImageOnlyFailure ]
webkit.org/b/210093 imported/w3c/web-platform-tests/css/css-flexbox/select-element-zero-height-001.html [ ImageOnlyFailure ]
webkit.org/b/210093 imported/w3c/web-platform-tests/css/css-flexbox/select-element-zero-height-002.html [ ImageOnlyFailure ]
webkit.org/b/210144 imported/w3c/web-platform-tests/css/css-flexbox/anonymous-flex-item-005.html [ ImageOnlyFailure ]
Deleted: trunk/LayoutTests/fast/flexbox/nested-column-intrinsic-min-disabled-expected.html (266694 => 266695)
--- trunk/LayoutTests/fast/flexbox/nested-column-intrinsic-min-disabled-expected.html 2020-09-07 08:28:41 UTC (rev 266694)
+++ trunk/LayoutTests/fast/flexbox/nested-column-intrinsic-min-disabled-expected.html 2020-09-07 09:20:46 UTC (rev 266695)
@@ -1,4 +0,0 @@
-<html>
-<body>
-<div>This height should be 0. We are violating the spec (section 4.5) for now until we can implement nested column intrinsic sizing properly.</div>
-</body>
Deleted: trunk/LayoutTests/fast/flexbox/nested-column-intrinsic-min-disabled.html (266694 => 266695)
--- trunk/LayoutTests/fast/flexbox/nested-column-intrinsic-min-disabled.html 2020-09-07 08:28:41 UTC (rev 266694)
+++ trunk/LayoutTests/fast/flexbox/nested-column-intrinsic-min-disabled.html 2020-09-07 09:20:46 UTC (rev 266695)
@@ -1,17 +0,0 @@
-<html>
-<head>
-<style>
-
-.flexbox {
- display: -webkit-flex;
-}
-
-.column {
- -webkit-flex-direction: column;
-}
-</style>
-</head>
-<body>
-<div class="flexbox column" style="height:0px">
-<div style="display:flex;background-color:red">This height should be 0. We are violating the spec (section 4.5) for now until we can implement nested column intrinsic sizing properly.</div>
-</div>
Modified: trunk/LayoutTests/platform/ios/fast/forms/auto-fill-button/input-strong-password-auto-fill-button-expected.txt (266694 => 266695)
--- trunk/LayoutTests/platform/ios/fast/forms/auto-fill-button/input-strong-password-auto-fill-button-expected.txt 2020-09-07 08:28:41 UTC (rev 266694)
+++ trunk/LayoutTests/platform/ios/fast/forms/auto-fill-button/input-strong-password-auto-fill-button-expected.txt 2020-09-07 09:20:46 UTC (rev 266695)
@@ -48,7 +48,7 @@
RenderBlock {DIV} at (0,1) size 206x12
layer at (654,57) size 20x12 scrollWidth 94 scrollHeight 26
RenderFlexibleBox {DIV} at (6,3) size 21x13
- RenderBlock {DIV} at (0,12) size 20x14 [color=#000000CC]
+ RenderBlock {DIV} at (0,12) size 95x14 [color=#000000CC]
RenderText {#text} at (6,0) size 89x14
text run at (6,0) width 89: "Strong Password"
layer at (654,57) size 20x12
@@ -62,7 +62,7 @@
RenderBlock {DIV} at (0,1) size 46x12
layer at (176,134) size 20x12 scrollWidth 94 scrollHeight 26
RenderFlexibleBox {DIV} at (6,47) size 21x13
- RenderBlock {DIV} at (0,12) size 20x14 [color=#000000CC]
+ RenderBlock {DIV} at (0,12) size 95x14 [color=#000000CC]
RenderText {#text} at (6,0) size 89x14
text run at (6,0) width 89: "Strong Password"
layer at (176,134) size 20x12
Modified: trunk/LayoutTests/platform/mac/fast/forms/auto-fill-button/input-strong-password-auto-fill-button-expected.txt (266694 => 266695)
--- trunk/LayoutTests/platform/mac/fast/forms/auto-fill-button/input-strong-password-auto-fill-button-expected.txt 2020-09-07 08:28:41 UTC (rev 266694)
+++ trunk/LayoutTests/platform/mac/fast/forms/auto-fill-button/input-strong-password-auto-fill-button-expected.txt 2020-09-07 09:20:46 UTC (rev 266695)
@@ -48,7 +48,7 @@
RenderBlock {DIV} at (0,0) size 206x13
layer at (653,55) size 20x13 scrollWidth 94 scrollHeight 26
RenderFlexibleBox {DIV} at (3,3) size 20x13
- RenderBlock {DIV} at (0,13) size 20x13 [color=#000000CC]
+ RenderBlock {DIV} at (0,13) size 95x13 [color=#000000CC]
RenderText {#text} at (6,0) size 89x13
text run at (6,0) width 89: "Strong Password"
layer at (653,55) size 20x13
@@ -62,7 +62,7 @@
RenderBlock {DIV} at (0,43) size 58x14
layer at (177,133) size 20x13 scrollWidth 94 scrollHeight 26
RenderFlexibleBox {DIV} at (3,46) size 20x14
- RenderBlock {DIV} at (0,13) size 20x13 [color=#000000CC]
+ RenderBlock {DIV} at (0,13) size 95x13 [color=#000000CC]
RenderText {#text} at (6,0) size 89x13
text run at (6,0) width 89: "Strong Password"
layer at (177,133) size 20x13
Modified: trunk/Source/WebCore/ChangeLog (266694 => 266695)
--- trunk/Source/WebCore/ChangeLog 2020-09-07 08:28:41 UTC (rev 266694)
+++ trunk/Source/WebCore/ChangeLog 2020-09-07 09:20:46 UTC (rev 266695)
@@ -1,3 +1,37 @@
+2020-09-02 Sergio Villar Senin <[email protected]>
+
+ [css-flexbox] min-height: auto not applied to nested flexboxes.
+ https://bugs.webkit.org/show_bug.cgi?id=210089
+
+ Reviewed by Daniel Bates.
+
+ Nested flexboxes with column direction were not computing correctly min-size:auto because
+ we were explicitly preventing them from doing so in the code. Implemented the required bits to
+ make it work correctly and thus removed the retriction. The idea is to set an indefinite override
+ containing block size so that percentages would be resolved to auto as spec'ed. The code which
+ decides whether to apply min-size:auto was refactored in the shouldApplyMinSizeAutoForChild() method.
+
+ In order not to cause regressions some other two additional changes were also implemented. First we
+ had to adjust childHasIntrinsicMainAxisSize() so that it also takes into account the cases where
+ shouldApplyMinSizeAutoForChild() is true and return true. Secondly we had to add an additional case
+ to mainAxisLengthIsDefinite() so that it returns false for column flows where the flexBasis is intrinsic.
+
+ Inspired by Blink's crrev.com/c/1641510, crrev.com/c/1269995 & crrev.com/c/1786297 by <[email protected]>
+
+ * rendering/RenderFlexibleBox.cpp:
+ (WebCore::RenderFlexibleBox::shouldApplyMinSizeAutoForChild const): Refactored from
+ RenderFlexibleBox::adjustChildSizeForMinAndMax.
+ (WebCore::RenderFlexibleBox::mainAxisLengthIsDefinite const): Additional case for column flows.
+ (WebCore::RenderFlexibleBox::layoutFlexItems): Reset m_hasDefiniteHeight to Unknown after calling
+ constructFlexItem() because the latter might set now an override containing block height which basically
+ potentially makes any cached size value incorrect.
+ (WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax): Directly call shouldApplyMinSizeAutoForChild().
+ (WebCore::RenderFlexibleBox::constructFlexItem): Set an indefinite override containing block size for
+ children with percentage sizes so that they're resolved as auto.
+ (WebCore::RenderFlexibleBox::childHasIntrinsicMainAxisSize const): Return true for those cases where the
+ main axis length is indefinite and also when shouldApplyMinSizeAutoForChild().
+ * rendering/RenderFlexibleBox.h:
+
2020-09-06 Myles C. Maxfield <[email protected]>
[iOS] attachmentActionFont() Needs to use kCTFontSymbolicTrait: @(kCTFontTraitTightLeading | kCTFontTraitEmphasized) to get the short emphasized footnote font
Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (266694 => 266695)
--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2020-09-07 08:28:41 UTC (rev 266694)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp 2020-09-07 09:20:46 UTC (rev 266695)
@@ -438,6 +438,13 @@
return style().flexWrap() != FlexWrap::NoWrap;
}
+bool RenderFlexibleBox::shouldApplyMinSizeAutoForChild(const RenderBox& child) const
+{
+ // css-flexbox section 4.5
+ auto minSize = isHorizontalFlow() ? child.style().minWidth() : child.style().minHeight();
+ return minSize.isAuto() && mainAxisOverflowForChild(child) == Overflow::Visible;
+}
+
Length RenderFlexibleBox::flexBasisForChild(const RenderBox& child) const
{
Length flexLength = child.style().flexBasis();
@@ -796,6 +803,8 @@
{
if (flexBasis.isAuto())
return false;
+ if (isColumnFlow() && flexBasis.isIntrinsic())
+ return false;
if (flexBasis.isPercentOrCalculated()) {
if (!isColumnFlow() || m_hasDefiniteHeight == SizeDefiniteness::Definite)
return true;
@@ -914,6 +923,9 @@
allItems.append(constructFlexItem(*child, relayoutChildren));
}
m_reversedOrderIteratorForHitTesting.reverse();
+
+ // constructFlexItem() might set the override containing block height so any value cached for definiteness might be incorrect.
+ m_hasDefiniteHeight = SizeDefiniteness::Unknown;
const LayoutUnit lineBreakLength = mainAxisContentExtent(LayoutUnit::max());
FlexLayoutAlgorithm flexAlgorithm(style(), lineBreakLength, allItems);
@@ -1128,12 +1140,7 @@
if (min.isSpecifiedOrIntrinsic())
return std::max(childSize, std::max(0_lu, computeMainAxisExtentForChild(child, MinSize, min).valueOr(childSize)));
- if (!isFlexibleBoxImpl() && min.isAuto() && mainAxisOverflowForChild(child) == Overflow::Visible && !(isColumnFlow() && is<RenderFlexibleBox>(child))) {
- // FIXME: For now, we do not handle min-height: auto for nested
- // column flexboxes. We need to implement
- // https://drafts.csswg.org/css-flexbox/#intrinsic-sizes before that
- // produces reasonable results. Tracking bug: https://crbug.com/581553
- // css-flexbox section 4.5
+ if (shouldApplyMinSizeAutoForChild(child)) {
// FIXME: If the min value is expected to be valid here, we need to come up with a non optional version of computeMainAxisExtentForChild and
// ensure it's valid through the virtual calls of computeIntrinsicLogicalContentHeightUsing.
LayoutUnit contentSize = computeMainAxisExtentForChild(child, MinSize, Length(MinContent)).valueOr(0);
@@ -1232,17 +1239,27 @@
FlexItem RenderFlexibleBox::constructFlexItem(RenderBox& child, bool relayoutChildren)
{
- // 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.
- if (childHasIntrinsicMainAxisSize(child) && child.needsLayout()) {
- child.clearOverrideContentSize();
- child.setChildNeedsLayout(MarkOnlyThis);
- child.layoutIfNeeded();
- cacheChildMainSize(child);
- relayoutChildren = false;
+ 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.setOverrideContainingBlockContentLogicalHeight(WTF::nullopt);
+ else
+ child.setOverrideContainingBlockContentLogicalWidth(WTF::nullopt);
+ child.clearOverrideContentSize();
+ child.setChildNeedsLayout(MarkOnlyThis);
+ child.layoutIfNeeded();
+ cacheChildMainSize(child);
+ relayoutChildren = false;
+ child.clearOverrideContainingBlockContentSize();
+ }
}
LayoutUnit borderAndPadding = isHorizontalFlow() ? child.horizontalBorderAndPaddingExtent() : child.verticalBorderAndPaddingExtent();
@@ -1556,15 +1573,23 @@
bool RenderFlexibleBox::childHasIntrinsicMainAxisSize(const RenderBox& child) const
{
- bool result = false;
- if (isHorizontalFlow() != child.style().isHorizontalWritingMode()) {
- Length childFlexBasis = flexBasisForChild(child);
- Length childMinSize = isHorizontalFlow() ? child.style().minWidth() : child.style().minHeight();
- Length childMaxSize = isHorizontalFlow() ? child.style().maxWidth() : child.style().maxHeight();
- if (childFlexBasis.isIntrinsic() || childMinSize.isIntrinsicOrAuto() || childMaxSize.isIntrinsic())
- result = true;
- }
- return result;
+ if (isHorizontalFlow() == child.style().isHorizontalWritingMode())
+ return false;
+
+ Length childFlexBasis = flexBasisForChild(child);
+ Length childMinSize = isHorizontalFlow() ? child.style().minWidth() : child.style().minHeight();
+ Length childMaxSize = isHorizontalFlow() ? child.style().maxWidth() : child.style().maxHeight();
+ // FIXME: we must run mainAxisLengthIsDefinite() because it might end up calling computePercentageLogicalHeight()
+ // which has some side effects like calling addPercentHeightDescendant() for example so it is not possible to skip
+ // the call for example by moving it to the end of the conditional _expression_. This is error-prone and we should
+ // refactor computePercentageLogicalHeight() at some point so that it only computes stuff without those side effects.
+ if (!mainAxisLengthIsDefinite(child, childFlexBasis) || childMinSize.isIntrinsic() || childMaxSize.isIntrinsic())
+ return true;
+
+ if (shouldApplyMinSizeAutoForChild(child))
+ return true;
+
+ return false;
}
Overflow RenderFlexibleBox::mainAxisOverflowForChild(const RenderBox& child) const
Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.h (266694 => 266695)
--- trunk/Source/WebCore/rendering/RenderFlexibleBox.h 2020-09-07 08:28:41 UTC (rev 266694)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.h 2020-09-07 09:20:46 UTC (rev 266695)
@@ -113,6 +113,7 @@
bool isLeftToRightFlow() const;
bool isMultiline() const;
Length flexBasisForChild(const RenderBox& child) const;
+ bool shouldApplyMinSizeAutoForChild(const RenderBox&) const;
LayoutUnit crossAxisExtentForChild(const RenderBox& child) const;
LayoutUnit crossAxisIntrinsicExtentForChild(const RenderBox& child) const;
LayoutUnit childIntrinsicLogicalHeight(const RenderBox& child) const;
