Commit f033b7d2 authored by Christian Biesinger's avatar Christian Biesinger Committed by Commit Bot

[css-flexbox] Apply min-height: auto to nested flexboxes again

To avoid the previous regression (see analysis in bug), we force
layout in the case where we otherwise would get an outdated result.

This relands https://chromium-review.googlesource.com/c/chromium/src/+/1246730
with an additional fix for the Chrome UI CSS. I also split out the
test change into its own patch.

IF THIS BREAKS ANY FURTHER CHROME UI:
Please don't revert this patch; instead, add min-height: 0 to any
inner nested flexboxes that may be affected by this patch. This is
an important change for web interop with the other browsers.

Bug: 596743,890100
Change-Id: Ice629c2a7823ef07d075fa23b99022b4012c6200
Reviewed-on: https://chromium-review.googlesource.com/1252682Reviewed-by: default avatarWenzhao (Colin) Zang <wzang@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595225}
parent c5fd98f7
...@@ -409,6 +409,7 @@ body.v2 { ...@@ -409,6 +409,7 @@ body.v2 {
.v2 .dialog-body { .v2 .dialog-body {
display: flex; display: flex;
height: 100%; height: 100%;
min-height: 0;
} }
.v2 .dialog-topbar { .v2 .dialog-topbar {
......
...@@ -2470,8 +2470,6 @@ crbug.com/467127 external/wpt/css/css-flexbox/getcomputedstyle/flexbox_computeds ...@@ -2470,8 +2470,6 @@ crbug.com/467127 external/wpt/css/css-flexbox/getcomputedstyle/flexbox_computeds
crbug.com/467127 [ Mac Win ] external/wpt/css/css-flexbox/ttwf-reftest-flex-wrap-reverse.html [ Failure ] crbug.com/467127 [ Mac Win ] external/wpt/css/css-flexbox/ttwf-reftest-flex-wrap-reverse.html [ Failure ]
crbug.com/467127 [ Mac Win ] external/wpt/css/css-flexbox/ttwf-reftest-flex-wrap.html [ Failure ] crbug.com/467127 [ Mac Win ] external/wpt/css/css-flexbox/ttwf-reftest-flex-wrap.html [ Failure ]
crbug.com/596743 external/wpt/css/css-flexbox/flex-minimum-height-flex-items-009.html [ Failure ]
crbug.com/467127 external/wpt/css/css-flexbox/negative-margins-001.html [ Failure ] crbug.com/467127 external/wpt/css/css-flexbox/negative-margins-001.html [ Failure ]
crbug.com/467127 external/wpt/css/css-flexbox/percentage-heights-003.html [ Failure ] crbug.com/467127 external/wpt/css/css-flexbox/percentage-heights-003.html [ Failure ]
crbug.com/467127 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-align-content-horiz-001a.xhtml [ Failure ] crbug.com/467127 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-align-content-horiz-001a.xhtml [ Failure ]
......
...@@ -437,6 +437,14 @@ bool LayoutFlexibleBox::IsMultiline() const { ...@@ -437,6 +437,14 @@ bool LayoutFlexibleBox::IsMultiline() const {
return StyleRef().FlexWrap() != EFlexWrap::kNowrap; return StyleRef().FlexWrap() != EFlexWrap::kNowrap;
} }
bool LayoutFlexibleBox::ShouldApplyMinSizeAutoForChild(
const LayoutBox& child) const {
Length min = IsHorizontalFlow() ? child.StyleRef().MinWidth()
: child.StyleRef().MinHeight();
return min.IsAuto() && !child.ShouldApplySizeContainment() &&
MainAxisOverflowForChild(child) == EOverflow::kVisible;
}
Length LayoutFlexibleBox::FlexBasisForChild(const LayoutBox& child) const { Length LayoutFlexibleBox::FlexBasisForChild(const LayoutBox& child) const {
Length flex_length = child.StyleRef().FlexBasis(); Length flex_length = child.StyleRef().FlexBasis();
if (flex_length.IsAuto()) { if (flex_length.IsAuto()) {
...@@ -997,14 +1005,7 @@ MinMaxSize LayoutFlexibleBox::ComputeMinAndMaxSizesForChild( ...@@ -997,14 +1005,7 @@ MinMaxSize LayoutFlexibleBox::ComputeMinAndMaxSizesForChild(
// computeMainAxisExtentForChild can return -1 when the child has a // computeMainAxisExtentForChild can return -1 when the child has a
// percentage min size, but we have an indefinite size in that axis. // percentage min size, but we have an indefinite size in that axis.
sizes.min_size = std::max(LayoutUnit(), sizes.min_size); sizes.min_size = std::max(LayoutUnit(), sizes.min_size);
} else if (min.IsAuto() && !child.ShouldApplySizeContainment() && } else if (ShouldApplyMinSizeAutoForChild(child)) {
MainAxisOverflowForChild(child) == EOverflow::kVisible &&
!(IsColumnFlow() && child.IsFlexibleBox())) {
// TODO(cbiesinger): 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
LayoutUnit content_size = LayoutUnit content_size =
ComputeMainAxisExtentForChild(child, kMinSize, Length(kMinContent)); ComputeMainAxisExtentForChild(child, kMinSize, Length(kMinContent));
DCHECK_GE(content_size, LayoutUnit()); DCHECK_GE(content_size, LayoutUnit());
...@@ -1124,6 +1125,14 @@ LayoutUnit LayoutFlexibleBox::AdjustChildSizeForAspectRatioCrossAxisMinAndMax( ...@@ -1124,6 +1125,14 @@ LayoutUnit LayoutFlexibleBox::AdjustChildSizeForAspectRatioCrossAxisMinAndMax(
DISABLE_CFI_PERF DISABLE_CFI_PERF
FlexItem LayoutFlexibleBox::ConstructFlexItem(LayoutBox& child, FlexItem LayoutFlexibleBox::ConstructFlexItem(LayoutBox& child,
ChildLayoutType layout_type) { ChildLayoutType layout_type) {
if (layout_type == kLayoutIfNeeded && IsColumnFlow() &&
child.IsFlexibleBox() && ShouldApplyMinSizeAutoForChild(child)) {
// In this case, we have to force-layout to update the intrinsic height of
// our child; otherwise, it may be too big because it is based on previous
// flexing of a descendant, which would be a problem for applying
// min-size: auto.
layout_type = kForceLayout;
}
if (layout_type != kNeverLayout && ChildHasIntrinsicMainAxisSize(child)) { if (layout_type != kNeverLayout && ChildHasIntrinsicMainAxisSize(child)) {
// If this condition is true, then ComputeMainAxisExtentForChild will call // If this condition is true, then ComputeMainAxisExtentForChild will call
// child.IntrinsicContentLogicalHeight() and // child.IntrinsicContentLogicalHeight() and
......
...@@ -111,6 +111,7 @@ class CORE_EXPORT LayoutFlexibleBox : public LayoutBlock { ...@@ -111,6 +111,7 @@ class CORE_EXPORT LayoutFlexibleBox : public LayoutBlock {
bool IsColumnFlow() const; bool IsColumnFlow() const;
bool IsLeftToRightFlow() const; bool IsLeftToRightFlow() const;
bool IsMultiline() const; bool IsMultiline() const;
bool ShouldApplyMinSizeAutoForChild(const LayoutBox& child) const;
Length FlexBasisForChild(const LayoutBox& child) const; Length FlexBasisForChild(const LayoutBox& child) const;
LayoutUnit CrossAxisExtentForChild(const LayoutBox& child) const; LayoutUnit CrossAxisExtentForChild(const LayoutBox& child) const;
LayoutUnit CrossAxisIntrinsicExtentForChild(const LayoutBox& child) const; LayoutUnit CrossAxisIntrinsicExtentForChild(const LayoutBox& child) const;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment