layout: Obey sizing keywords in layout_for_block_content_size() (#36015)

We were ignoring sizing keywords on the min and max sizing properties.
With this, flexbox layout has full support for sizing keywords.

Signed-off-by: Oriol Brufau <obrufau@igalia.com>

components/layout_2020/flexbox/layout.rs

@@ -41,9 +41,7 @@ use crate::positioned::{
 use crate::sizing::{
     ComputeInlineContentSizes, ContentSizes, InlineContentSizesResult, IntrinsicSizingMode,
 };
-use crate::style_ext::{
+use crate::style_ext::{AspectRatio, Clamp, ComputedValuesExt, ContentBoxSizesAndPBM, LayoutStyle};
-    AspectRatio, Clamp, ComputedValuesExt, ContentBoxSizesAndPBM, LayoutStyle, PaddingBorderMargin,
-};
 use crate::{
     ConstraintSpace, ContainingBlock, ContainingBlockSize, IndefiniteContainingBlock,
     SizeConstraint,
@@ -2314,11 +2312,8 @@ impl FlexItemBox {
             } else {
                 self.layout_for_block_content_size(
                     flex_context_getter(),
-                    pbm,
+                    &pbm_auto_is_zero,
-                    content_box_sizes.map(|size| size.preferred),
+                    content_box_sizes,
-                    // TODO(#32853): handle size keywords.
-                    content_box_sizes.map(|size| size.min.to_numeric().unwrap_or_default()),
-                    content_box_sizes.map(|size| size.max.to_numeric()),
                     preferred_aspect_ratio,
                     auto_cross_size_stretches_to_container_size,
                     IntrinsicSizingMode::Size,
@@ -2458,28 +2453,10 @@ impl FlexItemBox {
                 (sizes, depends_on_block_constraints)
             },
             FlexAxis::Column => {
-                let content_box_sizes = &content_box_sizes_and_pbm.content_box_sizes;
-                let content_box_size = content_box_sizes.map(|size| size.preferred);
-                let content_min_size_no_auto = LogicalVec2 {
-                    // TODO(#32853): handle size keywords.
-                    inline: content_box_sizes
-                        .inline
-                        .min
-                        .to_numeric()
-                        .unwrap_or_default(),
-                    block: content_min_main_size,
-                };
-                let content_max_box_size = LogicalVec2 {
-                    // TODO(#32853): handle size keywords.
-                    inline: content_box_sizes.inline.max.to_numeric(),
-                    block: content_max_main_size,
-                };
                 let size = self.layout_for_block_content_size(
                     flex_context_getter(),
-                    &content_box_sizes_and_pbm.pbm,
+                    &pbm_auto_is_zero,
-                    content_box_size,
+                    &content_box_sizes_and_pbm.content_box_sizes,
-                    content_min_size_no_auto,
-                    content_max_box_size,
                     preferred_aspect_ratio,
                     auto_cross_size_stretches_to_container_size,
                     IntrinsicSizingMode::Contribution,
@@ -2645,10 +2622,8 @@ impl FlexItemBox {
     fn layout_for_block_content_size(
         &self,
         flex_context: &FlexContext,
-        padding_border_margin: &PaddingBorderMargin,
+        pbm_auto_is_zero: &FlexRelativeVec2<Au>,
-        mut content_box_size: LogicalVec2<Size<Au>>,
+        content_box_sizes: &LogicalVec2<Sizes>,
-        mut min_size: LogicalVec2<Au>,
-        mut max_size: LogicalVec2<Option<Au>>,
         preferred_aspect_ratio: Option<AspectRatio>,
         auto_cross_size_stretches_to_container_size: bool,
         intrinsic_sizing_mode: IntrinsicSizingMode,
@@ -2662,34 +2637,27 @@ impl FlexItemBox {
         let style = self.independent_formatting_context.style();
         match &self.independent_formatting_context.contents {
             IndependentFormattingContextContents::Replaced(replaced) => {
-                content_box_size.inline = content_box_size.inline.map(|v| v.max(Au::zero()));
+                let get_used_size = |block_sizes| {
-                if intrinsic_sizing_mode == IntrinsicSizingMode::Size {
+                    replaced.used_size_as_if_inline_element_from_content_box_sizes(
-                    content_box_size.block = Size::Initial;
-                    min_size.block = Au::zero();
-                    max_size.block = None;
-                }
-                replaced
-                    .used_size_as_if_inline_element_from_content_box_sizes(
                         flex_context.containing_block,
                         style,
                         preferred_aspect_ratio,
                         LogicalVec2 {
-                            block: &Sizes::new(
+                            block: block_sizes,
-                                content_box_size.block,
+                            inline: &content_box_sizes.inline,
-                                Size::Numeric(min_size.block),
-                                max_size.block.map_or(Size::Initial, Size::Numeric),
-                            ),
-                            inline: &Sizes::new(
-                                content_box_size.inline,
-                                Size::Numeric(min_size.inline),
-                                max_size.inline.map_or(Size::Initial, Size::Numeric),
-                            ),
                         },
                         Size::FitContent.into(),
-                        padding_border_margin.padding_border_sums +
+                        LogicalVec2 {
-                            padding_border_margin.margin.auto_is(Au::zero).sum(),
+                            inline: pbm_auto_is_zero.cross,
+                            block: pbm_auto_is_zero.main,
+                        },
                     )
-                    .block
+                };
+                if intrinsic_sizing_mode == IntrinsicSizingMode::Size {
+                    get_used_size(&Sizes::default()).block
+                } else {
+                    get_used_size(&content_box_sizes.block).block
+                }
             },
             IndependentFormattingContextContents::NonReplaced(non_replaced) => {
                 // TODO: This is wrong if the item writing mode is different from the flex
@@ -2700,11 +2668,9 @@ impl FlexItemBox {
                     } else {
                         Size::FitContent
                     };
-                    let stretch_size = flex_context.containing_block.size.inline -
+                    let stretch_size =
-                        padding_border_margin.padding_border_sums.inline -
+                        flex_context.containing_block.size.inline - pbm_auto_is_zero.cross;
-                        padding_border_margin.margin.inline_start.auto_is(Au::zero) -
+                    let get_content_size = || {
-                        padding_border_margin.margin.inline_end.auto_is(Au::zero);
-                    let content_size = LazyCell::new(|| {
                         let constraint_space = ConstraintSpace::new(
                             SizeConstraint::default(),
                             style.writing_mode,
@@ -2713,11 +2679,15 @@ impl FlexItemBox {
                         self.independent_formatting_context
                             .inline_content_sizes(flex_context.layout_context, &constraint_space)
                             .sizes
-                    });
+                    };
-                    content_box_size
+                    content_box_sizes.inline.resolve(
-                        .inline
+                        Direction::Inline,
-                        .resolve_for_preferred(initial_behavior, Some(stretch_size), &content_size)
+                        initial_behavior,
-                        .clamp_between_extremums(min_size.inline, max_size.inline)
+                        Au::zero,
+                        Some(stretch_size),
+                        get_content_size,
+                        false,
+                    )
                 };
                 let item_as_containing_block = ContainingBlock {
                     size: ContainingBlockSize {
@@ -2760,27 +2730,27 @@ impl FlexItemBox {
                 };
                 match intrinsic_sizing_mode {
                     IntrinsicSizingMode::Contribution => {
-                        let stretch_size =
+                        let stretch_size = flex_context
-                            flex_context.containing_block.size.block.to_definite().map(
+                            .containing_block
-                                |block_size| {
+                            .size
-                                    block_size -
-                                        padding_border_margin.padding_border_sums.block -
-                                        padding_border_margin.margin.block_start.auto_is(Au::zero) -
-                                        padding_border_margin.margin.block_end.auto_is(Au::zero)
-                                },
-                            );
-                        let inner_block_size = content_box_size
                             .block
-                            .resolve_for_preferred(
+                            .to_definite()
+                            .map(|block_size| block_size - pbm_auto_is_zero.main);
+                        let inner_block_size = content_box_sizes.block.resolve(
+                            Direction::Block,
                             Size::FitContent,
+                            Au::zero,
                             stretch_size,
-                                &LazyCell::new(|| ContentSizes::from(content_block_size())),
+                            || ContentSizes::from(content_block_size()),
-                            )
+                            // Tables have a special sizing in the block axis that handles collapsed rows
-                            .clamp_between_extremums(min_size.block, max_size.block);
+                            // by ignoring the sizing properties and instead relying on the content block size,
-                        inner_block_size +
+                            // which should indirectly take sizing properties into account.
-                            padding_border_margin.padding_border_sums.block +
+                            // However, above we laid out the table with a SizeConstraint::default() block size,
-                            padding_border_margin.margin.block_start.auto_is(Au::zero) +
+                            // so the content block size doesn't take sizing properties into account.
-                            padding_border_margin.margin.block_end.auto_is(Au::zero)
+                            // Therefore, pretending that it's never a table tends to provide a better result.
+                            false, /* is_table */
+                        );
+                        inner_block_size + pbm_auto_is_zero.main
                     },
                     IntrinsicSizingMode::Size => content_block_size(),
                 }

tests/wpt/meta/MANIFEST.json

@@ -171064,6 +171064,19 @@
        {}
       ]
      ],
+     "flex-item-max-width-min-content-002.html": [
+      "7606f8ec5b108b275409ea85edce42c1348b02f3",
+      [
+       null,
+       [
+        [
+         "/css/reference/ref-filled-green-100px-square-only.html",
+         "=="
+        ]
+       ],
+       {}
+      ]
+     ],
      "flex-item-max-width-min-content.html": [
       "dc5ce0523a6a12e50aaf23f26c3529d23040bee7",
       [

tests/wpt/tests/css/css-flexbox/flex-item-max-width-min-content-002.html

@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<title>CSS Test: flex item with `max-width: min-content`</title>
+<link rel="author" title="Oriol Brufau" href="obrufau@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#flex-base-size">
+<link rel="help" href="https://drafts.csswg.org/css-sizing-3/#min-content">
+
+<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
+<meta name="assert" content="`max-width: min-content` limits the flex item to 100px wide,
+  so the floats need to stack vertically and thus the flex base size is 100px.">
+
+<p>Test passes if there is a filled green square.</p>
+<div style="display: flex; flex-direction: column; width: 200px; height: 100px">
+  <div style="max-width: min-content; background: green">
+    <div style="float: left; width: 100px; height: 50px"></div>
+    <div style="float: left; width: 100px; height: 50px"></div>
+  </div>
+</div>