layout: Fix block SizeConstraint for replaced elements (#37758)

#37433 didn't handle intrinsic contributions. This patch computes the
correct SizeConstraint to be used as the ConstraintSpace's block size
when computing intrinsic inline sizes.

Testing: Adding new test
Fixes: #37478

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

components/layout/flexbox/layout.rs

@@ -2214,12 +2214,30 @@ impl FlexItemBox {
                 .map(|v| Au::zero().max(v - pbm_auto_is_zero.cross)),
         };
 
-        // <https://drafts.csswg.org/css-flexbox/#definite-sizes>
+        let is_table = self.is_table();
-        // > If a single-line flex container has a definite cross size, the automatic preferred
+        let tentative_cross_content_size = if cross_axis_is_item_block_axis {
-        // > outer cross size of any stretched flex items is the flex container’s inner cross size
+            self.independent_formatting_context
-        // > (clamped to the flex item’s min and max cross size) and is considered definite.
+                .tentative_block_content_size(preferred_aspect_ratio)
-        let (preferred_cross_size, min_cross_size, max_cross_size) = content_cross_sizes
+        } else {
-            .resolve_each_extrinsic(Size::FitContent, Au::zero(), stretch_size.cross);
+            None
+        };
+        let (preferred_cross_size, min_cross_size, max_cross_size) =
+            if let Some(cross_content_size) = tentative_cross_content_size {
+                let (preferred, min, max) = content_cross_sizes.resolve_each(
+                    Size::FitContent,
+                    Au::zero,
+                    stretch_size.cross,
+                    || cross_content_size,
+                    is_table,
+                );
+                (Some(preferred), min, max)
+            } else {
+                content_cross_sizes.resolve_each_extrinsic(
+                    Size::FitContent,
+                    Au::zero(),
+                    stretch_size.cross,
+                )
+            };
         let cross_size = SizeConstraint::new(preferred_cross_size, min_cross_size, max_cross_size);
 
         // <https://drafts.csswg.org/css-flexbox/#transferred-size-suggestion>
@@ -2344,7 +2362,7 @@ impl FlexItemBox {
             get_automatic_minimum_size,
             stretch_size.main,
             &main_content_sizes,
-            self.is_table(),
+            is_table,
         );
 
         FlexItem {

components/layout/flow/mod.rs

@@ -534,6 +534,7 @@ fn compute_inline_content_sizes_for_block_level_boxes(
                     |constraint_space| {
                         base.inline_content_sizes(layout_context, constraint_space, contents)
                     },
+                    |_aspect_ratio| None,
                 );
                 // A block in the same BFC can overlap floats, it's not moved next to them,
                 // so we shouldn't add its size to the size of the floats.

components/layout/formatting_contexts.rs

@@ -7,6 +7,7 @@ use malloc_size_of_derive::MallocSizeOf;
 use script::layout_dom::{ServoLayoutElement, ServoLayoutNode};
 use servo_arc::Arc;
 use style::context::SharedStyleContext;
+use style::logical_geometry::Direction;
 use style::properties::ComputedValues;
 use style::selector_parser::PseudoElement;
 
@@ -21,7 +22,7 @@ use crate::layout_box_base::{
 };
 use crate::positioned::PositioningContext;
 use crate::replaced::ReplacedContents;
-use crate::sizing::{self, ComputeInlineContentSizes, InlineContentSizesResult};
+use crate::sizing::{self, ComputeInlineContentSizes, ContentSizes, InlineContentSizesResult};
 use crate::style_ext::{AspectRatio, DisplayInside, LayoutStyle};
 use crate::table::Table;
 use crate::taffy::TaffyContainer;
@@ -175,6 +176,35 @@ impl IndependentFormattingContext {
         }
     }
 
+    /// Computes the tentative intrinsic block sizes that may be needed while computing
+    /// the intrinsic inline sizes. Therefore, this ignores the values of the sizing
+    /// properties in both axes.
+    /// A return value of `None` indicates that there is no suitable tentative intrinsic
+    /// block size, so intrinsic keywords in the block sizing properties will be ignored,
+    /// possibly resulting in an indefinite [`SizeConstraint`] for computing the intrinsic
+    /// inline sizes and laying out the contents.
+    /// A return value of `Some` indicates that intrinsic keywords in the block sizing
+    /// properties will be resolved as the contained value, guaranteeing a definite amount
+    /// for computing the intrinsic inline sizes and laying out the contents.
+    pub(crate) fn tentative_block_content_size(
+        &self,
+        preferred_aspect_ratio: Option<AspectRatio>,
+    ) -> Option<ContentSizes> {
+        // See <https://github.com/w3c/csswg-drafts/issues/12333> regarding the difference
+        // in behavior for the replaced and non-replaced cases.
+        match &self.contents {
+            IndependentFormattingContextContents::NonReplaced(_) => None,
+            IndependentFormattingContextContents::Replaced(contents) => {
+                // For replaced elements with no ratio, the returned value doesn't matter.
+                let ratio = preferred_aspect_ratio?;
+                let writing_mode = self.style().writing_mode;
+                let inline_size = contents.fallback_inline_size(writing_mode);
+                let block_size = ratio.compute_dependent_size(Direction::Block, inline_size);
+                Some(block_size.into())
+            },
+        }
+    }
+
     pub(crate) fn outer_inline_content_sizes(
         &self,
         layout_context: &LayoutContext,
@@ -191,6 +221,7 @@ impl IndependentFormattingContext {
             true, /* establishes_containing_block */
             |padding_border_sums| self.preferred_aspect_ratio(padding_border_sums),
             |constraint_space| self.inline_content_sizes(layout_context, constraint_space),
+            |preferred_aspect_ratio| self.tentative_block_content_size(preferred_aspect_ratio),
         )
     }
 

components/layout/geom.rs

@@ -1031,29 +1031,50 @@ impl Sizes {
         get_content_size: impl FnOnce() -> ContentSizes,
         is_table: bool,
     ) -> Au {
-        // The provided `get_content_size` is a FnOnce but we may need its result multiple times.
-        // A LazyCell will only invoke it once if needed, and then reuse the result.
-        let content_size = LazyCell::new(get_content_size);
-
         if is_table && axis == Direction::Block {
             // The intrinsic block size of a table already takes sizing properties into account,
             // but it can be a smaller amount if there are collapsed rows.
             // Therefore, disregard sizing properties and just defer to the intrinsic size.
             // This is being discussed in https://github.com/w3c/csswg-drafts/issues/11408
-            return content_size.max_content;
+            return get_content_size().max_content;
+        }
+        let (preferred, min, max) = self.resolve_each(
+            automatic_size,
+            get_automatic_minimum_size,
+            stretch_size,
+            get_content_size,
+            is_table,
+        );
+        preferred.clamp_between_extremums(min, max)
     }
 
-        let preferred =
+    /// Resolves each of the three sizes into a numerical value, separately.
+    /// - The 1st returned value is the resolved preferred size.
+    /// - The 2nd returned value is the resolved minimum size.
+    /// - The 3rd returned value is the resolved maximum size. `None` means no maximum.
+    #[inline]
+    pub(crate) fn resolve_each(
+        &self,
+        automatic_size: Size<Au>,
+        get_automatic_minimum_size: impl FnOnce() -> Au,
+        stretch_size: Option<Au>,
+        get_content_size: impl FnOnce() -> ContentSizes,
+        is_table: bool,
+    ) -> (Au, Au, Option<Au>) {
+        // The provided `get_content_size` is a FnOnce but we may need its result multiple times.
+        // A LazyCell will only invoke it once if needed, and then reuse the result.
+        let content_size = LazyCell::new(get_content_size);
+        (
             self.preferred
-                .resolve_for_preferred(automatic_size, stretch_size, &content_size);
+                .resolve_for_preferred(automatic_size, stretch_size, &content_size),
-        let min = self.min.resolve_for_min(
+            self.min.resolve_for_min(
                 get_automatic_minimum_size,
                 stretch_size,
                 &content_size,
                 is_table,
-        );
+            ),
-        let max = self.max.resolve_for_max(stretch_size, &content_size);
+            self.max.resolve_for_max(stretch_size, &content_size),
-        preferred.clamp_between_extremums(min, max)
+        )
     }
 
     /// Tries to extrinsically resolve the three sizes into a single [`SizeConstraint`].

components/layout/sizing.rs

@@ -10,10 +10,11 @@ use std::ops::{Add, AddAssign};
 use app_units::Au;
 use malloc_size_of_derive::MallocSizeOf;
 use style::Zero;
+use style::logical_geometry::Direction;
 use style::values::computed::LengthPercentage;
 
 use crate::context::LayoutContext;
-use crate::geom::Size;
+use crate::geom::{Size, SizeConstraint};
 use crate::style_ext::{AspectRatio, Clamp, ComputedValuesExt, ContentBoxSizesAndPBM, LayoutStyle};
 use crate::{ConstraintSpace, IndefiniteContainingBlock, LogicalVec2};
 
@@ -125,7 +126,8 @@ pub(crate) fn outer_inline(
     is_replaced: bool,
     establishes_containing_block: bool,
     get_preferred_aspect_ratio: impl FnOnce(&LogicalVec2<Au>) -> Option<AspectRatio>,
-    get_content_size: impl FnOnce(&ConstraintSpace) -> InlineContentSizesResult,
+    get_inline_content_size: impl FnOnce(&ConstraintSpace) -> InlineContentSizesResult,
+    get_tentative_block_content_size: impl FnOnce(Option<AspectRatio>) -> Option<ContentSizes>,
 ) -> InlineContentSizesResult {
     let ContentBoxSizesAndPBM {
         content_box_sizes,
@@ -139,6 +141,7 @@ pub(crate) fn outer_inline(
         inline: pbm.padding_border_sums.inline + margin.inline_sum(),
     };
     let style = layout_style.style();
+    let is_table = layout_style.is_table();
     let content_size = LazyCell::new(|| {
         let constraint_space = if establishes_containing_block {
             let available_block_size = containing_block
@@ -153,15 +156,25 @@ pub(crate) fn outer_inline(
             } else {
                 Size::FitContent
             };
-            ConstraintSpace::new(
+            let aspect_ratio = get_preferred_aspect_ratio(&pbm.padding_border_sums);
+            let block_size =
+                if let Some(block_content_size) = get_tentative_block_content_size(aspect_ratio) {
+                    SizeConstraint::Definite(content_box_sizes.block.resolve(
+                        Direction::Block,
+                        automatic_size,
+                        || auto_minimum.block,
+                        available_block_size,
+                        || block_content_size,
+                        is_table,
+                    ))
+                } else {
                     content_box_sizes.block.resolve_extrinsic(
                         automatic_size,
                         auto_minimum.block,
                         available_block_size,
-                ),
-                style.writing_mode,
-                get_preferred_aspect_ratio(&pbm.padding_border_sums),
                     )
+                };
+            ConstraintSpace::new(block_size, style.writing_mode, aspect_ratio)
         } else {
             // This assumes that there is no preferred aspect ratio, or that there is no
             // block size constraint to be transferred so the ratio is irrelevant.
@@ -172,7 +185,7 @@ pub(crate) fn outer_inline(
                 None,
             )
         };
-        get_content_size(&constraint_space)
+        get_inline_content_size(&constraint_space)
     });
     let resolve_non_initial = |inline_size, stretch_values| {
         Some(match inline_size {
@@ -246,7 +259,7 @@ pub(crate) fn outer_inline(
 
     // Regardless of their sizing properties, tables are always forced to be at least
     // as big as their min-content size, so floor the minimums.
-    if layout_style.is_table() {
+    if is_table {
         min_min_content.max_assign(content_size.sizes.min_content);
         min_max_content.max_assign(content_size.sizes.min_content);
         min_depends_on_block_constraints |= content_size.depends_on_block_constraints;

tests/wpt/meta/MANIFEST.json

@@ -610358,6 +610358,13 @@
        {}
       ]
      ],
+     "keyword-sizes-for-intrinsic-contributions-002.html": [
+      "a053d88c1eefd0379ff7e612dfd3d1c39e7c581e",
+      [
+       null,
+       {}
+      ]
+     ],
      "keyword-sizes-for-intrinsic-contributions.html": [
       "b42bd71b8e7ccc802ba11e3bf3384b3854a0c3fa",
       [

tests/wpt/tests/css/css-sizing/keyword-sizes-for-intrinsic-contributions-002.html

@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+<title>Keyword sizes for intrinsic contributions</title>
+<link rel="author" title="Oriol Brufau" href="obrufau@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css-sizing-3/#sizing-values">
+<link rel="help" href="https://drafts.csswg.org/css-sizing-3/#intrinsic-contribution">
+<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/12333">
+<link rel="help" href="https://github.com/servo/servo/issues/37478">
+<meta assert="Intrinsic keywords for min/max block sizes on a replaced element affect the inline min/max-content contributions.">
+
+<style>
+.test { width: max-content; background: red; border: 5px solid; margin: 5px; }
+.test.flex-row { display: flex; flex-direction: row; }
+.test.flex-col { display: flex; flex-direction: column; }
+canvas { display: block; background: green; height: 0px; width: max-content; }
+</style>
+<div id="log"></div>
+
+<div class="test" data-expected-width="60" data-expected-height="60">
+  <canvas width="50" height="50" style="height: 0px; min-height: max-content">
+</div>
+<div class="test" data-expected-width="60" data-expected-height="60">
+  <canvas width="50" height="50" style="height: 100px; max-height: max-content"></canvas>
+</div>
+
+<div class="test flex-row" data-expected-width="60" data-expected-height="60">
+  <canvas width="50" height="50" style="height: 0px; min-height: max-content">
+</div>
+<div class="test flex-row" data-expected-width="60" data-expected-height="60">
+  <canvas width="50" height="50" style="height: 100px; max-height: max-content"></canvas>
+</div>
+
+<div class="test flex-col" data-expected-width="60" data-expected-height="60">
+  <canvas width="50" height="50" style="height: 0px; min-height: max-content">
+</div>
+<div class="test flex-col" data-expected-width="60" data-expected-height="60">
+  <canvas width="50" height="50" style="height: 100px; max-height: max-content"></canvas>
+</div>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/check-layout-th.js"></script>
+<script>
+checkLayout(".test");
+</script>