From af8b2568321477a28d4efb3d24f39c512347845d Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Mon, 28 Jul 2025 21:24:30 +0200 Subject: [PATCH] LibWeb: Avoid floats for BFC/FFC/GFCs with a definite width Applicable FCs with an indefinite width simply shrink in their available space as long as floats are intruding, but as soon as we have a definite width we must push the box down until it it has enough space again. Fixes #4136. --- .../LibWeb/Layout/BlockFormattingContext.cpp | 96 +++++++++++----- .../LibWeb/Layout/BlockFormattingContext.h | 1 + ...definite-width-avoids-float-intrusions.txt | 106 ++++++++++++++++++ ...th-width-auto-avoids-float-intrusions.txt} | 0 ...efinite-width-avoids-float-intrusions.html | 50 +++++++++ ...h-width-auto-avoids-float-intrusions.html} | 0 6 files changed, 226 insertions(+), 27 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/flex/flex-container-with-definite-width-avoids-float-intrusions.txt rename Tests/LibWeb/Layout/expected/flex/{flex-container-avoids-float-intrusions.txt => flex-container-with-width-auto-avoids-float-intrusions.txt} (100%) create mode 100644 Tests/LibWeb/Layout/input/flex/flex-container-with-definite-width-avoids-float-intrusions.html rename Tests/LibWeb/Layout/input/flex/{flex-container-avoids-float-intrusions.html => flex-container-with-width-auto-avoids-float-intrusions.html} (100%) diff --git a/Libraries/LibWeb/Layout/BlockFormattingContext.cpp b/Libraries/LibWeb/Layout/BlockFormattingContext.cpp index a115f79c677..dad28349a81 100644 --- a/Libraries/LibWeb/Layout/BlockFormattingContext.cpp +++ b/Libraries/LibWeb/Layout/BlockFormattingContext.cpp @@ -194,7 +194,7 @@ void BlockFormattingContext::compute_width(Box const& box, AvailableSpace const& // FIXME: This const_cast is gross. const_cast(replaced).prepare_for_replaced_layout(); } - compute_width_for_block_level_replaced_element_in_normal_flow(box, remaining_available_space); + compute_width_for_block_level_replaced_element_in_normal_flow(box, available_space); if (box.is_floating()) { // 10.3.6 Floating, replaced elements: // https://www.w3.org/TR/CSS22/visudet.html#float-replaced-width @@ -210,23 +210,19 @@ void BlockFormattingContext::compute_width(Box const& box, AvailableSpace const& } auto const& computed_values = box.computed_values(); - - auto width_of_containing_block = remaining_available_space.width.to_px_or_zero(); - - auto zero_value = CSS::Length::make_px(0); - - auto margin_left = computed_values.margin().left().resolved(box, width_of_containing_block); - auto margin_right = computed_values.margin().right().resolved(box, width_of_containing_block); - auto const padding_left = computed_values.padding().left().resolved(box, width_of_containing_block).to_px(box); - auto const padding_right = computed_values.padding().right().resolved(box, width_of_containing_block).to_px(box); + auto available_width_px = available_space.width.to_px_or_zero(); + auto margin_left = computed_values.margin().left().resolved(box, available_width_px); + auto margin_right = computed_values.margin().right().resolved(box, available_width_px); + auto const padding_left = computed_values.padding().left().resolved(box, available_width_px); + auto const padding_right = computed_values.padding().right().resolved(box, available_width_px); auto& box_state = m_state.get_mutable(box); box_state.margin_left = margin_left.to_px(box); box_state.margin_right = margin_right.to_px(box); box_state.border_left = computed_values.border_left().width; box_state.border_right = computed_values.border_right().width; - box_state.padding_left = padding_left; - box_state.padding_right = padding_right; + box_state.padding_left = padding_left.to_px(box); + box_state.padding_right = padding_right.to_px(box); // NOTE: If we are calculating the min-content or max-content width of this box, // and the width should be treated as auto, then we can simply return here, @@ -234,14 +230,16 @@ void BlockFormattingContext::compute_width(Box const& box, AvailableSpace const& if (box_state.width_constraint != SizeConstraint::None) return; + auto const remaining_width_px = remaining_available_space.width.to_px_or_zero(); + auto const zero_value = CSS::Length::make_px(0); + auto try_compute_width = [&](CSS::Length const& a_width) { CSS::Length width = a_width; - margin_left = computed_values.margin().left().resolved(box, width_of_containing_block); - margin_right = computed_values.margin().right().resolved(box, width_of_containing_block); + margin_left = computed_values.margin().left().resolved(box, available_space.width.to_px_or_zero()); + margin_right = computed_values.margin().right().resolved(box, available_space.width.to_px_or_zero()); CSSPixels total_px = computed_values.border_left().width + computed_values.border_right().width; - for (auto& value : { margin_left, CSS::Length::make_px(padding_left), width, CSS::Length::make_px(padding_right), margin_right }) { + for (auto& value : { margin_left, padding_left, width, padding_right, margin_right }) total_px += value.to_px(box); - } if (!box.is_inline()) { // 10.3.3 Block-level, non-replaced elements in normal flow @@ -249,7 +247,7 @@ void BlockFormattingContext::compute_width(Box const& box, AvailableSpace const& // 'border-right-width' (plus any of 'margin-left' or 'margin-right' that are not 'auto') is larger than the // width of the containing block, then any 'auto' values for 'margin-left' or 'margin-right' are, for the // following rules, treated as zero. - if (!width.is_auto() && total_px > width_of_containing_block) { + if (!width.is_auto() && total_px > remaining_width_px) { if (margin_left.is_auto()) margin_left = zero_value; if (margin_right.is_auto()) @@ -257,7 +255,7 @@ void BlockFormattingContext::compute_width(Box const& box, AvailableSpace const& } // 10.3.3 cont'd. - auto underflow_px = width_of_containing_block - total_px; + auto underflow_px = remaining_width_px - total_px; if (available_space.width.is_intrinsic_sizing_constraint()) underflow_px = 0; @@ -306,9 +304,9 @@ void BlockFormattingContext::compute_width(Box const& box, AvailableSpace const& } if (is(box)) return CSS::Length::make_px(compute_table_box_width_inside_table_wrapper(box, remaining_available_space)); - if (should_treat_width_as_auto(box, remaining_available_space)) + if (should_treat_width_as_auto(box, available_space)) return CSS::Length::make_auto(); - return CSS::Length::make_px(calculate_inner_width(box, remaining_available_space.width, computed_values.width())); + return CSS::Length::make_px(calculate_inner_width(box, available_space.width, computed_values.width())); }(); // 1. The tentative used width is calculated (without 'min-width' and 'max-width') @@ -317,20 +315,18 @@ void BlockFormattingContext::compute_width(Box const& box, AvailableSpace const& // 2. The tentative used width is greater than 'max-width', the rules above are applied again, // but this time using the computed value of 'max-width' as the computed value for 'width'. if (!should_treat_max_width_as_none(box, available_space.width)) { - auto max_width = calculate_inner_width(box, remaining_available_space.width, computed_values.max_width()); - if (used_width.to_px(box) > max_width) { + auto max_width = calculate_inner_width(box, available_space.width, computed_values.max_width()); + if (used_width.to_px(box) > max_width) used_width = try_compute_width(CSS::Length::make_px(max_width)); - } } // 3. If the resulting width is smaller than 'min-width', the rules above are applied again, // but this time using the value of 'min-width' as the computed value for 'width'. if (!computed_values.min_width().is_auto()) { - auto min_width = calculate_inner_width(box, remaining_available_space.width, computed_values.min_width()); + auto min_width = calculate_inner_width(box, available_space.width, computed_values.min_width()); auto used_width_px = used_width.is_auto() ? remaining_available_space.width : AvailableSize::make_definite(used_width.to_px(box)); - if (used_width_px < min_width) { + if (used_width_px < min_width) used_width = try_compute_width(CSS::Length::make_px(min_width)); - } } if (!box_is_sized_as_replaced_element(box) && !used_width.is_auto()) @@ -340,6 +336,45 @@ void BlockFormattingContext::compute_width(Box const& box, AvailableSpace const& box_state.margin_right = margin_right.to_px(box); } +void BlockFormattingContext::avoid_float_intrusions(Box const& box, AvailableSpace const& available_space) +{ + if (box.computed_values().width().is_auto()) + return; + if (!available_space.width.is_definite()) + return; + if (!box_should_avoid_floats_because_it_establishes_fc(box)) + return; + + // https://drafts.csswg.org/css2/#floats + // If necessary, implementations should clear the said element by placing it below any preceding floats, but may + // place it adjacent to such floats if there is sufficient space. + auto& box_state = m_state.get_mutable(box); + while (true) { + auto box_in_root_rect = margin_box_rect_in_ancestor_coordinate_space(box_state, root()); + auto space_used_by_floats = space_used_and_containing_margin_for_floats(box_in_root_rect.y()); + auto remaining_space = available_space.width.to_px_or_zero() - space_used_by_floats.left_used_space - space_used_by_floats.right_used_space; + if (box_state.border_box_width() <= remaining_space) + break; + + Optional topmost_float_bottom; + auto find_topmost_float = [&](auto float_box) { + if (!float_box) + return; + auto const& float_used_values = m_state.get(*float_box); + auto float_in_root_rect = margin_box_rect_in_ancestor_coordinate_space(float_used_values, root()); + auto float_bottom = float_in_root_rect.bottom(); + if (!topmost_float_bottom.has_value() || topmost_float_bottom.value() > float_bottom) + topmost_float_bottom = float_bottom; + }; + find_topmost_float(space_used_by_floats.matching_left_float_box); + find_topmost_float(space_used_by_floats.matching_right_float_box); + if (!topmost_float_bottom.has_value()) + break; + + box_state.set_content_y(box_state.offset.y() + topmost_float_bottom.value() - box_in_root_rect.y()); + } +} + void BlockFormattingContext::compute_width_for_floating_box(Box const& box, AvailableSpace const& available_space) { // 10.3.5 Floating, non-replaced elements @@ -749,6 +784,7 @@ void BlockFormattingContext::layout_block_level_box(Box const& box, BlockContain place_block_level_element_in_normal_flow_vertically(box, y + margin_top); compute_width(box, available_space); + avoid_float_intrusions(box, available_space); place_block_level_element_in_normal_flow_horizontally(box, available_space); @@ -963,9 +999,9 @@ void BlockFormattingContext::place_block_level_element_in_normal_flow_vertically auto& box_state = m_state.get_mutable(child_box); y += box_state.border_box_top(); box_state.set_content_y(y); + for (auto const& float_box : m_left_floats.all_boxes) float_box->margin_box_rect_in_root_coordinate_space = margin_box_rect_in_ancestor_coordinate_space(float_box->used_values, root()); - for (auto const& float_box : m_right_floats.all_boxes) float_box->margin_box_rect_in_root_coordinate_space = margin_box_rect_in_ancestor_coordinate_space(float_box->used_values, root()); } @@ -982,7 +1018,13 @@ void BlockFormattingContext::place_block_level_element_in_normal_flow_horizontal auto box_in_root_rect = content_box_rect_in_ancestor_coordinate_space(box_state, root()); auto space_and_containing_margin = space_used_and_containing_margin_for_floats(box_in_root_rect.y()); available_width_within_containing_block -= space_and_containing_margin.left_used_space + space_and_containing_margin.right_used_space; + + // Since this box has a FC, it should avoid floats which means we cannot have its border box overlap with any + // float's margin box. We start off at the right-most border of the floats, and if this box' margin-left is not + // auto, we must overlap that margin with the floats as far as possible. x = space_and_containing_margin.left_used_space; + if (!child_box.computed_values().margin().left().is_auto()) + x = max(x - max(box_state.margin_left, 0), 0); // All non-anonymous containing blocks that are ancestors of the child box, but are not ancestors of the left // float box, might have left margins set that overlap with the used float space. diff --git a/Libraries/LibWeb/Layout/BlockFormattingContext.h b/Libraries/LibWeb/Layout/BlockFormattingContext.h index d535bb2bf6f..4098cc57821 100644 --- a/Libraries/LibWeb/Layout/BlockFormattingContext.h +++ b/Libraries/LibWeb/Layout/BlockFormattingContext.h @@ -31,6 +31,7 @@ public: bool box_should_avoid_floats_because_it_establishes_fc(Box const&); void compute_width(Box const&, AvailableSpace const&); + void avoid_float_intrusions(Box const&, AvailableSpace const&); // https://www.w3.org/TR/css-display/#block-formatting-context-root BlockContainer const& root() const { return static_cast(context_box()); } diff --git a/Tests/LibWeb/Layout/expected/flex/flex-container-with-definite-width-avoids-float-intrusions.txt b/Tests/LibWeb/Layout/expected/flex/flex-container-with-definite-width-avoids-float-intrusions.txt new file mode 100644 index 00000000000..ee8dbfb09dd --- /dev/null +++ b/Tests/LibWeb/Layout/expected/flex/flex-container-with-definite-width-avoids-float-intrusions.txt @@ -0,0 +1,106 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x1098 [BFC] children: not-inline + BlockContainer at (8,8) content-size 400x990 children: not-inline + BlockContainer <(anonymous)> at (8,8) content-size 400x18 children: inline + BreakNode
+ BlockContainer at (8,26) content-size 100x100 floating [BFC] children: not-inline + Box at (8,126) content-size 400x100 flex-container(row) [FFC] children: not-inline + BlockContainer <(anonymous)> at (8,126) content-size 257.3125x100 flex-item [BFC] children: inline + frag 0 from TextNode start: 0, length: 31, rect: [8,126 257.3125x18] baseline: 13.796875 + "This should clear the blue box." + TextNode <#text> + BlockContainer <(anonymous)> at (8,226) content-size 400x0 children: inline + TextNode <#text> + BlockContainer
at (9,235) content-size 398x0 [BFC] children: not-inline + BlockContainer at (8,244) content-size 100x100 floating [BFC] children: not-inline + BlockContainer at (358,244) content-size 50x200 floating [BFC] children: not-inline + Box at (8,444) content-size 400x100 flex-container(row) [FFC] children: not-inline + BlockContainer <(anonymous)> at (8,444) content-size 268.5625x100 flex-item [BFC] children: inline + frag 0 from TextNode start: 0, length: 32, rect: [8,444 268.5625x18] baseline: 13.796875 + "This should clear the green box." + TextNode <#text> + BlockContainer <(anonymous)> at (8,544) content-size 400x0 children: inline + TextNode <#text> + BlockContainer
at (9,553) content-size 398x0 [BFC] children: not-inline + BlockContainer at (8,562) content-size 100x100 floating [BFC] children: not-inline + BlockContainer at (358,562) content-size 50x200 floating [BFC] children: not-inline + Box at (58,662) content-size 300x100 flex-container(row) [FFC] children: not-inline + BlockContainer <(anonymous)> at (58,662) content-size 300x100 flex-item [BFC] children: inline + frag 0 from TextNode start: 0, length: 34, rect: [58,662 287.78125x18] baseline: 13.796875 + "This should clear the blue box and" + frag 1 from TextNode start: 35, length: 32, rect: [58,680 258.65625x18] baseline: 13.796875 + "sit flush against the green box." + TextNode <#text> + BlockContainer <(anonymous)> at (8,762) content-size 400x0 children: inline + TextNode <#text> + BlockContainer
at (9,771) content-size 398x0 [BFC] children: not-inline + BlockContainer at (8,780) content-size 100x100 floating [BFC] children: not-inline + Box at (148,780) content-size 150x100 flex-container(row) [FFC] children: not-inline + BlockContainer <(anonymous)> at (148,780) content-size 150x100 flex-item [BFC] children: inline + frag 0 from TextNode start: 0, length: 15, rect: [148,780 123.125x18] baseline: 13.796875 + "This should sit" + frag 1 from TextNode start: 16, length: 17, rect: [148,798 136.578125x18] baseline: 13.796875 + "flush against the" + frag 2 from TextNode start: 34, length: 14, rect: [148,816 114.703125x18] baseline: 13.796875 + "blue box, with" + frag 3 from TextNode start: 49, length: 15, rect: [148,834 125.484375x18] baseline: 13.796875 + "40px of padding" + frag 4 from TextNode start: 65, length: 12, rect: [148,852 88.6875x18] baseline: 13.796875 + "to the left." + TextNode <#text> + BlockContainer <(anonymous)> at (8,880) content-size 400x0 children: inline + TextNode <#text> + BlockContainer
at (9,889) content-size 398x0 [BFC] children: not-inline + BlockContainer at (8,898) content-size 100x100 floating [BFC] children: not-inline + BlockContainer at (358,898) content-size 50x200 floating [BFC] children: not-inline + Box at (133,898) content-size 200x100 flex-container(row) [FFC] children: not-inline + BlockContainer <(anonymous)> at (133,898) content-size 200x100 flex-item [BFC] children: inline + frag 0 from TextNode start: 0, length: 23, rect: [133,898 196.75x18] baseline: 13.796875 + "This should be centered" + frag 1 from TextNode start: 24, length: 20, rect: [133,916 171.1875x18] baseline: 13.796875 + "between the blue and" + frag 2 from TextNode start: 45, length: 12, rect: [133,934 103.375x18] baseline: 13.796875 + "green boxes." + TextNode <#text> + BlockContainer <(anonymous)> at (8,998) content-size 400x0 children: inline + TextNode <#text> + +ViewportPaintable (Viewport<#document>) [0,0 800x600] overflow: [0,0 800x1098] + PaintableWithLines (BlockContainer) [0,0 800x1098] + PaintableWithLines (BlockContainer) [8,8 400x990] overflow: [8,8 400x1090] + PaintableWithLines (BlockContainer(anonymous)) [8,8 400x18] overflow: [8,8 400x118] + PaintableWithLines (BlockContainer
.a) [8,26 100x100] + PaintableBox (Box
.c) [8,126 400x100] + PaintableWithLines (BlockContainer(anonymous)) [8,126 257.3125x100] + TextPaintable (TextNode<#text>) + PaintableWithLines (BlockContainer(anonymous)) [8,226 400x0] + PaintableWithLines (BlockContainer
) [8,234 400x2] + PaintableWithLines (BlockContainer
.a) [8,244 100x100] + PaintableWithLines (BlockContainer
.b) [358,244 50x200] + PaintableBox (Box
.c) [8,444 400x100] + PaintableWithLines (BlockContainer(anonymous)) [8,444 268.5625x100] + TextPaintable (TextNode<#text>) + PaintableWithLines (BlockContainer(anonymous)) [8,544 400x0] + PaintableWithLines (BlockContainer
) [8,552 400x2] + PaintableWithLines (BlockContainer
.a) [8,562 100x100] + PaintableWithLines (BlockContainer
.b) [358,562 50x200] + PaintableBox (Box
.d) [58,662 300x100] + PaintableWithLines (BlockContainer(anonymous)) [58,662 300x100] + TextPaintable (TextNode<#text>) + PaintableWithLines (BlockContainer(anonymous)) [8,762 400x0] + PaintableWithLines (BlockContainer
) [8,770 400x2] + PaintableWithLines (BlockContainer
.a) [8,780 100x100] + PaintableBox (Box
.e) [108,780 190x100] + PaintableWithLines (BlockContainer(anonymous)) [148,780 150x100] + TextPaintable (TextNode<#text>) + PaintableWithLines (BlockContainer(anonymous)) [8,880 400x0] + PaintableWithLines (BlockContainer
) [8,888 400x2] + PaintableWithLines (BlockContainer
.a) [8,898 100x100] + PaintableWithLines (BlockContainer
.b) [358,898 50x200] + PaintableBox (Box
.f) [133,898 200x100] + PaintableWithLines (BlockContainer(anonymous)) [133,898 200x100] + TextPaintable (TextNode<#text>) + PaintableWithLines (BlockContainer(anonymous)) [8,998 400x0] + +SC for Viewport<#document> [0,0 800x600] [children: 1] (z-index: auto) + SC for BlockContainer [0,0 800x1098] [children: 0] (z-index: auto) diff --git a/Tests/LibWeb/Layout/expected/flex/flex-container-avoids-float-intrusions.txt b/Tests/LibWeb/Layout/expected/flex/flex-container-with-width-auto-avoids-float-intrusions.txt similarity index 100% rename from Tests/LibWeb/Layout/expected/flex/flex-container-avoids-float-intrusions.txt rename to Tests/LibWeb/Layout/expected/flex/flex-container-with-width-auto-avoids-float-intrusions.txt diff --git a/Tests/LibWeb/Layout/input/flex/flex-container-with-definite-width-avoids-float-intrusions.html b/Tests/LibWeb/Layout/input/flex/flex-container-with-definite-width-avoids-float-intrusions.html new file mode 100644 index 00000000000..057340ecacc --- /dev/null +++ b/Tests/LibWeb/Layout/input/flex/flex-container-with-definite-width-avoids-float-intrusions.html @@ -0,0 +1,50 @@ + +
This should clear the blue box.
+
This should clear the green box.
+
This should clear the blue box and sit flush against the green box.
+
This should sit flush against the blue box, with 40px of padding to the left.
+
This should be centered between the blue and green boxes.
diff --git a/Tests/LibWeb/Layout/input/flex/flex-container-avoids-float-intrusions.html b/Tests/LibWeb/Layout/input/flex/flex-container-with-width-auto-avoids-float-intrusions.html similarity index 100% rename from Tests/LibWeb/Layout/input/flex/flex-container-avoids-float-intrusions.html rename to Tests/LibWeb/Layout/input/flex/flex-container-with-width-auto-avoids-float-intrusions.html