layout: Map document selection offsets across inline formatting context text transformations - #46864
Merged
Conversation
…xt text transformations This change adds an `OffsetMap` type which is used to map DOM text offsets to post-text transformation offsets during inline formatting context construction. This is necessary to properly display selections across text that is modified due to white space collapsing or CSS `text-transform` processing. Signed-off-by: Martin Robinson <mrobinson@igalia.com> Co-authored-by: Simon Sapin <simon@igalia.com>
Signed-off-by: Simon Sapin <simon@igalia.com>
Signed-off-by: Simon Sapin <simon@igalia.com>
Signed-off-by: Simon Sapin <simon@igalia.com>
Signed-off-by: Simon Sapin <simon@igalia.com>
It’s always the same input type, no need to make it `Box<dyn …>` Signed-off-by: Simon Sapin <simon@igalia.com>
Signed-off-by: Simon Sapin <simon@igalia.com>
Signed-off-by: Simon Sapin <simon@igalia.com>
This enables binary search instead of linear scan, and reverse mapping Signed-off-by: Simon Sapin <simon@igalia.com>
Signed-off-by: Simon Sapin <simon@igalia.com>
Signed-off-by: Simon Sapin <simon@igalia.com>
Signed-off-by: Simon Sapin <simon@igalia.com>
Signed-off-by: Simon Sapin <simon@igalia.com>
|
🔨 Triggering try run (#30469831136) for Linux (WPT) |
|
Instead of a semantic representation have `CharacterTransformIteration` be a bit more generic. Also do a few minor cleanups: - Remove `InlineFormattingContextBuilder::current_original_character_offset`. This can just be `OffsetMap::current_original_character_offset`- - Reorder some things so that they are gruoped together:wq logically Signed-off-by: Martin Robinson <mrobinson@igalia.com>
SimonSapin
reviewed
Jul 29, 2026
Comment on lines
+35
to
+36
| /// The character that were produced during this iteration. | ||
| characters: [char; 3], |
Member
There was a problem hiding this comment.
Let’s add a comment that only the self.characters[..self.produced.0] slice should be considered.
Alternatively, replace both fields with an ArrayVec (external crate but already in our dependencies) and use its .len() wherever produced is needed
Comment on lines
+43
to
+47
| let mut characters = ['\0'; 3]; | ||
| let mut produced = 0; | ||
| for (array_entry, character) in characters.iter_mut().zip(iterator) { | ||
| *array_entry = character; | ||
| produced += 1; |
Member
There was a problem hiding this comment.
With an ArrayVec this would be .collect()
Comment on lines
+68
to
+69
| produced: Utf32CodeUnits(if character.is_some() { 1 } else { 0 }), | ||
| characters: [character.unwrap_or_default(), '\0', '\0'], |
Member
There was a problem hiding this comment.
With ArrayVec: option.into_iter().collect()
Comment on lines
77
to
85
| pub(crate) fn each_char(self, mut each: impl FnMut(char)) { | ||
| match self { | ||
| CharacterTransformIteration::OneToOne(c) => each(c), | ||
| CharacterTransformIteration::WhitespaceCharsCollapsedToOneSpace(_) => each(' '), | ||
| CharacterTransformIteration::WhitespaceCharsCollapsedToOneNewline(_) => each('\n'), | ||
| CharacterTransformIteration::WhitespaceCharsCollapsedToNothing(_) => {}, | ||
| CharacterTransformIteration::ToLowercase(iter) => iter.for_each(each), | ||
| CharacterTransformIteration::ToUppercase(iter) => iter.for_each(each), | ||
| CharacterTransformIteration::ToTitlecase(iter) => iter.for_each(each), | ||
| for character in &self.characters[..self.produced.0] { | ||
| each(*character) | ||
| } | ||
| } | ||
|
|
||
| pub fn push_chars_to(self, string: &mut String) { | ||
| pub(crate) fn push_chars_to(self, string: &mut String) { | ||
| self.each_char(|character| string.push(character)); | ||
| } |
Member
There was a problem hiding this comment.
Now that we no longer carry multiple iterator types, let’s replace these two push-based methods with one that returns &[char] (or no method at all, if the field is ArrayVec)
| } else { | ||
| output.push(iteration); | ||
| *iteration = | ||
| CharacterTransformIteration::case_mapped(to_titlecase(iteration.characters[0])); |
Member
There was a problem hiding this comment.
If we’re back to a single use of to_titlecase let’s inline it
|
Test results for linux-wpt from try job (#30469831136): Flaky unexpected result (41)
Stable unexpected results that are known to be intermittent (21)
|
|
✨ Try run (#30469831136) succeeded. |
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Signed-off-by: Simon Sapin <simon@igalia.com>
Signed-off-by: Simon Sapin <simon@igalia.com>
SimonSapin
approved these changes
Jul 29, 2026
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
mrobinson
enabled auto-merge
July 30, 2026 09:22
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change adds an
OffsetMaptype which is used to map DOM textoffsets to post-text transformation offsets during inline formatting
context construction. This is necessary to properly display selections
across text that is modified due to white space collapsing or CSS
text-transformprocessing.This change fixes on test case in WPT dealing with
text-transformas a side effect.
Testing: This change updates WPT tests results.