From 3d7e788981ff4ad6c82fb96b1c556839e66706e1 Mon Sep 17 00:00:00 2001 From: Andi Gallo Date: Sat, 19 Aug 2023 03:36:53 +0000 Subject: [PATCH] LibWeb: Improve the line breaking algorithm Check the width of the next token after white space to decide line breaks. The next width can also be the total width of multiple tokens. This better follows the CSS Text specification and matches behavior of other browsers. Fixes #20388. --- .../nowrap-and-no-line-break-opportunity.txt | 30 ++++++++++ .../space-is-soft-line-break-opportunity.txt | 28 +++++++++ .../expected/table/line-breaking-in-cells.txt | 58 +++++++++++++++++++ .../nowrap-and-no-line-break-opportunity.html | 13 +++++ .../space-is-soft-line-break-opportunity.html | 13 +++++ .../input/table/line-breaking-in-cells.html | 25 ++++++++ .../LibWeb/Layout/InlineFormattingContext.cpp | 33 +++++++---- .../LibWeb/Layout/InlineLevelIterator.cpp | 37 +++++++++++- .../LibWeb/Layout/InlineLevelIterator.h | 3 + 9 files changed, 227 insertions(+), 13 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/nowrap-and-no-line-break-opportunity.txt create mode 100644 Tests/LibWeb/Layout/expected/space-is-soft-line-break-opportunity.txt create mode 100644 Tests/LibWeb/Layout/expected/table/line-breaking-in-cells.txt create mode 100644 Tests/LibWeb/Layout/input/nowrap-and-no-line-break-opportunity.html create mode 100644 Tests/LibWeb/Layout/input/space-is-soft-line-break-opportunity.html create mode 100644 Tests/LibWeb/Layout/input/table/line-breaking-in-cells.html diff --git a/Tests/LibWeb/Layout/expected/nowrap-and-no-line-break-opportunity.txt b/Tests/LibWeb/Layout/expected/nowrap-and-no-line-break-opportunity.txt new file mode 100644 index 00000000000..3bdbfa2df47 --- /dev/null +++ b/Tests/LibWeb/Layout/expected/nowrap-and-no-line-break-opportunity.txt @@ -0,0 +1,30 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x600 [BFC] children: not-inline + BlockContainer at (8,8) content-size 784x19.46875 children: not-inline + BlockContainer at (9,9) content-size 50x17.46875 children: inline + line 0 width: 79.40625, height: 17.46875, bottom: 17.46875, baseline: 13.53125 + frag 0 from TextNode start: 0, length: 3, rect: [9,9 33.921875x17.46875] + "ABC" + frag 1 from TextNode start: 0, length: 1, rect: [43,9 11.5625x17.46875] + "X" + frag 2 from TextNode start: 0, length: 3, rect: [54,9 33.921875x17.46875] + "ABC" + TextNode <#text> + InlineNode + TextNode <#text> + InlineNode + TextNode <#text> + InlineNode + TextNode <#text> + TextNode <#text> + +ViewportPaintable (Viewport<#document>) [0,0 800x600] + PaintableWithLines (BlockContainer) [0,0 800x600] + PaintableWithLines (BlockContainer) [8,8 784x19.46875] + PaintableWithLines (BlockContainer
.fixed_width) [8,8 52x19.46875] overflow: [9,9 78.921875x17.46875] + InlinePaintable (InlineNode.nowrap) + TextPaintable (TextNode<#text>) + InlinePaintable (InlineNode) + TextPaintable (TextNode<#text>) + InlinePaintable (InlineNode.nowrap) + TextPaintable (TextNode<#text>) diff --git a/Tests/LibWeb/Layout/expected/space-is-soft-line-break-opportunity.txt b/Tests/LibWeb/Layout/expected/space-is-soft-line-break-opportunity.txt new file mode 100644 index 00000000000..e91ee892d6f --- /dev/null +++ b/Tests/LibWeb/Layout/expected/space-is-soft-line-break-opportunity.txt @@ -0,0 +1,28 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x600 [BFC] children: not-inline + BlockContainer at (8,8) content-size 784x37.40625 children: not-inline + BlockContainer at (9,9) content-size 50x35.40625 children: inline + line 0 width: 33.921875, height: 17.46875, bottom: 17.46875, baseline: 13.53125 + frag 0 from TextNode start: 0, length: 3, rect: [9,9 33.921875x17.46875] + "ABC" + line 1 width: 33.921875, height: 17.9375, bottom: 35.40625, baseline: 13.53125 + frag 0 from TextNode start: 0, length: 3, rect: [9,26 33.921875x17.46875] + "ABC" + TextNode <#text> + InlineNode + TextNode <#text> + InlineNode + TextNode <#text> + InlineNode + TextNode <#text> + TextNode <#text> + +ViewportPaintable (Viewport<#document>) [0,0 800x600] + PaintableWithLines (BlockContainer) [0,0 800x600] + PaintableWithLines (BlockContainer) [8,8 784x37.40625] + PaintableWithLines (BlockContainer
.fixed_width) [8,8 52x37.40625] + InlinePaintable (InlineNode.nowrap) + TextPaintable (TextNode<#text>) + InlinePaintable (InlineNode) + InlinePaintable (InlineNode.nowrap) + TextPaintable (TextNode<#text>) diff --git a/Tests/LibWeb/Layout/expected/table/line-breaking-in-cells.txt b/Tests/LibWeb/Layout/expected/table/line-breaking-in-cells.txt new file mode 100644 index 00000000000..38e8287ce3e --- /dev/null +++ b/Tests/LibWeb/Layout/expected/table/line-breaking-in-cells.txt @@ -0,0 +1,58 @@ +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x600 [BFC] children: not-inline + BlockContainer at (8,8) content-size 784x39.40625 children: not-inline + TableWrapper <(anonymous)> at (8,8) content-size 61x39.40625 [BFC] children: not-inline + Box at (8,8) content-size 61x39.40625 table-box [TFC] children: not-inline + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + Box at (8,8) content-size 61x39.40625 table-row-group children: not-inline + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + Box at (8,8) content-size 61x39.40625 table-row children: not-inline + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer
at (10,18.96875) content-size 14.296875x17.46875 table-cell [BFC] children: inline + line 0 width: 14.265625, height: 17.46875, bottom: 17.46875, baseline: 13.53125 + frag 0 from TextNode start: 0, length: 1, rect: [10,18.96875 14.265625x17.46875] + "A" + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer at (28.296875,10) content-size 20.40625x35.40625 table-cell [BFC] children: inline + line 0 width: 9.34375, height: 17.46875, bottom: 17.46875, baseline: 13.53125 + frag 0 from TextNode start: 1, length: 1, rect: [28.296875,10 9.34375x17.46875] + "B" + line 1 width: 10.3125, height: 17.9375, bottom: 35.40625, baseline: 13.53125 + frag 0 from TextNode start: 0, length: 1, rect: [28.296875,27 10.3125x17.46875] + "C" + TextNode <#text> + BreakNode
+ TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer 		at (52.703125,18.96875) content-size 14.296875x17.46875 table-cell [BFC] children: inline + line 0 width: 11.140625, height: 17.46875, bottom: 17.46875, baseline: 13.53125 + frag 0 from TextNode start: 1, length: 1, rect: [52.703125,18.96875 11.140625x17.46875] + "D" + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + BlockContainer <(anonymous)> (not painted) children: inline + TextNode <#text> + +ViewportPaintable (Viewport<#document>) [0,0 800x600] + PaintableWithLines (BlockContainer) [0,0 800x600] + PaintableWithLines (BlockContainer) [8,8 784x39.40625] + PaintableWithLines (TableWrapper(anonymous)) [8,8 61x39.40625] + PaintableBox (Box) [8,8 61x39.40625] + PaintableBox (Box) [8,8 61x39.40625] + PaintableBox (Box) [8,8 61x39.40625] + PaintableWithLines (BlockContainer
) [8,8 18.296875x39.40625] + TextPaintable (TextNode<#text>) + PaintableWithLines (BlockContainer) [26.296875,8 24.40625x39.40625] + TextPaintable (TextNode<#text>) + TextPaintable (TextNode<#text>) + PaintableWithLines (BlockContainer) [50.703125,8 18.296875x39.40625] + TextPaintable (TextNode<#text>) diff --git a/Tests/LibWeb/Layout/input/nowrap-and-no-line-break-opportunity.html b/Tests/LibWeb/Layout/input/nowrap-and-no-line-break-opportunity.html new file mode 100644 index 00000000000..38956595532 --- /dev/null +++ b/Tests/LibWeb/Layout/input/nowrap-and-no-line-break-opportunity.html @@ -0,0 +1,13 @@ + +
+ ABCXABC +
\ No newline at end of file diff --git a/Tests/LibWeb/Layout/input/space-is-soft-line-break-opportunity.html b/Tests/LibWeb/Layout/input/space-is-soft-line-break-opportunity.html new file mode 100644 index 00000000000..f6831a013a6 --- /dev/null +++ b/Tests/LibWeb/Layout/input/space-is-soft-line-break-opportunity.html @@ -0,0 +1,13 @@ + +
+ ABC ABC +
\ No newline at end of file diff --git a/Tests/LibWeb/Layout/input/table/line-breaking-in-cells.html b/Tests/LibWeb/Layout/input/table/line-breaking-in-cells.html new file mode 100644 index 00000000000..b9cb8242afa --- /dev/null +++ b/Tests/LibWeb/Layout/input/table/line-breaking-in-cells.html @@ -0,0 +1,25 @@ + + + + + + + + + + +
A + + B +
C + 		+ D +
\ No newline at end of file diff --git a/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp b/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp index ef211d7efd2..1a544056f43 100644 --- a/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp +++ b/Userland/Libraries/LibWeb/Layout/InlineFormattingContext.cpp @@ -250,8 +250,14 @@ void InlineFormattingContext::generate_line_boxes(LayoutMode layout_mode) auto& item = item_opt.value(); // Ignore collapsible whitespace chunks at the start of line, and if the last fragment already ends in whitespace. - if (item.is_collapsible_whitespace && (line_boxes.is_empty() || line_boxes.last().is_empty_or_ends_in_whitespace())) + if (item.is_collapsible_whitespace && (line_boxes.is_empty() || line_boxes.last().is_empty_or_ends_in_whitespace())) { + if (item.node->computed_values().white_space() != CSS::WhiteSpace::Nowrap) { + auto next_width = iterator.next_non_whitespace_sequence_width(); + if (next_width > 0) + line_builder.break_if_needed(next_width); + } continue; + } switch (item.type) { case InlineLevelIterator::Item::Type::ForcedBreak: { @@ -287,17 +293,24 @@ void InlineFormattingContext::generate_line_boxes(LayoutMode layout_mode) case InlineLevelIterator::Item::Type::Text: { auto& text_node = verify_cast(*item.node); - if (text_node.computed_values().white_space() != CSS::WhiteSpace::Nowrap && line_builder.break_if_needed(item.border_box_width())) { + if (text_node.computed_values().white_space() != CSS::WhiteSpace::Nowrap) { + bool is_whitespace = false; + CSSPixels next_width = 0; + // If we're in a whitespace-collapsing context, we can simply check the flag. + if (item.is_collapsible_whitespace) { + is_whitespace = true; + next_width = iterator.next_non_whitespace_sequence_width(); + } else { + // In whitespace-preserving contexts (white-space: pre*), we have to check manually. + auto view = text_node.text_for_rendering().substring_view(item.offset_in_node, item.length_in_node); + is_whitespace = view.is_whitespace(); + if (is_whitespace) + next_width = iterator.next_non_whitespace_sequence_width(); + } + // If whitespace caused us to break, we swallow the whitespace instead of // putting it on the next line. - - // If we're in a whitespace-collapsing context, we can simply check the flag. - if (item.is_collapsible_whitespace) - break; - - // In whitespace-preserving contexts (white-space: pre*), we have to check manually. - auto view = text_node.text_for_rendering().substring_view(item.offset_in_node, item.length_in_node); - if (view.is_whitespace()) + if (is_whitespace && next_width > 0 && line_builder.break_if_needed(item.border_box_width() + next_width)) break; } line_builder.append_text_chunk( diff --git a/Userland/Libraries/LibWeb/Layout/InlineLevelIterator.cpp b/Userland/Libraries/LibWeb/Layout/InlineLevelIterator.cpp index 368df002477..daaf49664c2 100644 --- a/Userland/Libraries/LibWeb/Layout/InlineLevelIterator.cpp +++ b/Userland/Libraries/LibWeb/Layout/InlineLevelIterator.cpp @@ -125,6 +125,37 @@ void InlineLevelIterator::skip_to_next() } Optional InlineLevelIterator::next() +{ + if (m_lookahead_items.is_empty()) + return next_without_lookahead(); + return m_lookahead_items.dequeue(); +} + +CSSPixels InlineLevelIterator::next_non_whitespace_sequence_width() +{ + CSSPixels next_width = 0; + for (;;) { + auto next_item_opt = next_without_lookahead(); + if (!next_item_opt.has_value()) + break; + m_lookahead_items.enqueue(next_item_opt.release_value()); + auto& next_item = m_lookahead_items.tail(); + if (next_item.node->computed_values().white_space() != CSS::WhiteSpace::Nowrap) { + if (next_item.type != InlineLevelIterator::Item::Type::Text) + break; + if (next_item.is_collapsible_whitespace) + break; + auto& next_text_node = verify_cast(*(next_item.node)); + auto next_view = next_text_node.text_for_rendering().substring_view(next_item.offset_in_node, next_item.length_in_node); + if (next_view.is_whitespace()) + break; + } + next_width += next_item.border_box_width(); + } + return next_width; +} + +Optional InlineLevelIterator::next_without_lookahead() { if (!m_current_node) return {}; @@ -139,7 +170,7 @@ Optional InlineLevelIterator::next() if (!chunk_opt.has_value()) { m_text_node_context = {}; skip_to_next(); - return next(); + return next_without_lookahead(); } m_text_node_context->next_chunk = m_text_node_context->chunk_iterator.next(); @@ -200,12 +231,12 @@ Optional InlineLevelIterator::next() if (is(*m_current_node)) { skip_to_next(); - return next(); + return next_without_lookahead(); } if (!is(*m_current_node)) { skip_to_next(); - return next(); + return next_without_lookahead(); } if (is(*m_current_node)) { diff --git a/Userland/Libraries/LibWeb/Layout/InlineLevelIterator.h b/Userland/Libraries/LibWeb/Layout/InlineLevelIterator.h index a8f15f48fb3..0f194923c04 100644 --- a/Userland/Libraries/LibWeb/Layout/InlineLevelIterator.h +++ b/Userland/Libraries/LibWeb/Layout/InlineLevelIterator.h @@ -52,8 +52,10 @@ public: InlineLevelIterator(Layout::InlineFormattingContext&, LayoutState&, Layout::BlockContainer const&, LayoutMode); Optional next(); + CSSPixels next_non_whitespace_sequence_width(); private: + Optional next_without_lookahead(); void skip_to_next(); void compute_next(); @@ -96,6 +98,7 @@ private: Optional m_extra_trailing_metrics; Vector> m_box_model_node_stack; + Queue m_lookahead_items; }; }