Commit 95fddf83 authored by David Grogan's avatar David Grogan Committed by Commit Bot

[css-flex] Fix flex base size for aspect ratio items of column flexboxes

This is very similar to
https://chromium-review.googlesource.com/c/chromium/src/+/2399082 ,
which dealt with items whose inline axis was the flexbox's main axis.
This patch deals with items whose block axis is the flexbox's main axis.

Old behavior: Use post-layout height of image items, obeying
min/main/max height.

New behavior: use the intrinsic height after applying the aspect ratio
and min/main/max width. Ignore computed min/main/max height.

Fixed: 987000
Change-Id: If64eeca9cc541bec863b1132aea4fd89305f5d9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2433629
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811420}
parent 95d66a58
......@@ -463,6 +463,76 @@ LayoutUnit ComputeIntrinsicInlineSizeForAspectRatioElement(
.ClampNegativeToZero();
}
// The value produced by this function will be available via
// replaced_node.Layout()->IntrinsicBlockSize(), once NGReplacedLayoutAlgorithm
// exists.
LayoutUnit ComputeIntrinsicBlockSizeForAspectRatioElement(
const NGBlockNode& node,
const NGConstraintSpace& space,
const MinMaxSizes& used_min_max_inline_sizes) {
DCHECK(node.HasAspectRatio());
LogicalSize aspect_ratio = node.GetAspectRatio();
const ComputedStyle& style = node.Style();
NGBoxStrut border_padding =
ComputeBorders(space, node) + ComputePadding(space, style);
DCHECK_NE(style.LogicalWidth().GetType(), Length::Type::kFixed)
<< "Flex will not use this function if the inline size of the replaced "
"element is definite.";
base::Optional<LayoutUnit> intrinsic_inline;
base::Optional<LayoutUnit> intrinsic_block;
node.IntrinsicSize(&intrinsic_inline, &intrinsic_block);
// intrinsic_inline and intrinsic_block can be empty independent of each
// other.
if (intrinsic_inline) {
LayoutUnit intrinsic_inline_border_box =
*intrinsic_inline + border_padding.InlineSum();
intrinsic_inline_border_box =
used_min_max_inline_sizes.ClampSizeToMinAndMax(
intrinsic_inline_border_box);
return BlockSizeFromAspectRatio(border_padding, aspect_ratio,
EBoxSizing::kContentBox,
intrinsic_inline_border_box);
}
if (intrinsic_block) {
MinMaxSizes block_min_max = {LayoutUnit(), LayoutUnit::Max()};
if (used_min_max_inline_sizes.min_size > LayoutUnit()) {
block_min_max.min_size = BlockSizeFromAspectRatio(
border_padding, aspect_ratio, EBoxSizing::kContentBox,
used_min_max_inline_sizes.min_size);
}
if (used_min_max_inline_sizes.max_size != LayoutUnit::Max()) {
block_min_max.max_size = BlockSizeFromAspectRatio(
border_padding, aspect_ratio, EBoxSizing::kContentBox,
used_min_max_inline_sizes.max_size);
}
// Minimum size wins over maximum size.
block_min_max.max_size =
std::max(block_min_max.max_size, block_min_max.min_size);
LayoutUnit intrinsic_block_border_box =
*intrinsic_block + border_padding.BlockSum();
return block_min_max.ClampSizeToMinAndMax(intrinsic_block_border_box);
}
// If control flow reaches here, the item has aspect ratio only, no natural
// sizes. Spec says:
// * If the available space is definite in the inline axis, use the stretch
// fit into that size for the inline size and calculate the block size using
// the aspect ratio.
// https://drafts.csswg.org/css-sizing-3/#intrinsic-sizes
DCHECK_NE(space.AvailableSize().inline_size, kIndefiniteSize);
NGBoxStrut margins = ComputeMarginsForSelf(space, style);
LayoutUnit stretch_into_available_inline_size(
(space.AvailableSize().inline_size - margins.InlineSum())
.ClampNegativeToZero());
return BlockSizeFromAspectRatio(border_padding, aspect_ratio,
EBoxSizing::kContentBox,
stretch_into_available_inline_size);
}
} // namespace
void NGFlexLayoutAlgorithm::ConstructAndAppendFlexItems() {
......@@ -650,12 +720,15 @@ void NGFlexLayoutAlgorithm::ConstructAndAppendFlexItems() {
}
} else {
// Parts C, D, and E for what are usually column flex containers.
//
// This is the post-layout height for aspect-ratio items, which matches
// legacy but isn't always correct.
// TODO(dgrogan): Replace with a variant of ComputeReplacedSize that
// ignores min-height, height, max-height.
flex_base_border_box = IntrinsicBlockSizeFunc();
if (child.HasAspectRatio() && child.IsReplaced() &&
RuntimeEnabledFeatures::FlexAspectRatioEnabled()) {
// Legacy uses the post-layout size for this case, which isn't always
// correct.
flex_base_border_box = ComputeIntrinsicBlockSizeForAspectRatioElement(
child, flex_basis_space, min_max_sizes_in_cross_axis_direction);
} else {
flex_base_border_box = IntrinsicBlockSizeFunc();
}
}
} else {
// Part A of 9.2.3 https://drafts.csswg.org/css-flexbox/#algo-main-item
......
......@@ -487,6 +487,8 @@ crbug.com/704294 external/wpt/css/css-flexbox/flex-aspect-ratio-img-row-006.html
crbug.com/704294 external/wpt/css/css-flexbox/canvas-dynamic-change-001.html [ Failure ]
crbug.com/987000 external/wpt/css/css-flexbox/flex-aspect-ratio-img-row-007.html [ Failure ]
crbug.com/987000 external/wpt/css/css-flexbox/flex-aspect-ratio-img-row-010.html [ Failure ]
crbug.com/987000 external/wpt/css/css-flexbox/flex-aspect-ratio-img-column-012.html [ Failure ]
crbug.com/987000 external/wpt/css/css-flexbox/flex-aspect-ratio-img-column-015.html [ Failure ]
crbug.com/987000 external/wpt/css/css-flexbox/svg-root-as-flex-item-002.html [ Failure ]
# These would need a rebaseline back from LayoutNGBlockFlow to LayoutBlockFlow
......
<!doctype html>
<title>Aspect-ratio flex item</title>
<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
<meta name="assert" content="flex base size is not influenced by specified min-height for aspect ratio items">
<p>Test passes if there is a filled green square.</p>
<!-- Chrome 86 makes the green rectangle be 100 x 149.5 -->
<!-- Make this align-items:flex-start because with align-items:stretch, 100x149.5 is the right size due to Part B of the flex base size algorithm. -->
<div style="display: flex; flex-direction: column; width:100px; height: 200px; align-items: flex-start;">
<img style="min-height: 100px; flex: 1 0 auto;" src="support/1x1-green.png">
<div style="flex: 1 0 1px;"></div>
</div>
<!DOCTYPE html>
<title>SVG img as flex item</title>
<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
<meta name="assert" content="SVG's specified intrinsic height is honored when used as a flex base size and no intrinsic width is specified." />
<p>Test passes if there is a filled green square.</p>
<div style="display: flex; width: 100px; height: 100px; align-items: flex-start;">
<img src="data:image/svg+xml,%3Csvg viewBox='0 0 400 200' height='50px' xmlns='http://www.w3.org/2000/svg' %3E%3Crect width='100%' height='100%' fill='green' /%3E%3C/svg%3E" style="border-top: 50px solid green; min-height: 0px;">
</div>
<!DOCTYPE html>
<title>SVG img as flex item</title>
<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
<link rel="help" href="https://www.w3.org/TR/CSS22/visudet.html#min-max-widths" title="w > max-width line of the table">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<meta name="assert" content="Flex base size of image item with aspect ratio + intrinsic width + no intrinsic height honors transferred max-width." />
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="display: flex; flex-direction: column; width: 100px; align-items: flex-start">
<img src="data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 100 100' width='200'%3E%3Crect width='100%25' height='100%25' fill='green'/%3E%3C/svg%3E" style="max-width: 100px; flex: 0 0 auto; background: red;">
</div>
<!DOCTYPE html>
<title>SVG img as flex item</title>
<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
<link rel="help" href="https://www.w3.org/TR/CSS22/visudet.html#min-max-widths" title="h > max-height line of the table">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<meta name="assert" content="Flex base size of image item with aspect ratio + intrinsic height + no intrinsic width honors transferred max-width." />
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="display: flex; flex-direction: column; width: 100px; align-items: flex-start">
<img src="data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 100 100' height='200'%3E%3Crect width='100%25' height='100%25' fill='green'/%3E%3C/svg%3E" style="max-width: 100px; flex: 0 0 auto; background: red;">
</div>
<!DOCTYPE html>
<title>SVG root as flex item</title>
<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
<link rel="help" href="https://www.w3.org/TR/css-sizing-3/#intrinsic-sizes" title="For boxes with an intrinsic aspect ratio, but no intrinsic size">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<meta name="assert" content="SVG's intrinsic width when used as flex base size stretches into the available size when it has no specified intrinsic sizes and is passed through the aspect ratio " />
<style>
#reference-overlapped-red {
position: absolute;
background-color: red;
width: 100px;
height: 100px;
z-index: -1;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div id="reference-overlapped-red"></div>
<div style="display: flex; flex-direction: column; width: 100px; align-items: flex-start;">
<svg viewBox="0 0 200 200">
<rect width="100%" height="100%" fill="green" />
</svg>
</div>
<!DOCTYPE html>
<title>SVG root as flex item</title>
<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
<link rel="help" href="https://www.w3.org/TR/css-sizing-3/#intrinsic-sizes" title="For boxes with an intrinsic aspect ratio, but no intrinsic size">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<meta name="assert" content="SVG's intrinsic width when used as flex base size stretches into the available size (minus inline margins) when it has no specified intrinsic sizes and is passed through the aspect ratio, block margins have no effect. " />
<style>
#reference-overlapped-red {
position: absolute;
background-color: red;
width: 100px;
height: 100px;
z-index: -1;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div id="reference-overlapped-red"></div>
<div style="display: flex; flex-direction: column; width: 150px; align-items: flex-start;">
<svg viewBox="0 0 200 200" style="margin-right: 50px; flex 1 0 auto; margin-bottom: 70px;">
<rect width="100%" height="100%" fill="green" />
</svg>
</div>
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