LibWeb: Stop treating intrinsic size keywords as auto in CSS heights

This commit introduces proper handling of three intrinsic size keywords
when used for CSS heights:

- min-content
- max-content
- fit-content

This necessitated a few plumbing changes, since we can't resolve these
values without having access to containing block widths.

This fixes some visual glitches on https://www.supabase.com/ as well
as a number of WPT tests. It also improves the appearance of dialogs.

Libraries/LibWeb/Layout/BlockFormattingContext.cpp

@@ -416,16 +416,16 @@ void BlockFormattingContext::resolve_used_height_if_not_treated_as_auto(Box cons
     auto const& computed_values = box.computed_values();
     auto& box_state = m_state.get_mutable(box);
 
-    auto height = calculate_inner_height(box, available_space.height, box.computed_values().height());
+    auto height = calculate_inner_height(box, available_space, box.computed_values().height());
 
     if (!should_treat_max_height_as_none(box, available_space.height)) {
         if (!computed_values.max_height().is_auto()) {
-            auto max_height = calculate_inner_height(box, available_space.height, computed_values.max_height());
+            auto max_height = calculate_inner_height(box, available_space, computed_values.max_height());
             height = min(height, max_height);
         }
     }
     if (!computed_values.min_height().is_auto()) {
-        height = max(height, calculate_inner_height(box, available_space.height, computed_values.min_height()));
+        height = max(height, calculate_inner_height(box, available_space, computed_values.min_height()));
     }
 
     box_state.set_content_height(height);
@@ -454,12 +454,12 @@ void BlockFormattingContext::resolve_used_height_if_treated_as_auto(Box const& b
 
     if (!should_treat_max_height_as_none(box, available_space.height)) {
         if (!computed_values.max_height().is_auto()) {
-            auto max_height = calculate_inner_height(box, available_space.height, computed_values.max_height());
+            auto max_height = calculate_inner_height(box, available_space, computed_values.max_height());
             height = min(height, max_height);
         }
     }
     if (!computed_values.min_height().is_auto()) {
-        height = max(height, calculate_inner_height(box, available_space.height, computed_values.min_height()));
+        height = max(height, calculate_inner_height(box, available_space, computed_values.min_height()));
     }
 
     if (box.document().in_quirks_mode()

Libraries/LibWeb/Layout/FlexFormattingContext.cpp

@@ -25,7 +25,7 @@ CSSPixels FlexFormattingContext::get_pixel_width(Box const& box, CSS::Size const
 
 CSSPixels FlexFormattingContext::get_pixel_height(Box const& box, CSS::Size const& size) const
 {
-    return calculate_inner_height(box, m_available_space->height, size);
+    return calculate_inner_height(box, m_available_space.value(), size);
 }
 
 FlexFormattingContext::FlexFormattingContext(LayoutState& state, LayoutMode layout_mode, Box const& flex_container, FormattingContext* parent)

Libraries/LibWeb/Layout/FormattingContext.cpp

@@ -624,7 +624,7 @@ CSSPixels FormattingContext::tentative_height_for_replaced_element(Box const& bo
         return 150;
 
     // FIXME: Handle cases when available_space is not definite.
-    return calculate_inner_height(box, available_space.height, computed_height);
+    return calculate_inner_height(box, available_space, computed_height);
 }
 
 CSSPixels FormattingContext::compute_height_for_replaced_element(Box const& box, AvailableSpace const& available_space) const
@@ -954,12 +954,12 @@ void FormattingContext::compute_height_for_absolutely_positioned_non_replaced_el
         auto const& computed_max_height = box.computed_values().max_height();
         auto constrained_height = unconstrained_height;
         if (!computed_max_height.is_none()) {
-            auto inner_max_height = calculate_inner_height(box, available_space.height, computed_max_height);
+            auto inner_max_height = calculate_inner_height(box, available_space, computed_max_height);
             if (inner_max_height < constrained_height.to_px(box))
                 constrained_height = CSS::Length::make_px(inner_max_height);
         }
         if (!computed_min_height.is_auto()) {
-            auto inner_min_height = calculate_inner_height(box, available_space.height, computed_min_height);
+            auto inner_min_height = calculate_inner_height(box, available_space, computed_min_height);
             if (inner_min_height > constrained_height.to_px(box))
                 constrained_height = CSS::Length::make_px(inner_min_height);
         }
@@ -1148,7 +1148,7 @@ void FormattingContext::compute_height_for_absolutely_positioned_non_replaced_el
             return CSS::Length::make_px(compute_table_box_height_inside_table_wrapper(box, available_space));
         if (should_treat_height_as_auto(box, available_space))
             return CSS::Length::make_auto();
-        return CSS::Length::make_px(calculate_inner_height(box, available_space.height, box.computed_values().height()));
+        return CSS::Length::make_px(calculate_inner_height(box, available_space, box.computed_values().height()));
     }());
 
     used_height = apply_min_max_height_constraints(used_height);
@@ -1614,11 +1614,23 @@ CSSPixels FormattingContext::calculate_inner_width(Layout::Box const& box, Avail
     return width.to_px(box, width_of_containing_block);
 }
 
-CSSPixels FormattingContext::calculate_inner_height(Layout::Box const& box, AvailableSize const& available_height, CSS::Size const& height) const
+CSSPixels FormattingContext::calculate_inner_height(Layout::Box const& box, AvailableSpace const& available_space, CSS::Size const& height) const
 {
     VERIFY(!height.is_auto());
-    auto height_of_containing_block = available_height.to_px_or_zero();
+
+    if (height.is_fit_content()) {
+        return calculate_fit_content_height(box, available_space);
+    }
+    if (height.is_max_content()) {
+        return calculate_max_content_height(box, available_space.width.to_px_or_zero());
+    }
+    if (height.is_min_content()) {
+        return calculate_min_content_height(box, available_space.width.to_px_or_zero());
+    }
+
+    auto height_of_containing_block = available_space.height.to_px_or_zero();
     auto& computed_values = box.computed_values();
+
     if (computed_values.box_sizing() == CSS::BoxSizing::BorderBox) {
         auto const& state = m_state.get(box);
         auto inner_height = height.to_px(box, height_of_containing_block)
@@ -1682,7 +1694,7 @@ CSSPixels FormattingContext::calculate_stretch_fit_height(Box const& box, Availa
         - box_state.border_bottom;
 }
 
-bool FormattingContext::should_treat_width_as_auto(Box const& box, AvailableSpace const& available_space)
+bool FormattingContext::should_treat_width_as_auto(Box const& box, AvailableSpace const& available_space) const
 {
     auto const& computed_width = box.computed_values().width();
     if (computed_width.is_auto())
@@ -1693,35 +1705,42 @@ bool FormattingContext::should_treat_width_as_auto(Box const& box, AvailableSpac
         if (available_space.width.is_indefinite())
             return true;
     }
-    // AD-HOC: If the box has a preferred aspect ratio and no natural height,
+    // AD-HOC: If the box has a preferred aspect ratio and an intrinsic keyword for width...
-    //         we treat the width as auto, since it can't be resolved through the ratio.
+    if (box.has_preferred_aspect_ratio()
-    if (computed_width.is_min_content() || computed_width.is_max_content() || computed_width.is_fit_content()) {
+        && (computed_width.is_min_content() || computed_width.is_max_content() || computed_width.is_fit_content())) {
-        if (box.has_preferred_aspect_ratio() && !box.has_natural_height())
+        // If the box has no natural height to resolve the aspect ratio, we treat the width as auto.
+        if (!box.has_natural_height())
+            return true;
+        // If the box has definite height, we can resolve the width through the aspect ratio.
+        if (m_state.get(box).has_definite_height())
             return true;
     }
     return false;
 }
 
-bool FormattingContext::should_treat_height_as_auto(Box const& box, AvailableSpace const& available_space)
+bool FormattingContext::should_treat_height_as_auto(Box const& box, AvailableSpace const& available_space) const
 {
     auto computed_height = box.computed_values().height();
     if (computed_height.is_auto())
         return true;
 
-    // https://www.w3.org/TR/css-sizing-3/#valdef-width-min-content
+    if (computed_height.contains_percentage()) {
-    // https://www.w3.org/TR/css-sizing-3/#valdef-width-max-content
-    // https://www.w3.org/TR/css-sizing-3/#valdef-width-fit-content
-    // For a box’s block size, unless otherwise specified, this is equivalent to its automatic size.
-    // FIXME: If height is not the block axis size, then we should be concerned with the width instead.
-    if (computed_height.is_min_content() || computed_height.is_max_content() || computed_height.is_fit_content())
-        return true;
-
-    if (box.computed_values().height().contains_percentage()) {
         if (available_space.height.is_max_content())
             return true;
         if (available_space.height.is_indefinite())
             return true;
     }
+
+    // AD-HOC: If the box has a preferred aspect ratio and an intrinsic keyword for height...
+    if (box.has_preferred_aspect_ratio()
+        && (computed_height.is_min_content() || computed_height.is_max_content() || computed_height.is_fit_content())) {
+        // If the box has no natural width to resolve the aspect ratio, we treat the height as auto.
+        if (!box.has_natural_width())
+            return true;
+        // If the box has definite width, we can resolve the height through the aspect ratio.
+        if (m_state.get(box).has_definite_width())
+            return true;
+    }
     return false;
 }
 

Libraries/LibWeb/Layout/FormattingContext.h

@@ -76,7 +76,7 @@ public:
     CSSPixels calculate_fit_content_width(Layout::Box const&, AvailableSpace const&) const;
 
     CSSPixels calculate_inner_width(Layout::Box const&, AvailableSize const&, CSS::Size const& width) const;
-    CSSPixels calculate_inner_height(Layout::Box const&, AvailableSize const&, CSS::Size const& height) const;
+    [[nodiscard]] CSSPixels calculate_inner_height(Layout::Box const&, AvailableSpace const&, CSS::Size const& height) const;
 
     virtual CSSPixels greatest_child_width(Box const&) const;
 
@@ -102,8 +102,8 @@ public:
 protected:
     FormattingContext(Type, LayoutMode, LayoutState&, Box const&, FormattingContext* parent = nullptr);
 
-    static bool should_treat_width_as_auto(Box const&, AvailableSpace const&);
+    [[nodiscard]] bool should_treat_width_as_auto(Box const&, AvailableSpace const&) const;
-    static bool should_treat_height_as_auto(Box const&, AvailableSpace const&);
+    [[nodiscard]] bool should_treat_height_as_auto(Box const&, AvailableSpace const&) const;
 
     [[nodiscard]] bool should_treat_max_width_as_none(Box const&, AvailableSize const&) const;
     [[nodiscard]] bool should_treat_max_height_as_none(Box const&, AvailableSize const&) const;

Libraries/LibWeb/Layout/GridFormattingContext.cpp

@@ -2032,7 +2032,7 @@ void GridFormattingContext::run(AvailableSpace const& available_space)
 
     CSSPixels min_height = 0;
     if (!grid_computed_values.min_height().is_auto())
-        min_height = calculate_inner_height(grid_container(), available_space.height, grid_computed_values.min_height());
+        min_height = calculate_inner_height(grid_container(), available_space, grid_computed_values.min_height());
 
     // If automatic grid container height is less than min-height, we need to re-run the track sizing algorithm
     if (m_automatic_content_height < min_height) {
@@ -2355,7 +2355,7 @@ CSSPixels GridFormattingContext::calculate_grid_container_maximum_size(GridDimen
     auto const& computed_values = grid_container().computed_values();
     if (dimension == GridDimension::Column)
         return calculate_inner_width(grid_container(), m_available_space->width, computed_values.max_width());
-    return calculate_inner_height(grid_container(), m_available_space->height, computed_values.max_height());
+    return calculate_inner_height(grid_container(), m_available_space.value(), computed_values.max_height());
 }
 
 CSS::Size const& GridFormattingContext::get_item_preferred_size(GridItem const& item, GridDimension const dimension) const

Libraries/LibWeb/Layout/InlineFormattingContext.cpp

@@ -185,13 +185,11 @@ void InlineFormattingContext::dimension_box_on_line(Box const& box, LayoutMode l
 
     auto independent_formatting_context = layout_inside(box, layout_mode, box_state.available_inner_space_or_constraints_from(*m_available_space));
 
-    auto const& height_value = box.computed_values().height();
     if (should_treat_height_as_auto(box, *m_available_space)) {
         // FIXME: (10.6.6) If 'height' is 'auto', the height depends on the element's descendants per 10.6.7.
-        parent().resolve_used_height_if_treated_as_auto(box, AvailableSpace(AvailableSize::make_indefinite(), AvailableSize::make_indefinite()));
+        parent().resolve_used_height_if_treated_as_auto(box, *m_available_space);
     } else {
-        auto inner_height = calculate_inner_height(box, AvailableSize::make_definite(m_containing_block_used_values.content_height()), height_value);
+        parent().resolve_used_height_if_not_treated_as_auto(box, *m_available_space);
-        box_state.set_content_height(inner_height);
     }
 
     if (independent_formatting_context)

Tests/LibWeb/Layout/expected/dialog-open-modal.txt

@@ -2,11 +2,11 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
   BlockContainer <html> at (0,0) content-size 800x600 [BFC] children: not-inline
     BlockContainer <body> at (8,8) content-size 784x0 children: inline
       TextNode <#text>
-  Box <dialog#modal> at (358.84375,19) content-size 82.3125x562 positioned flex-container(row) [FFC] children: not-inline
+  Box <dialog#modal> at (358.84375,291.5) content-size 82.3125x17 positioned flex-container(row) [FFC] children: not-inline
     BlockContainer <(anonymous)> (not painted) [BFC] children: inline
       TextNode <#text>
-    BlockContainer <span> at (358.84375,19) content-size 82.3125x562 flex-item [BFC] children: inline
+    BlockContainer <span> at (358.84375,291.5) content-size 82.3125x17 flex-item [BFC] children: inline
-      frag 0 from TextNode start: 0, length: 10, rect: [358.84375,19 82.3125x17] baseline: 13.296875
+      frag 0 from TextNode start: 0, length: 10, rect: [358.84375,291.5 82.3125x17] baseline: 13.296875
           "I'm a node"
       TextNode <#text>
     BlockContainer <(anonymous)> (not painted) [BFC] children: inline
@@ -15,6 +15,6 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
 ViewportPaintable (Viewport<#document>) [0,0 800x600]
   PaintableWithLines (BlockContainer<HTML>) [0,0 800x600]
     PaintableWithLines (BlockContainer<BODY>) [8,8 784x0]
-  PaintableBox (Box<DIALOG>#modal) [339.84375,0 120.3125x600]
+  PaintableBox (Box<DIALOG>#modal) [339.84375,272.5 120.3125x55]
-    PaintableWithLines (BlockContainer<SPAN>) [358.84375,19 82.3125x562]
+    PaintableWithLines (BlockContainer<SPAN>) [358.84375,291.5 82.3125x17]
       TextPaintable (TextNode<#text>)

Tests/LibWeb/Layout/expected/resolve-cyclic-percentage-against-zero-when-available-size-is-min-content.txt

@@ -12,13 +12,13 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
       BlockContainer <(anonymous)> at (10,435) content-size 780x0 children: inline
         TextNode <#text>
       BlockContainer <div.h.min> at (11,436) content-size 778x402 children: inline
-        frag 0 from ImageBox start: 0, length: 0, rect: [12,437 400x400] baseline: 402
+        frag 0 from ImageBox start: 0, length: 0, rect: [12,437 402x402] baseline: 404
-        ImageBox <img> at (12,437) content-size 400x400 children: not-inline
+        ImageBox <img> at (12,437) content-size 402x402 children: not-inline
       BlockContainer <(anonymous)> at (10,839) content-size 780x0 children: inline
         TextNode <#text>
       BlockContainer <div.h.max> at (11,840) content-size 778x402 children: inline
-        frag 0 from ImageBox start: 0, length: 0, rect: [12,841 400x400] baseline: 402
+        frag 0 from ImageBox start: 0, length: 0, rect: [12,841 402x402] baseline: 404
-        ImageBox <img> at (12,841) content-size 400x400 children: not-inline
+        ImageBox <img> at (12,841) content-size 402x402 children: not-inline
       BlockContainer <(anonymous)> at (10,1243) content-size 780x0 children: inline
         TextNode <#text>
 
@@ -31,9 +31,9 @@ ViewportPaintable (Viewport<#document>) [0,0 800x600] overflow: [0,0 800x1253]
       PaintableWithLines (BlockContainer<DIV>.w.max) [10,29 404x406] overflow: [11,30 403x404]
         ImagePaintable (ImageBox<IMG>) [11,30 404x404]
       PaintableWithLines (BlockContainer(anonymous)) [10,435 780x0]
-      PaintableWithLines (BlockContainer<DIV>.h.min) [10,435 780x404]
+      PaintableWithLines (BlockContainer<DIV>.h.min) [10,435 780x404] overflow: [11,436 778x403]
-        ImagePaintable (ImageBox<IMG>) [11,436 402x402]
+        ImagePaintable (ImageBox<IMG>) [11,436 404x404]
       PaintableWithLines (BlockContainer(anonymous)) [10,839 780x0]
-      PaintableWithLines (BlockContainer<DIV>.h.max) [10,839 780x404]
+      PaintableWithLines (BlockContainer<DIV>.h.max) [10,839 780x404] overflow: [11,840 778x403]
-        ImagePaintable (ImageBox<IMG>) [11,840 402x402]
+        ImagePaintable (ImageBox<IMG>) [11,840 404x404]
       PaintableWithLines (BlockContainer(anonymous)) [10,1243 780x0]

Tests/LibWeb/Layout/expected/top-layer.txt

@@ -2,15 +2,15 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
   BlockContainer <html> at (0,0) content-size 800x16 [BFC] children: not-inline
     BlockContainer <body> at (8,8) content-size 784x0 children: inline
       TextNode <#text>
-  BlockContainer <dialog#dialog> at (196.671875,19) content-size 406.65625x562 positioned [BFC] children: not-inline
+  BlockContainer <dialog#dialog> at (196.671875,275.5) content-size 406.65625x49 positioned [BFC] children: not-inline
-    BlockContainer <p> at (196.671875,35) content-size 406.65625x17 children: inline
+    BlockContainer <p> at (196.671875,291.5) content-size 406.65625x17 children: inline
-      frag 0 from TextNode start: 0, length: 50, rect: [196.671875,35 406.65625x17] baseline: 13.296875
+      frag 0 from TextNode start: 0, length: 50, rect: [196.671875,291.5 406.65625x17] baseline: 13.296875
           "Dialog's layout node should be a child of viewport"
       TextNode <#text>
 
 ViewportPaintable (Viewport<#document>) [0,0 800x600]
   PaintableWithLines (BlockContainer<HTML>) [0,0 800x16]
     PaintableWithLines (BlockContainer<BODY>) [8,8 784x0]
-  PaintableWithLines (BlockContainer<DIALOG>#dialog) [177.671875,0 444.65625x600]
+  PaintableWithLines (BlockContainer<DIALOG>#dialog) [177.671875,256.5 444.65625x87]
-    PaintableWithLines (BlockContainer<P>) [196.671875,35 406.65625x17]
+    PaintableWithLines (BlockContainer<P>) [196.671875,291.5 406.65625x17]
       TextPaintable (TextNode<#text>)

Tests/LibWeb/Text/expected/wpt-import/css/css-flexbox/flex-minimum-height-flex-items-031.txt

@@ -6,78 +6,77 @@ Rerun
 
 Found 72 tests
 
-24 Pass
+72 Pass
-48 Fail
 Details
 Result	Test Name	MessagePass	.flex 1	
 Pass	.flex 2	
-Fail	.flex 3	
+Pass	.flex 3	
-Fail	.flex 4	
+Pass	.flex 4	
-Fail	.flex 5	
+Pass	.flex 5	
-Fail	.flex 6	
+Pass	.flex 6	
 Pass	.flex 7	
 Pass	.flex 8	
-Fail	.flex 9	
+Pass	.flex 9	
-Fail	.flex 10	
+Pass	.flex 10	
-Fail	.flex 11	
+Pass	.flex 11	
-Fail	.flex 12	
+Pass	.flex 12	
 Pass	.flex 13	
 Pass	.flex 14	
-Fail	.flex 15	
+Pass	.flex 15	
-Fail	.flex 16	
+Pass	.flex 16	
-Fail	.flex 17	
+Pass	.flex 17	
-Fail	.flex 18	
+Pass	.flex 18	
 Pass	.flex 19	
 Pass	.flex 20	
-Fail	.flex 21	
+Pass	.flex 21	
-Fail	.flex 22	
+Pass	.flex 22	
-Fail	.flex 23	
+Pass	.flex 23	
-Fail	.flex 24	
+Pass	.flex 24	
 Pass	.flex 25	
 Pass	.flex 26	
-Fail	.flex 27	
+Pass	.flex 27	
-Fail	.flex 28	
+Pass	.flex 28	
-Fail	.flex 29	
+Pass	.flex 29	
-Fail	.flex 30	
+Pass	.flex 30	
 Pass	.flex 31	
 Pass	.flex 32	
-Fail	.flex 33	
+Pass	.flex 33	
-Fail	.flex 34	
+Pass	.flex 34	
-Fail	.flex 35	
+Pass	.flex 35	
-Fail	.flex 36	
+Pass	.flex 36	
 Pass	.flex 37	
 Pass	.flex 38	
-Fail	.flex 39	
+Pass	.flex 39	
-Fail	.flex 40	
+Pass	.flex 40	
-Fail	.flex 41	
+Pass	.flex 41	
-Fail	.flex 42	
+Pass	.flex 42	
 Pass	.flex 43	
 Pass	.flex 44	
-Fail	.flex 45	
+Pass	.flex 45	
-Fail	.flex 46	
+Pass	.flex 46	
-Fail	.flex 47	
+Pass	.flex 47	
-Fail	.flex 48	
+Pass	.flex 48	
 Pass	.flex 49	
 Pass	.flex 50	
-Fail	.flex 51	
+Pass	.flex 51	
-Fail	.flex 52	
+Pass	.flex 52	
-Fail	.flex 53	
+Pass	.flex 53	
-Fail	.flex 54	
+Pass	.flex 54	
 Pass	.flex 55	
 Pass	.flex 56	
-Fail	.flex 57	
+Pass	.flex 57	
-Fail	.flex 58	
+Pass	.flex 58	
-Fail	.flex 59	
+Pass	.flex 59	
-Fail	.flex 60	
+Pass	.flex 60	
 Pass	.flex 61	
 Pass	.flex 62	
-Fail	.flex 63	
+Pass	.flex 63	
-Fail	.flex 64	
+Pass	.flex 64	
-Fail	.flex 65	
+Pass	.flex 65	
-Fail	.flex 66	
+Pass	.flex 66	
 Pass	.flex 67	
 Pass	.flex 68	
-Fail	.flex 69	
+Pass	.flex 69	
-Fail	.flex 70	
+Pass	.flex 70	
-Fail	.flex 71	
+Pass	.flex 71	
-Fail	.flex 72	
+Pass	.flex 72