From 9ee64b5694ed1ea64dda85350891a618b843bace Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Thu, 27 Apr 2023 04:34:32 +0300 Subject: [PATCH] LibWeb: Prevent margin double-counting with "collapse through" boxes If there is a remaining margin-bottom in margin collapsing state tracker after laying out all boxes in the current BFC, it must be assigned to the last in-flow child since margin collapsing cannot occur across a formatting context boundary. The current issue where margin-bottom may be counted twice due to "collapse through" margins in the last in-flow child box is addressed with this fix by excluding such boxes during the search for a box to assign the remaining margin. Test case coming with this fix has a layout bug with incorrectly computed line height. --- .../block-and-inline/margin-collapse-5.txt | 12 +++++++++++ .../block-and-inline/margin-collapse-5.html | 14 +++++++++++++ .../LibWeb/Layout/BlockFormattingContext.cpp | 20 ++++++++++--------- 3 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/block-and-inline/margin-collapse-5.txt create mode 100644 Tests/LibWeb/Layout/input/block-and-inline/margin-collapse-5.html diff --git a/Tests/LibWeb/Layout/expected/block-and-inline/margin-collapse-5.txt b/Tests/LibWeb/Layout/expected/block-and-inline/margin-collapse-5.txt new file mode 100644 index 0000000000..2f740ec5d7 --- /dev/null +++ b/Tests/LibWeb/Layout/expected/block-and-inline/margin-collapse-5.txt @@ -0,0 +1,12 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (1,1) content-size 798x134.46875 children: inline + line 0 width: 170.96875, height: 134.46875, bottom: 134.46875, baseline: 13.53125 + frag 0 from BlockContainer start: 0, length: 0, rect: [2,15 168.96875x119.46875] + BlockContainer at (2,15) content-size 168.96875x119.46875 inline-block children: not-inline + BlockContainer at (3,16) content-size 166.96875x17.46875 children: inline + line 0 width: 166.96875, height: 17.46875, bottom: 17.46875, baseline: 13.53125 + frag 0 from TextNode start: 0, length: 21, rect: [3,16 166.96875x17.46875] + "suspiciously tall box" + TextNode <#text> + BlockContainer <(anonymous)> at (2,134.46875) content-size 168.96875x0 children: inline + TextNode <#text> diff --git a/Tests/LibWeb/Layout/input/block-and-inline/margin-collapse-5.html b/Tests/LibWeb/Layout/input/block-and-inline/margin-collapse-5.html new file mode 100644 index 0000000000..90fba786bb --- /dev/null +++ b/Tests/LibWeb/Layout/input/block-and-inline/margin-collapse-5.html @@ -0,0 +1,14 @@ +
suspiciously tall box
\ No newline at end of file diff --git a/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp index 4013194d45..fdd5df089e 100644 --- a/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/BlockFormattingContext.cpp @@ -48,6 +48,15 @@ CSSPixels BlockFormattingContext::automatic_content_height() const return compute_auto_height_for_block_formatting_context_root(root()); } +static bool margins_collapse_through(Box const& box, LayoutState& state) +{ + // FIXME: A box's own margins collapse if the 'min-height' property is zero, and it has neither top or bottom borders + // nor top or bottom padding, and it has a 'height' of either 0 or 'auto', and it does not contain a line box, and + // all of its in-flow children's margins (if any) collapse. + // https://www.w3.org/TR/CSS22/box.html#collapsing-margins + return state.get(box).border_box_height() == 0; +} + void BlockFormattingContext::run(Box const&, LayoutMode layout_mode, AvailableSpace const& available_space) { if (is(root())) { @@ -65,6 +74,8 @@ void BlockFormattingContext::run(Box const&, LayoutMode layout_mode, AvailableSp for (auto* child_box = root().last_child_of_type(); child_box; child_box = child_box->previous_sibling_of_type()) { if (child_box->is_absolutely_positioned() || child_box->is_floating()) continue; + if (margins_collapse_through(*child_box, m_state)) + continue; m_state.get_mutable(*child_box).margin_bottom = m_margin_state.current_collapsed_margin().value(); break; } @@ -379,15 +390,6 @@ void BlockFormattingContext::layout_inline_children(BlockContainer const& block_ block_container_state.set_content_height(context.automatic_content_height()); } -static bool margins_collapse_through(Box const& box, LayoutState& state) -{ - // FIXME: A box's own margins collapse if the 'min-height' property is zero, and it has neither top or bottom borders - // nor top or bottom padding, and it has a 'height' of either 0 or 'auto', and it does not contain a line box, and - // all of its in-flow children's margins (if any) collapse. - // https://www.w3.org/TR/CSS22/box.html#collapsing-margins - return state.get(box).border_box_height() == 0; -} - CSSPixels BlockFormattingContext::compute_auto_height_for_block_level_element(Box const& box, AvailableSpace const& available_space) { if (creates_block_formatting_context(box)) {