From 002e79a658f553bfc275e770fff3dce86543119a Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Wed, 26 Mar 2025 16:11:50 +0000 Subject: [PATCH] LibWeb: Exclude trailing whitespace from line width when placing floats When generating line boxes, we place floats simultaneously with the other items on the same line. The CSS text spec requires us to trim the whitespace at the end of each line, but we only did so after laying out all the line boxes. This changes the way we calculate the current line box width for floats by subtracting the amount of pixels that the current trailing whitespace is using. Fixes #4050. --- Libraries/LibWeb/Layout/LineBox.cpp | 55 +++++++++++++------ Libraries/LibWeb/Layout/LineBox.h | 8 +++ Libraries/LibWeb/Layout/LineBuilder.cpp | 6 +- .../float-following-trailing-whitespace.txt | 28 ++++++++++ .../float-following-trailing-whitespace.html | 21 +++++++ 5 files changed, 99 insertions(+), 19 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/block-and-inline/float-following-trailing-whitespace.txt create mode 100644 Tests/LibWeb/Layout/input/block-and-inline/float-following-trailing-whitespace.html diff --git a/Libraries/LibWeb/Layout/LineBox.cpp b/Libraries/LibWeb/Layout/LineBox.cpp index 82f743e1468..84467e40c63 100644 --- a/Libraries/LibWeb/Layout/LineBox.cpp +++ b/Libraries/LibWeb/Layout/LineBox.cpp @@ -5,7 +5,6 @@ */ #include -#include #include #include #include @@ -54,50 +53,70 @@ void LineBox::add_fragment(Node const& layout_node, int start, int length, CSSPi m_block_length = max(m_block_length, content_height + border_box_top + border_box_bottom); } -void LineBox::trim_trailing_whitespace() +CSSPixels LineBox::calculate_or_trim_trailing_whitespace(RemoveTrailingWhitespace should_remove) { auto should_trim = [](LineBoxFragment* fragment) { auto ws = fragment->layout_node().computed_values().white_space(); return ws == CSS::WhiteSpace::Normal || ws == CSS::WhiteSpace::Nowrap || ws == CSS::WhiteSpace::PreLine; }; + CSSPixels whitespace_width = 0; LineBoxFragment* last_fragment = nullptr; + size_t fragment_index = m_fragments.size(); for (;;) { - if (m_fragments.is_empty()) - return; - // last_fragment cannot be null from here on down, as m_fragments is not empty. - last_fragment = &m_fragments.last(); + if (fragment_index == 0) + return whitespace_width; + + last_fragment = &m_fragments[--fragment_index]; auto const* dom_node = last_fragment->layout_node().dom_node(); if (dom_node) { auto cursor_position = dom_node->document().cursor_position(); if (cursor_position && cursor_position->node() == dom_node) - return; + return whitespace_width; } if (!should_trim(last_fragment)) - return; - if (last_fragment->is_justifiable_whitespace()) { - m_inline_length -= last_fragment->inline_length(); - m_fragments.remove(m_fragments.size() - 1); - } else { + return whitespace_width; + if (!last_fragment->is_justifiable_whitespace()) break; + + whitespace_width += last_fragment->inline_length(); + if (should_remove == RemoveTrailingWhitespace::Yes) { + m_inline_length -= last_fragment->inline_length(); + m_fragments.remove(fragment_index); } } auto last_text = last_fragment->text(); if (last_text.is_null()) - return; + return whitespace_width; - while (last_fragment->length()) { - auto last_character = last_text[last_fragment->length() - 1]; + size_t last_fragment_length = last_fragment->length(); + while (last_fragment_length) { + auto last_character = last_text[--last_fragment_length]; if (!is_ascii_space(last_character)) break; auto const& font = last_fragment->glyph_run() ? last_fragment->glyph_run()->font() : last_fragment->layout_node().first_available_font(); int last_character_width = font.glyph_width(last_character); - last_fragment->m_length -= 1; - last_fragment->set_inline_length(last_fragment->inline_length() - last_character_width); - m_inline_length -= last_character_width; + whitespace_width += last_character_width; + if (should_remove == RemoveTrailingWhitespace::Yes) { + last_fragment->m_length -= 1; + last_fragment->set_inline_length(last_fragment->inline_length() - last_character_width); + m_inline_length -= last_character_width; + } } + + return whitespace_width; +} + +CSSPixels LineBox::get_trailing_whitespace_width() const +{ + return const_cast(*this).calculate_or_trim_trailing_whitespace(RemoveTrailingWhitespace::No); +} + +void LineBox::trim_trailing_whitespace() +{ + calculate_or_trim_trailing_whitespace(RemoveTrailingWhitespace::Yes); } bool LineBox::is_empty_or_ends_in_whitespace() const diff --git a/Libraries/LibWeb/Layout/LineBox.h b/Libraries/LibWeb/Layout/LineBox.h index 0f22a5e39d6..2daf8473be7 100644 --- a/Libraries/LibWeb/Layout/LineBox.h +++ b/Libraries/LibWeb/Layout/LineBox.h @@ -32,6 +32,7 @@ public: Vector const& fragments() const { return m_fragments; } Vector& fragments() { return m_fragments; } + CSSPixels get_trailing_whitespace_width() const; void trim_trailing_whitespace(); bool is_empty_or_ends_in_whitespace() const; @@ -44,6 +45,13 @@ private: friend class InlineFormattingContext; friend class LineBuilder; + enum class RemoveTrailingWhitespace : u8 { + Yes, + No, + }; + + CSSPixels calculate_or_trim_trailing_whitespace(RemoveTrailingWhitespace); + Vector m_fragments; CSSPixels m_inline_length { 0 }; CSSPixels m_block_length { 0 }; diff --git a/Libraries/LibWeb/Layout/LineBuilder.cpp b/Libraries/LibWeb/Layout/LineBuilder.cpp index 4268931db31..60d53e4d98b 100644 --- a/Libraries/LibWeb/Layout/LineBuilder.cpp +++ b/Libraries/LibWeb/Layout/LineBuilder.cpp @@ -117,10 +117,14 @@ CSSPixels LineBuilder::y_for_float_to_be_inserted_here(Box const& box) CSSPixels candidate_block_offset = m_current_block_offset; + // Determine the current line width and subtract trailing whitespace, since those have not yet been removed while + // placing floating boxes. auto const& current_line = ensure_last_line_box(); + auto current_line_width = current_line.width() - current_line.get_trailing_whitespace_width(); + // If there's already inline content on the current line, check if the new float can fit // alongside the content. If not, place it on the next line. - if (current_line.width() > 0 && (current_line.width() + width) > m_available_width_for_current_line) + if (current_line_width > 0 && (current_line_width + width) > m_available_width_for_current_line) candidate_block_offset += current_line.height(); // Then, look for the next Y position where we can fit the new float. diff --git a/Tests/LibWeb/Layout/expected/block-and-inline/float-following-trailing-whitespace.txt b/Tests/LibWeb/Layout/expected/block-and-inline/float-following-trailing-whitespace.txt new file mode 100644 index 00000000000..38a71a32976 --- /dev/null +++ b/Tests/LibWeb/Layout/expected/block-and-inline/float-following-trailing-whitespace.txt @@ -0,0 +1,28 @@ +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: not-inline + BlockContainer at (8,8) content-size 100x17 children: inline + frag 0 from BlockContainer start: 0, length: 0, rect: [8,8 50x17] baseline: 13.296875 + TextNode <#text> + BlockContainer at (8,8) content-size 50x17 inline-block [BFC] children: inline + frag 0 from TextNode start: 0, length: 3, rect: [8,8 27.15625x17] baseline: 13.296875 + "foo" + TextNode <#text> + TextNode <#text> + BlockContainer at (58,8) content-size 50x17 floating [BFC] children: inline + frag 0 from TextNode start: 0, length: 3, rect: [58,8 27.640625x17] baseline: 13.296875 + "bar" + TextNode <#text> + TextNode <#text> + BlockContainer <(anonymous)> at (8,25) content-size 784x0 children: inline + TextNode <#text> + +ViewportPaintable (Viewport<#document>) [0,0 800x600] + PaintableWithLines (BlockContainer) [0,0 800x33] + PaintableWithLines (BlockContainer) [8,8 784x17] + PaintableWithLines (BlockContainer
.a) [8,8 100x17] + PaintableWithLines (BlockContainer
.b) [8,8 50x17] + TextPaintable (TextNode<#text>) + PaintableWithLines (BlockContainer
.c) [58,8 50x17] + TextPaintable (TextNode<#text>) + PaintableWithLines (BlockContainer(anonymous)) [8,25 784x0] diff --git a/Tests/LibWeb/Layout/input/block-and-inline/float-following-trailing-whitespace.html b/Tests/LibWeb/Layout/input/block-and-inline/float-following-trailing-whitespace.html new file mode 100644 index 00000000000..3696f1ca3e1 --- /dev/null +++ b/Tests/LibWeb/Layout/input/block-and-inline/float-following-trailing-whitespace.html @@ -0,0 +1,21 @@ + + +
+
foo
+
bar
+