Skip to content

GitLab

T
tangled
Commit 5918bde3 authored by Christian Biesinger's avatar Christian Biesinger Committed by Commit Bot
Browse files
Options

[css-flexbox] ChildIntrinsicLogicalWidth should use fit-content, not max-content 

This function needs to match the actual sizing we use. But, since
this is the cross-axis size, we use fit-content:
https://drafts.csswg.org/css-flexbox/#algo-cross-item

See LayoutBox::SizesLogicalWidthToFitContent and LayoutBox::ComputeLogicalWidthUsing
for where we do that fit-content sizing; this code makes us
match the ComputeLogicalWidthUsing calculation.

This bug is currently somewhat hard to trigger because it requires
a combination of all of:
- flex-wrap: wrap
- flex-direction: column
- More than one flex line
- Not using align-items: flex-start or align-content: flex-start
- No explicit width on the flex item
- The max-content width needs to be larger than the width of flexbox

The reason this does not matter in other cases is because we only use
this function to set FlexLine::cross_axis_extent, which we overwrite in
AlignFlexLines if we only have one line. And if we do have more than
one line, we only use this value for aligning items and lines, so
it only matters if we don't use flex-start alignment.

This bug will be much easier to trigger once bug 599828 is fixed (there
are at least two real-world sites that are affected by this once
that bug is fixed)

Bug: 876749
Change-Id: I673026bedfaf0e4c4a25830ef686b200ec8f54a1
Reviewed-on: https://chromium-review.googlesource.com/c/1327746
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607342}
parent ba9a4288
Show whitespace changes
Inline Side-by-side
Showing with 61 additions and 3 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ 5918bde3
......@@ -1637,6 +1637,8 @@ crbug.com/249112 virtual/layout_ng_experimental/external/wpt/css/css-flexbox/fle
crbug.com/467127 virtual/layout_ng_experimental/external/wpt/css/css-flexbox/flex-minimum-width-flex-items-008.xht [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-flexbox/flex-order.html [ Failure ]
crbug.com/591099 [ Mac ] virtual/layout_ng_experimental/external/wpt/css/css-flexbox/flex-vertical-align-effect.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-flexbox/flex-wrap-002.html [ Skip ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-flexbox/flex-wrap-003.html [ Skip ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-flexbox/flexbox-flex-direction-column-reverse.htm [ Skip ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-flexbox/flexbox-flex-direction-column.htm [ Skip ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-flexbox/flexbox-flex-wrap-flexing.html [ Failure ]
......
third_party/WebKit/LayoutTests/external/wpt/css/css-flexbox/flex-wrap-002.html 0 → 100644
View file @ 5918bde3
<!DOCTYPE html>
<link rel="author" title="Google LLC" href="http://www.google.com" />
<link rel="help" href="https://drafts.csswg.org/css-flexbox/#cross-sizing" />
<title>css-flexbox: Tests that we size items in a wrapping column flexbox as fit-content</title>
<link rel="match" href="../reference/ref-filled-green-100px-square.xht" />
<meta name="assert" content="The flexbox here should have two flex lines, each 50px wide and 100px tall. The flex items overflow but are transparent." />
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<!-- This makes sure that we only see green if the flex items are sized correctly -->
<div style="position: absolute; width: 100px; height: 100px; background: green;"></div>
<div style="display: flex; flex-direction: column; flex-wrap: wrap; width: 50px; height: 100px; line-height: 20px;">
<div style="background-color: red; height: 100px;">
<!-- These zero-height divs give the flex item has a min-content width of
25px and a max-content width of 125px -->
<div style="width: 25px; display: inline-block;"></div>
<div style="width: 25px; display: inline-block;"></div>
<div style="width: 25px; display: inline-block;"></div>
<div style="width: 25px; display: inline-block;"></div>
<div style="width: 25px; display: inline-block;"></div>
</div>
<div style="background-color: red; height: 100px;">
<div style="width: 25px; display: inline-block;"></div>
<div style="width: 25px; display: inline-block;"></div>
<div style="width: 25px; display: inline-block;"></div>
<div style="width: 25px; display: inline-block;"></div>
<div style="width: 25px; display: inline-block;"></div>
</div>
</div>
third_party/WebKit/LayoutTests/external/wpt/css/css-flexbox/flex-wrap-003.html 0 → 100644
View file @ 5918bde3
<!DOCTYPE html>
<link rel="author" title="Google LLC" href="http://www.google.com" />
<link rel="help" href="https://drafts.csswg.org/css-flexbox/#cross-sizing" />
<title>css-flexbox: Tests that we size items in a wrapping column flexbox as fit-content</title>
<link rel="match" href="../reference/ref-filled-green-100px-square.xht" />
<meta name="assert" content="The flexbox here should have one flex line, 100px by 100px. The flex items overflow but are transparent." />
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<!-- This makes sure that we only see green if the flex items are sized correctly -->
<div style="position: absolute; width: 100px; height: 100px; background: green;"></div>
<div style="display: flex; flex-direction: column; flex-wrap: wrap; width: 100px; height: 100px; line-height: 20x;">
<div style="background-color: red; height: 100px;">
<!-- These zero-height divs give the flex item has a min-content width of
50px and a max-content width of 250px -->
<div style="width: 50px; display: inline-block;"></div>
<div style="width: 50px; display: inline-block;"></div>
<div style="width: 50px; display: inline-block;"></div>
<div style="width: 50px; display: inline-block;"></div>
<div style="width: 50px; display: inline-block;"></div>
</div>
</div>
third_party/blink/renderer/core/layout/layout_flexible_box.cc
View file @ 5918bde3
......@@ -480,9 +480,12 @@ LayoutUnit LayoutFlexibleBox::ChildIntrinsicLogicalWidth(
// This should only be called if the logical width is the cross size
DCHECK(!MainAxisIsInlineAxis(child));
// If our height is auto, make sure that our returned height is unaffected by
// earlier layouts by returning the max preferred logical width
if (!CrossAxisLengthIsDefinite(child, child.StyleRef().LogicalWidth()))
return child.MaxPreferredLogicalWidth();
// earlier layouts by returning the shrink-to-fit size.
if (!CrossAxisLengthIsDefinite(child, child.StyleRef().LogicalWidth())) {
MinMaxSize sizes{child.MinPreferredLogicalWidth(),
child.MaxPreferredLogicalWidth()};
return sizes.ShrinkToFit(ContentLogicalWidth());
}
return child.LogicalWidth();
}
......
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