From e28bfe5c9c065cae6aa2edcd8a6bf0dde1c318f4 Mon Sep 17 00:00:00 2001 From: Oriol Brufau Date: Thu, 14 Mar 2024 10:53:38 +0100 Subject: [PATCH] Obey white-space when intrinsically sizing an IFC The old logic was assuming that all whitespace was a break opportunity, and that no newlines would be preserved. Note that text shaping considers the advance of a newline to be the same as a space. This was problematic because if we have a segment with a preserved space and newline, only the advance of the space should contrinute to the size of the block container. Therefore, I'm changing the breaker logic in other to have newline characters in their own segment. Then glyph_run_is_whitespace_ending_with_preserved_newline can just be renamed to glyph_run_is_preserved_newline. --- components/gfx/text/text_run.rs | 46 ++++++++++++------- components/layout_2020/flow/inline.rs | 42 +++++++++-------- components/layout_2020/flow/text_run.rs | 11 ++--- .../generated-content/content-175.xht.ini | 2 - .../table-anonymous-objects-009.xht.ini | 2 - .../table-anonymous-objects-010.xht.ini | 2 - .../table-anonymous-objects-011.xht.ini | 2 - .../table-anonymous-objects-012.xht.ini | 2 - .../table-anonymous-objects-197.xht.ini | 2 - .../table-anonymous-objects-198.xht.ini | 2 - .../css/CSS2/text/white-space-pre-001.xht.ini | 2 - .../meta/css/css-lists/inline-list.html.ini | 2 - .../tab-size/tab-size-integer-001.html.ini | 2 + .../tab-size/tab-size-integer-002.html.ini | 2 + .../tab-size/tab-size-integer-003.html.ini | 2 + .../tab-size/tab-size-length-001.html.ini | 2 + .../tab-size/tab-size-length-002.html.ini | 2 + .../tab-size/tab-size-percent-001.html.ini | 2 + .../white-space-intrinsic-size-015.html.ini | 2 - .../white-space-intrinsic-size-016.html.ini | 2 - .../white-space-intrinsic-size-018.html.ini | 2 - .../meta/css/white-space-pre-wrap.htm.ini | 2 + .../white_space_intrinsic_sizes_a.html.ini | 2 - 23 files changed, 70 insertions(+), 69 deletions(-) delete mode 100644 tests/wpt/meta/css/CSS2/generated-content/content-175.xht.ini delete mode 100644 tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-009.xht.ini delete mode 100644 tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-010.xht.ini delete mode 100644 tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-011.xht.ini delete mode 100644 tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-012.xht.ini delete mode 100644 tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-197.xht.ini delete mode 100644 tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-198.xht.ini delete mode 100644 tests/wpt/meta/css/CSS2/text/white-space-pre-001.xht.ini delete mode 100644 tests/wpt/meta/css/css-lists/inline-list.html.ini create mode 100644 tests/wpt/meta/css/css-text/tab-size/tab-size-integer-001.html.ini create mode 100644 tests/wpt/meta/css/css-text/tab-size/tab-size-integer-002.html.ini create mode 100644 tests/wpt/meta/css/css-text/tab-size/tab-size-integer-003.html.ini create mode 100644 tests/wpt/meta/css/css-text/tab-size/tab-size-length-001.html.ini create mode 100644 tests/wpt/meta/css/css-text/tab-size/tab-size-length-002.html.ini create mode 100644 tests/wpt/meta/css/css-text/tab-size/tab-size-percent-001.html.ini delete mode 100644 tests/wpt/meta/css/css-text/white-space/white-space-intrinsic-size-015.html.ini delete mode 100644 tests/wpt/meta/css/css-text/white-space/white-space-intrinsic-size-016.html.ini delete mode 100644 tests/wpt/meta/css/css-text/white-space/white-space-intrinsic-size-018.html.ini create mode 100644 tests/wpt/mozilla/meta/css/white-space-pre-wrap.htm.ini delete mode 100644 tests/wpt/mozilla/meta/css/white_space_intrinsic_sizes_a.html.ini diff --git a/components/gfx/text/text_run.rs b/components/gfx/text/text_run.rs index 21c239257fb00..a40d82387a99a 100644 --- a/components/gfx/text/text_run.rs +++ b/components/gfx/text/text_run.rs @@ -225,6 +225,16 @@ impl<'a> TextRun { let breaker = breaker.as_mut().unwrap(); + let mut push_range = |range: &std::ops::Range, options: &ShapingOptions| { + glyphs.push(GlyphRun { + glyph_store: font.shape_text(&text[range.clone()], &options), + range: Range::new( + ByteIndex(range.start as isize), + ByteIndex(range.len() as isize), + ), + }); + }; + while !finished { let (idx, _is_hard_break) = breaker.next(text); if idx == text.len() { @@ -240,9 +250,9 @@ impl<'a> TextRun { // Split off any trailing whitespace into a separate glyph run. let mut whitespace = slice.end..slice.end; - if let Some((i, _)) = word - .char_indices() - .rev() + let mut rev_char_indices = word.char_indices().rev().peekable(); + let ends_with_newline = rev_char_indices.peek().map_or(false, |&(_, c)| c == '\n'); + if let Some((i, _)) = rev_char_indices .take_while(|&(_, c)| char_is_whitespace(c)) .last() { @@ -254,26 +264,28 @@ impl<'a> TextRun { continue; } if !slice.is_empty() { - glyphs.push(GlyphRun { - glyph_store: font.shape_text(&text[slice.clone()], options), - range: Range::new( - ByteIndex(slice.start as isize), - ByteIndex(slice.len() as isize), - ), - }); + push_range(&slice, options); } if !whitespace.is_empty() { let mut options = *options; options .flags .insert(ShapingFlags::IS_WHITESPACE_SHAPING_FLAG); - glyphs.push(GlyphRun { - glyph_store: font.shape_text(&text[whitespace.clone()], &options), - range: Range::new( - ByteIndex(whitespace.start as isize), - ByteIndex(whitespace.len() as isize), - ), - }); + + // The breaker breaks after every newline, so either there is none, + // or there is exactly one at the very end. In the latter case, + // split it into a different run. That's because shaping considers + // a newline to have the same advance as a space, but during layout + // we want to treat the newline as having no advance. + if ends_with_newline { + whitespace.end -= 1; + if !whitespace.is_empty() { + push_range(&whitespace, &options); + } + whitespace.start = whitespace.end; + whitespace.end += 1; + } + push_range(&whitespace, &options); } slice.start = whitespace.end; } diff --git a/components/layout_2020/flow/inline.rs b/components/layout_2020/flow/inline.rs index 11b8f4a5ce5db..dddfb1617a53f 100644 --- a/components/layout_2020/flow/inline.rs +++ b/components/layout_2020/flow/inline.rs @@ -2248,8 +2248,8 @@ struct ContentSizesComputation<'a> { current_line: ContentSizes, /// Size for whitepsace pending to be added to this line. pending_whitespace: Au, - /// Whether or not this IFC has seen any non-whitespace content. - had_non_whitespace_content_yet: bool, + /// Whether or not this IFC has seen any content, excluding discarded whitespace. + had_content_yet: bool, /// Stack of ending padding, margin, and border to add to the length /// when an inline box finishes. ending_inline_pbm_stack: Vec, @@ -2304,31 +2304,35 @@ impl<'a> ContentSizesComputation<'a> { for run in segment.runs.iter() { let advance = run.glyph_store.total_advance(); - if !run.glyph_store.is_whitespace() { - self.had_non_whitespace_content_yet = true; - self.current_line.min_content += advance; - self.current_line.max_content += self.pending_whitespace + advance; - self.pending_whitespace = Au::zero(); - } else { + if run.glyph_store.is_whitespace() { // If this run is a forced line break, we *must* break the line // and start measuring from the inline origin once more. - if text_run.glyph_run_is_whitespace_ending_with_preserved_newline(run) { - self.had_non_whitespace_content_yet = true; + if text_run.glyph_run_is_preserved_newline(run) { + self.had_content_yet = true; self.forced_line_break(); self.current_line = ContentSizes::zero(); continue; } - // Discard any leading whitespace in the IFC. This will always be trimmed. - if !self.had_non_whitespace_content_yet { + let white_space = + text_run.parent_style.get_inherited_text().white_space; + if !self.had_content_yet && !white_space.preserve_spaces() { + // Discard any leading whitespace in the IFC. This will always be trimmed. + continue; + } + if white_space.allow_wrap() { + // Wait to take into account other whitespace until we see more content. + // Whitespace at the end of the IFC will always be trimmed. + self.line_break_opportunity(); + self.pending_whitespace += advance; continue; } - - // Wait to take into account other whitespace until we see more content. - // Whitespace at the end of the IFC will always be trimmed. - self.line_break_opportunity(); - self.pending_whitespace += advance; } + + self.had_content_yet = true; + self.current_line.min_content += advance; + self.current_line.max_content += self.pending_whitespace + advance; + self.pending_whitespace = Au::zero(); } } }, @@ -2341,7 +2345,7 @@ impl<'a> ContentSizesComputation<'a> { self.current_line.min_content += self.pending_whitespace + outer.min_content; self.current_line.max_content += self.pending_whitespace + outer.max_content; self.pending_whitespace = Au::zero(); - self.had_non_whitespace_content_yet = true; + self.had_content_yet = true; }, _ => {}, }); @@ -2380,7 +2384,7 @@ impl<'a> ContentSizesComputation<'a> { paragraph: ContentSizes::zero(), current_line: ContentSizes::zero(), pending_whitespace: Au::zero(), - had_non_whitespace_content_yet: false, + had_content_yet: false, ending_inline_pbm_stack: Vec::new(), } .traverse(inline_formatting_context) diff --git a/components/layout_2020/flow/text_run.rs b/components/layout_2020/flow/text_run.rs index 2667c474d9ed0..d072124608c1b 100644 --- a/components/layout_2020/flow/text_run.rs +++ b/components/layout_2020/flow/text_run.rs @@ -150,7 +150,7 @@ impl TextRunSegment { // If this whitespace forces a line break, queue up a hard line break the next time we // see any content. We don't line break immediately, because we'd like to finish processing // any ongoing inline boxes before ending the line. - if text_run.glyph_run_is_whitespace_ending_with_preserved_newline(run) { + if text_run.glyph_run_is_preserved_newline(run) { ifc.defer_forced_line_break(); continue; } @@ -426,11 +426,8 @@ impl TextRun { self.prevent_soft_wrap_opportunity_at_end; } - pub(super) fn glyph_run_is_whitespace_ending_with_preserved_newline( - &self, - run: &GlyphRun, - ) -> bool { - if !run.glyph_store.is_whitespace() { + pub(super) fn glyph_run_is_preserved_newline(&self, run: &GlyphRun) -> bool { + if !run.glyph_store.is_whitespace() || run.range.length() != ByteIndex(1) { return false; } if !self @@ -442,7 +439,7 @@ impl TextRun { return false; } - let last_byte = self.text.as_bytes().get(run.range.end().to_usize() - 1); + let last_byte = self.text.as_bytes().get(run.range.begin().to_usize()); last_byte == Some(&b'\n') } } diff --git a/tests/wpt/meta/css/CSS2/generated-content/content-175.xht.ini b/tests/wpt/meta/css/CSS2/generated-content/content-175.xht.ini deleted file mode 100644 index 6d8d2bb07d3fb..0000000000000 --- a/tests/wpt/meta/css/CSS2/generated-content/content-175.xht.ini +++ /dev/null @@ -1,2 +0,0 @@ -[content-175.xht] - expected: FAIL diff --git a/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-009.xht.ini b/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-009.xht.ini deleted file mode 100644 index 91d95f9df4755..0000000000000 --- a/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-009.xht.ini +++ /dev/null @@ -1,2 +0,0 @@ -[table-anonymous-objects-009.xht] - expected: FAIL diff --git a/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-010.xht.ini b/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-010.xht.ini deleted file mode 100644 index 12c3a9c74b3b3..0000000000000 --- a/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-010.xht.ini +++ /dev/null @@ -1,2 +0,0 @@ -[table-anonymous-objects-010.xht] - expected: FAIL diff --git a/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-011.xht.ini b/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-011.xht.ini deleted file mode 100644 index d230ae4bafbf1..0000000000000 --- a/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-011.xht.ini +++ /dev/null @@ -1,2 +0,0 @@ -[table-anonymous-objects-011.xht] - expected: FAIL diff --git a/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-012.xht.ini b/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-012.xht.ini deleted file mode 100644 index e08d936cda1f1..0000000000000 --- a/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-012.xht.ini +++ /dev/null @@ -1,2 +0,0 @@ -[table-anonymous-objects-012.xht] - expected: FAIL diff --git a/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-197.xht.ini b/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-197.xht.ini deleted file mode 100644 index fda41d32144e5..0000000000000 --- a/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-197.xht.ini +++ /dev/null @@ -1,2 +0,0 @@ -[table-anonymous-objects-197.xht] - expected: FAIL diff --git a/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-198.xht.ini b/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-198.xht.ini deleted file mode 100644 index 98c69fb5cd9fe..0000000000000 --- a/tests/wpt/meta/css/CSS2/tables/table-anonymous-objects-198.xht.ini +++ /dev/null @@ -1,2 +0,0 @@ -[table-anonymous-objects-198.xht] - expected: FAIL diff --git a/tests/wpt/meta/css/CSS2/text/white-space-pre-001.xht.ini b/tests/wpt/meta/css/CSS2/text/white-space-pre-001.xht.ini deleted file mode 100644 index 8e2d4b13074ba..0000000000000 --- a/tests/wpt/meta/css/CSS2/text/white-space-pre-001.xht.ini +++ /dev/null @@ -1,2 +0,0 @@ -[white-space-pre-001.xht] - expected: FAIL diff --git a/tests/wpt/meta/css/css-lists/inline-list.html.ini b/tests/wpt/meta/css/css-lists/inline-list.html.ini deleted file mode 100644 index ebcb4fec824f9..0000000000000 --- a/tests/wpt/meta/css/css-lists/inline-list.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[inline-list.html] - expected: FAIL diff --git a/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-001.html.ini b/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-001.html.ini new file mode 100644 index 0000000000000..37c1a524220e8 --- /dev/null +++ b/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-001.html.ini @@ -0,0 +1,2 @@ +[tab-size-integer-001.html] + expected: FAIL diff --git a/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-002.html.ini b/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-002.html.ini new file mode 100644 index 0000000000000..066c0b60bdc76 --- /dev/null +++ b/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-002.html.ini @@ -0,0 +1,2 @@ +[tab-size-integer-002.html] + expected: FAIL diff --git a/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-003.html.ini b/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-003.html.ini new file mode 100644 index 0000000000000..0c6fd1eff9d98 --- /dev/null +++ b/tests/wpt/meta/css/css-text/tab-size/tab-size-integer-003.html.ini @@ -0,0 +1,2 @@ +[tab-size-integer-003.html] + expected: FAIL diff --git a/tests/wpt/meta/css/css-text/tab-size/tab-size-length-001.html.ini b/tests/wpt/meta/css/css-text/tab-size/tab-size-length-001.html.ini new file mode 100644 index 0000000000000..81b7b1df8c751 --- /dev/null +++ b/tests/wpt/meta/css/css-text/tab-size/tab-size-length-001.html.ini @@ -0,0 +1,2 @@ +[tab-size-length-001.html] + expected: FAIL diff --git a/tests/wpt/meta/css/css-text/tab-size/tab-size-length-002.html.ini b/tests/wpt/meta/css/css-text/tab-size/tab-size-length-002.html.ini new file mode 100644 index 0000000000000..86cf5cd341eae --- /dev/null +++ b/tests/wpt/meta/css/css-text/tab-size/tab-size-length-002.html.ini @@ -0,0 +1,2 @@ +[tab-size-length-002.html] + expected: FAIL diff --git a/tests/wpt/meta/css/css-text/tab-size/tab-size-percent-001.html.ini b/tests/wpt/meta/css/css-text/tab-size/tab-size-percent-001.html.ini new file mode 100644 index 0000000000000..5eb8b3b910198 --- /dev/null +++ b/tests/wpt/meta/css/css-text/tab-size/tab-size-percent-001.html.ini @@ -0,0 +1,2 @@ +[tab-size-percent-001.html] + expected: FAIL diff --git a/tests/wpt/meta/css/css-text/white-space/white-space-intrinsic-size-015.html.ini b/tests/wpt/meta/css/css-text/white-space/white-space-intrinsic-size-015.html.ini deleted file mode 100644 index 89bdfca9200b5..0000000000000 --- a/tests/wpt/meta/css/css-text/white-space/white-space-intrinsic-size-015.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[white-space-intrinsic-size-015.html] - expected: FAIL diff --git a/tests/wpt/meta/css/css-text/white-space/white-space-intrinsic-size-016.html.ini b/tests/wpt/meta/css/css-text/white-space/white-space-intrinsic-size-016.html.ini deleted file mode 100644 index cd83045f9ac8d..0000000000000 --- a/tests/wpt/meta/css/css-text/white-space/white-space-intrinsic-size-016.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[white-space-intrinsic-size-016.html] - expected: FAIL diff --git a/tests/wpt/meta/css/css-text/white-space/white-space-intrinsic-size-018.html.ini b/tests/wpt/meta/css/css-text/white-space/white-space-intrinsic-size-018.html.ini deleted file mode 100644 index 368a69e07d33d..0000000000000 --- a/tests/wpt/meta/css/css-text/white-space/white-space-intrinsic-size-018.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[white-space-intrinsic-size-018.html] - expected: FAIL diff --git a/tests/wpt/mozilla/meta/css/white-space-pre-wrap.htm.ini b/tests/wpt/mozilla/meta/css/white-space-pre-wrap.htm.ini new file mode 100644 index 0000000000000..521c793832b93 --- /dev/null +++ b/tests/wpt/mozilla/meta/css/white-space-pre-wrap.htm.ini @@ -0,0 +1,2 @@ +[white-space-pre-wrap.htm] + expected: FAIL diff --git a/tests/wpt/mozilla/meta/css/white_space_intrinsic_sizes_a.html.ini b/tests/wpt/mozilla/meta/css/white_space_intrinsic_sizes_a.html.ini deleted file mode 100644 index 1330b71867f4e..0000000000000 --- a/tests/wpt/mozilla/meta/css/white_space_intrinsic_sizes_a.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[white_space_intrinsic_sizes_a.html] - expected: FAIL