From 202cac900d6b660bd21de33d3433cf0ae9d8f0ab Mon Sep 17 00:00:00 2001 From: Oriol Brufau Date: Fri, 4 Apr 2025 03:15:40 -0700 Subject: [PATCH] layout: Restrict stretch alignment to flex items with computed auto size (#36288) We were allowing `align-self: stretch` to stretch flex items whose cross size behaves as `auto`, including cyclic percentages. However, https://github.com/w3c/csswg-drafts/issues/4525 resolved that stretching should only happen when the cross size computes to `auto`. So this patch exposes this information in `ContentBoxSizesAndPBM`, and refactors the flexbox stretching logic. Fixes: #36285 Testing: - `/css/css-flexbox/quirks-auto-block-size-with-percentage-item.html` - `/css/css-flexbox/stretch-requires-computed-auto-size.html` Signed-off-by: Oriol Brufau --- components/layout_2020/flexbox/layout.rs | 118 +++++++++--------- components/layout_2020/flow/mod.rs | 2 + components/layout_2020/sizing.rs | 3 +- components/layout_2020/style_ext.rs | 5 +- tests/wpt/meta/MANIFEST.json | 13 ++ ...o-block-size-with-percentage-item.html.ini | 3 - .../stretch-requires-computed-auto-size.html | 17 +++ 7 files changed, 94 insertions(+), 67 deletions(-) delete mode 100644 tests/wpt/meta/css/css-flexbox/quirks-auto-block-size-with-percentage-item.html.ini create mode 100644 tests/wpt/tests/css/css-flexbox/stretch-requires-computed-auto-size.html diff --git a/components/layout_2020/flexbox/layout.rs b/components/layout_2020/flexbox/layout.rs index f71f5da6915..28466f32efa 100644 --- a/components/layout_2020/flexbox/layout.rs +++ b/components/layout_2020/flexbox/layout.rs @@ -57,7 +57,12 @@ struct FlexContext<'a> { /// A flex item with some intermediate results struct FlexItem<'a> { box_: &'a FlexItemBox, + + /// The preferred, min and max inner cross sizes. If the flex container is single-line + /// and [`Self::cross_size_stretches_to_line`] is true, then the preferred cross size + /// is set to [`Size::Stretch`]. content_cross_sizes: Sizes, + padding: FlexRelativeSides, border: FlexRelativeSides, margin: FlexRelativeSides, @@ -94,10 +99,17 @@ struct FlexItem<'a> { /// preferred_aspect_ratio: Option, - /// Whether an [auto](Size::Initial) [preferred](Sizes::preferred) - /// [cross size](Self::content_cross_sizes) stretches the item - /// to fill the flex container. - auto_cross_size_stretches_to_container_size: bool, + /// Whether the preferred cross size of the item stretches to fill the flex line. + /// This happens when the size computes to `auto`, the used value of `align-self` + /// is `stretch`, and neither of the cross-axis margins are `auto`. + /// + /// + /// Note the following sizes are not sufficient: + /// - A size that only behaves as `auto` (like a cyclic percentage). + /// The computed value needs to be `auto` too. + /// - A `stretch` size. It stretches to the containing block, not to the line + /// (under discussion in ). + cross_size_stretches_to_line: bool, } /// Child of a FlexContainer. Can either be absolutely positioned, or not. If not, @@ -324,21 +336,6 @@ struct FinalFlexLineLayout { } impl FlexContainerConfig { - /// Whether an item with an `auto` preferred cross size needs to be stretched - /// to fill the flex container. - /// - fn item_with_auto_cross_size_stretches_to_container_size( - &self, - item_style: &ComputedValues, - item_margin: &FlexRelativeSides, - ) -> bool { - self.container_is_single_line && - item_with_auto_cross_size_stretches_to_line_size( - AlignItems(self.resolve_align_self_for_child(item_style)), - item_margin, - ) - } - fn resolve_reversable_flex_alignment( &self, align_flags: AlignFlags, @@ -1127,14 +1124,6 @@ impl<'a> FlexItem<'a> { &|| flex_context, ) } - - fn stretches_to_line(&self) -> bool { - // Note this returns false for a `stretch` size, because that stretches to the - // containing block, not to the flex line. - // To be discussed in https://github.com/w3c/csswg-drafts/issues/11784. - self.content_cross_sizes.preferred.is_initial() && - item_with_auto_cross_size_stretches_to_line_size(self.align_self, &self.margin) - } } fn cross_axis_is_item_block_axis( @@ -1148,7 +1137,8 @@ fn cross_axis_is_item_block_axis( container_is_row ^ item_is_orthogonal } -/// Whether an item with an `auto` preferred cross size will stretched to fill the cross size of its flex line. +/// Whether an item with a computed preferred cross size of `auto` will stretch +/// to fill the cross size of its flex line. /// fn item_with_auto_cross_size_stretches_to_line_size( align_self: AlignItems, @@ -1585,8 +1575,7 @@ impl InitialFlexLineLayout<'_> { let mut item_used_cross_sizes = Vec::with_capacity(item_count); let mut item_margins = Vec::with_capacity(item_count); for item in self.items.iter_mut() { - let stretches = item.item.stretches_to_line(); - let used_cross_size = if stretches { + let used_cross_size = if item.item.cross_size_stretches_to_line { let (axis, content_size) = match flex_context.config.flex_axis { FlexAxis::Row => (Direction::Block, item.layout_result.content_size.block), FlexAxis::Column => (Direction::Inline, item.layout_result.content_size.inline), @@ -1611,7 +1600,7 @@ impl InitialFlexLineLayout<'_> { // “If the flex item has `align-self: stretch`, redo layout for its contents, // treating this used size as its definite cross size so that percentage-sized // children can be resolved.” - if stretches { + if item.item.cross_size_stretches_to_line { let new_layout = item.item.layout( item.used_main_size, flex_context, @@ -1821,11 +1810,6 @@ impl FlexItem<'_> { (used_main_size, cross_size) } else { let cross_size = used_cross_size_override.unwrap_or_else(|| { - let automatic_size = if self.auto_cross_size_stretches_to_container_size { - Size::Stretch - } else { - Size::FitContent - }; let stretch_size = Au::zero().max(containing_block.size.inline - self.pbm_auto_is_zero.cross); let get_content_size = || { @@ -1840,7 +1824,7 @@ impl FlexItem<'_> { }; self.content_cross_sizes.resolve( Direction::Inline, - automatic_size, + Size::FitContent, Au::zero, Some(stretch_size), get_content_size, @@ -1957,7 +1941,7 @@ impl FlexItem<'_> { containing_block, &independent_formatting_context.base, flex_axis == FlexAxis::Column || - self.stretches_to_line() || + self.cross_size_stretches_to_line || self.depends_on_block_constraints, ); let CacheableLayoutResult { @@ -2200,11 +2184,13 @@ impl FlexItemBox { } else { Direction::Block }; + let align_self = AlignItems(config.resolve_align_self_for_child(style)); let ContentBoxSizesAndPBM { content_box_sizes, pbm, depends_on_block_constraints, + preferred_size_computes_to_auto, } = content_box_sizes_and_pbm; let preferred_aspect_ratio = self @@ -2219,12 +2205,28 @@ impl FlexItemBox { main: padding_border.main, cross: padding_border.cross, } + margin_auto_is_zero.sum_by_axis(); - let auto_cross_size_stretches_to_container_size = - config.item_with_auto_cross_size_stretches_to_container_size(style, &margin); - let (content_main_sizes, content_cross_sizes) = match flex_axis { - FlexAxis::Row => (&content_box_sizes.inline, &content_box_sizes.block), - FlexAxis::Column => (&content_box_sizes.block, &content_box_sizes.inline), - }; + let (content_main_sizes, mut content_cross_sizes, cross_size_computes_to_auto) = + match flex_axis { + FlexAxis::Row => ( + &content_box_sizes.inline, + content_box_sizes.block.clone(), + preferred_size_computes_to_auto.block, + ), + FlexAxis::Column => ( + &content_box_sizes.block, + content_box_sizes.inline.clone(), + preferred_size_computes_to_auto.inline, + ), + }; + let cross_size_stretches_to_line = cross_size_computes_to_auto && + item_with_auto_cross_size_stretches_to_line_size(align_self, &margin); + if cross_size_stretches_to_line && config.container_is_single_line { + // + // > If a single-line flex container has a definite cross size, the automatic preferred + // > outer cross size of any stretched flex items is the flex container’s inner cross size. + // Therefore, set it to `stretch`, which has the desired behavior. + content_cross_sizes.preferred = Size::Stretch; + } let containing_block_size = flex_axis.vec2_to_flex_relative(containing_block.size); let stretch_size = FlexRelativeVec2 { main: containing_block_size @@ -2240,15 +2242,7 @@ impl FlexItemBox { // > outer cross size of any stretched flex items is the flex container’s inner cross size // > (clamped to the flex item’s min and max cross size) and is considered definite. let (preferred_cross_size, min_cross_size, max_cross_size) = content_cross_sizes - .resolve_each_extrinsic( - if auto_cross_size_stretches_to_container_size { - Size::Stretch - } else { - Size::FitContent - }, - Au::zero(), - stretch_size.cross, - ); + .resolve_each_extrinsic(Size::FitContent, Au::zero(), stretch_size.cross); let cross_size = SizeConstraint::new(preferred_cross_size, min_cross_size, max_cross_size); // @@ -2309,7 +2303,7 @@ impl FlexItemBox { &pbm_auto_is_zero, content_box_sizes, preferred_aspect_ratio, - auto_cross_size_stretches_to_container_size, + content_cross_sizes.preferred == Size::Stretch, IntrinsicSizingMode::Size, ) .into() @@ -2377,7 +2371,7 @@ impl FlexItemBox { FlexItem { box_: self, - content_cross_sizes: content_cross_sizes.clone(), + content_cross_sizes, padding, border, margin: config.sides_to_flex_relative(pbm.margin), @@ -2388,10 +2382,10 @@ impl FlexItemBox { .clamp_between_extremums(content_min_main_size, content_max_main_size), content_min_main_size, content_max_main_size, - align_self: AlignItems(config.resolve_align_self_for_child(style)), + align_self, depends_on_block_constraints: *depends_on_block_constraints, preferred_aspect_ratio, - auto_cross_size_stretches_to_container_size, + cross_size_stretches_to_line, } } @@ -2410,12 +2404,12 @@ impl FlexItemBox { // TODO: when laying out a column container with an indefinite main size, // we compute the base sizes of the items twice. We should consider caching. let FlexItem { + content_cross_sizes, flex_base_size, content_min_main_size, content_max_main_size, pbm_auto_is_zero, preferred_aspect_ratio, - auto_cross_size_stretches_to_container_size, .. } = self.to_flex_item( layout_context, @@ -2442,7 +2436,7 @@ impl FlexItemBox { layout_context, containing_block, &auto_minimum, - auto_cross_size_stretches_to_container_size, + content_cross_sizes.preferred == Size::Stretch, ); (sizes, depends_on_block_constraints) }, @@ -2452,7 +2446,7 @@ impl FlexItemBox { &pbm_auto_is_zero, &content_box_sizes_and_pbm.content_box_sizes, preferred_aspect_ratio, - auto_cross_size_stretches_to_container_size, + content_cross_sizes.preferred == Size::Stretch, IntrinsicSizingMode::Contribution, ); (size.into(), true) @@ -2616,7 +2610,7 @@ impl FlexItemBox { pbm_auto_is_zero: &FlexRelativeVec2, content_box_sizes: &LogicalVec2, preferred_aspect_ratio: Option, - auto_cross_size_stretches_to_container_size: bool, + cross_size_stretches_to_container_size: bool, intrinsic_sizing_mode: IntrinsicSizingMode, ) -> Au { let mut positioning_context = PositioningContext::new_for_style(self.style()) @@ -2657,7 +2651,7 @@ impl FlexItemBox { // TODO: This is wrong if the item writing mode is different from the flex // container's writing mode. let inline_size = { - let initial_behavior = if auto_cross_size_stretches_to_container_size { + let initial_behavior = if cross_size_stretches_to_container_size { Size::Stretch } else { Size::FitContent diff --git a/components/layout_2020/flow/mod.rs b/components/layout_2020/flow/mod.rs index da8f5c9f16a..fd3c1de800d 100644 --- a/components/layout_2020/flow/mod.rs +++ b/components/layout_2020/flow/mod.rs @@ -1215,6 +1215,7 @@ impl IndependentNonReplacedContents { content_box_sizes, pbm, depends_on_block_constraints, + .. } = self .layout_style(base) .content_box_sizes_and_padding_border_margin(&containing_block.into()); @@ -1696,6 +1697,7 @@ fn solve_containing_block_padding_and_border_for_in_flow_box<'a>( content_box_sizes, pbm, depends_on_block_constraints, + .. } = layout_style.content_box_sizes_and_padding_border_margin(&containing_block.into()); let pbm_sums = pbm.sums_auto_is_zero(ignore_block_margins_for_stretch); diff --git a/components/layout_2020/sizing.rs b/components/layout_2020/sizing.rs index 125a832fe6c..45595bc0871 100644 --- a/components/layout_2020/sizing.rs +++ b/components/layout_2020/sizing.rs @@ -130,6 +130,7 @@ pub(crate) fn outer_inline( content_box_sizes, pbm, mut depends_on_block_constraints, + preferred_size_computes_to_auto, } = layout_style.content_box_sizes_and_padding_border_margin(containing_block); let margin = pbm.margin.map(|v| v.auto_is(Au::zero)); let pbm_sums = LogicalVec2 { @@ -143,7 +144,7 @@ pub(crate) fn outer_inline( .size .block .map(|v| Au::zero().max(v - pbm_sums.block)); - let automatic_size = if content_box_sizes.block.preferred.is_initial() && + let automatic_size = if preferred_size_computes_to_auto.block && auto_block_size_stretches_to_containing_block { depends_on_block_constraints = true; diff --git a/components/layout_2020/style_ext.rs b/components/layout_2020/style_ext.rs index f3269fb5461..d734b7532d5 100644 --- a/components/layout_2020/style_ext.rs +++ b/components/layout_2020/style_ext.rs @@ -235,6 +235,7 @@ pub(crate) struct ContentBoxSizesAndPBM { pub content_box_sizes: LogicalVec2, pub pbm: PaddingBorderMargin, pub depends_on_block_constraints: bool, + pub preferred_size_computes_to_auto: LogicalVec2, } #[derive(Clone, Debug, PartialEq)] @@ -1000,6 +1001,8 @@ impl LayoutStyle<'_> { let box_size = style.box_size(writing_mode); let min_size = style.min_box_size(writing_mode); let max_size = style.max_box_size(writing_mode); + let preferred_size_computes_to_auto = box_size.map(|size| size.is_initial()); + let depends_on_block_constraints = |size: &Size| { match size { // fit-content is like clamp(min-content, stretch, max-content), but currently @@ -1014,7 +1017,6 @@ impl LayoutStyle<'_> { _ => false, } }; - let depends_on_block_constraints = depends_on_block_constraints(&box_size.block) || depends_on_block_constraints(&min_size.block) || depends_on_block_constraints(&max_size.block) || @@ -1045,6 +1047,7 @@ impl LayoutStyle<'_> { }, pbm, depends_on_block_constraints, + preferred_size_computes_to_auto, } } diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index d9c5da592b5..d4ae9d452f6 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -180356,6 +180356,19 @@ {} ] ], + "stretch-requires-computed-auto-size.html": [ + "01bc822af2a20636efe2d3a281ddaa41f5898906", + [ + null, + [ + [ + "/css/reference/ref-filled-green-100px-square.xht", + "==" + ] + ], + {} + ] + ], "stretched-child-in-nested-flexbox.html": [ "325d79e7c63f3e301c21f53f8bc97b48ae15c6b5", [ diff --git a/tests/wpt/meta/css/css-flexbox/quirks-auto-block-size-with-percentage-item.html.ini b/tests/wpt/meta/css/css-flexbox/quirks-auto-block-size-with-percentage-item.html.ini deleted file mode 100644 index 2db27f1d053..00000000000 --- a/tests/wpt/meta/css/css-flexbox/quirks-auto-block-size-with-percentage-item.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[quirks-auto-block-size-with-percentage-item.html] - [#container 1] - expected: FAIL diff --git a/tests/wpt/tests/css/css-flexbox/stretch-requires-computed-auto-size.html b/tests/wpt/tests/css/css-flexbox/stretch-requires-computed-auto-size.html new file mode 100644 index 00000000000..01bc822af2a --- /dev/null +++ b/tests/wpt/tests/css/css-flexbox/stretch-requires-computed-auto-size.html @@ -0,0 +1,17 @@ + +Stretch alignment with percentage that behaves as auto + + + + + + +

Test passes if there is a filled green square and no red.

+ +
+
+
+