From 1041dbb007653e654f3052c4d0ed81d27e1e789f Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 20 Jan 2024 20:15:09 +0100 Subject: [PATCH] LibWeb: Don't lose track of inline margins when collapsing whitespace When iterating inline level chunks for a piece of text like " hello ", we will get three separate items from InlineLevelIterator: - Text " " - Text "hello" - Text " " If the first item also had some leading margin (e.g margin-left: 10px) we would lose that information when deciding that the whitespace is collapsible. This patch fixes the issue by accumulating the amount of leading margin present in any collapsed whitespace items, and then adding them to the next non-whitespace item in IFC. It's a wee bit hackish, but so is the rest of the leading/trailing margin mechanism. This makes the header menu on https://www.gimp.org/ look proper. :^) --- ...hat-starts-with-collapsible-whitespace.txt | 20 +++++++++++++++++++ ...at-starts-with-collapsible-whitespace.html | 10 ++++++++++ .../LibWeb/Layout/InlineFormattingContext.cpp | 9 +++++++++ 3 files changed, 39 insertions(+) create mode 100644 Tests/LibWeb/Layout/expected/block-and-inline/leading-margin-on-inline-content-that-starts-with-collapsible-whitespace.txt create mode 100644 Tests/LibWeb/Layout/input/block-and-inline/leading-margin-on-inline-content-that-starts-with-collapsible-whitespace.html diff --git a/Tests/LibWeb/Layout/expected/block-and-inline/leading-margin-on-inline-content-that-starts-with-collapsible-whitespace.txt b/Tests/LibWeb/Layout/expected/block-and-inline/leading-margin-on-inline-content-that-starts-with-collapsible-whitespace.txt new file mode 100644 index 00000000000..09cea00ce30 --- /dev/null +++ b/Tests/LibWeb/Layout/expected/block-and-inline/leading-margin-on-inline-content-that-starts-with-collapsible-whitespace.txt @@ -0,0 +1,20 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x33 [BFC] children: not-inline + BlockContainer at (8,8) content-size 784x17 children: inline + InlineNode
+ frag 0 from TextNode start: 1, length: 9, rect: [28,8 82.125x17] baseline: 13.296875 + "Download " + TextNode <#text> + InlineNode
+ frag 0 from TextNode start: 1, length: 4, rect: [150,8 39.5625x17] baseline: 13.296875 + "News" + TextNode <#text> + TextNode <#text> + +ViewportPaintable (Viewport<#document>) [0,0 800x600] + PaintableWithLines (BlockContainer) [0,0 800x33] + PaintableWithLines (BlockContainer) [8,8 784x17] + InlinePaintable (InlineNode
) + TextPaintable (TextNode<#text>) + InlinePaintable (InlineNode
) + TextPaintable (TextNode<#text>) diff --git a/Tests/LibWeb/Layout/input/block-and-inline/leading-margin-on-inline-content-that-starts-with-collapsible-whitespace.html b/Tests/LibWeb/Layout/input/block-and-inline/leading-margin-on-inline-content-that-starts-with-collapsible-whitespace.html new file mode 100644 index 00000000000..e477c9f5ca9 --- /dev/null +++ b/Tests/LibWeb/Layout/input/block-and-inline/leading-margin-on-inline-content-that-starts-with-collapsible-whitespace.html @@ -0,0 +1,10 @@ +
Download
News
diff --git a/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp index 470fe81c4ae..9e7f1c811d1 100644 --- a/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp @@ -249,6 +249,11 @@ void InlineFormattingContext::generate_line_boxes(LayoutMode layout_mode) InlineLevelIterator iterator(*this, m_state, containing_block(), layout_mode); LineBuilder line_builder(*this, m_state); + // NOTE: When we ignore collapsible whitespace chunks at the start of a line, + // we have to remember how much start margin that chunk had in the inline + // axis, so that we can add it to the first non-whitespace chunk. + CSSPixels leading_margin_from_collapsible_whitespace = 0; + for (;;) { auto item_opt = iterator.next(); if (!item_opt.has_value()) @@ -262,9 +267,13 @@ void InlineFormattingContext::generate_line_boxes(LayoutMode layout_mode) if (next_width > 0) line_builder.break_if_needed(next_width); } + leading_margin_from_collapsible_whitespace += item.margin_start; continue; } + item.margin_start += leading_margin_from_collapsible_whitespace; + leading_margin_from_collapsible_whitespace = 0; + switch (item.type) { case InlineLevelIterator::Item::Type::ForcedBreak: { line_builder.break_line(LineBuilder::ForcedBreak::Yes);