fix: count display units as UTF-16 code units on macOS#583
Merged
Conversation
display_len/ch_width define the "display unit" for all document
offsets the core hands to the UI (tables, separators, navigation
targets), so they must match the expectations of the platform text
control those offsets are fed into via SetInsertionPoint/SetSelection.
Certain text controls (GTK) use Unicode codepoints as their notion
of a character / "display unit". Those are equivalent to Rust
chars. Other platforms (Windows and MacOS) use UTF-16 code units.
Higher codepoints (those from Unicode planes 1-16, also called Astral
or BMP, notably including Emoji), must be expressed with 2 UTF-16
code units.
On macOS, the text control is backed by NSTextView, and wxWidgets passes
NSRange values through unconverted (wxNSTextViewControl::GetSelection/
SetSelection in src/osx/cocoa/textctrl.mm; GetLastPosition is
[NSString length]), so positions are UTF-16 code units exactly as on
Windows. Verified three ways: by reading the vendored wxWidgets source
through every layer including the wxdragon passthrough,
empirically with a throwaway wxdragon probe on macOS —
get_last_position of "A\u{1D11E}B" returns 4 (UTF-16), not 3 (chars),
and with a human test, which indeed showed that navigation commands
move to the wrong document position if Emojis are present and
this patch is not applied.
Counting chars() on macOS therefore made every offset after an astral
character (emoji, musical symbols, some CJK) undershoot by one unit
per such character. Windows and macOS now share the UTF-16 arm; GTK,
which indexes GtkTextIter offsets by Unicode characters, keeps
chars().
This also makes the two {xml,html}_table_display_length tests pass
unmodified on macOS: their hardcoded expectations asserted the
correct UTF-16 semantics all along, and the macOS implementation was
the bug.
Side finding, not addressed here: wxTextEntry::GetStringSelection on
macOS mixes UTF-16 positions into char-indexed wxString::Mid, so it
returns wrong text after astral characters (upstream wx bug). The
Android app similarly indexes Kotlin strings (UTF-16) with core
offsets, which remain char-based on Android.
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
2 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.
display_len/ch_width define the "display unit" for all document offsets the core hands to the UI (tables, separators, navigation targets), so they must match the expectations of the platform text control those offsets are fed into via SetInsertionPoint/SetSelection.
Certain text controls (GTK) use Unicode codepoints as their notion of a character / "display unit". Those are equivalent to Rust chars. Other platforms (Windows and MacOS) use UTF-16 code units.
Higher codepoints (those from Unicode planes 1-16, also called Astral or BMP, notably including Emoji), must be expressed with 2 UTF-16 code units.
On macOS, the text control is backed by NSTextView, and wxWidgets passes NSRange values through unconverted (wxNSTextViewControl::GetSelection/ SetSelection in src/osx/cocoa/textctrl.mm; GetLastPosition is [NSString length]), so positions are UTF-16 code units exactly as on Windows. Verified three ways: by reading the vendored wxWidgets source through every layer including the wxdragon passthrough, empirically with a throwaway wxdragon probe on macOS — get_last_position of "A\u{1D11E}B" returns 4 (UTF-16), not 3 (chars), and with a human test, which indeed showed that navigation commands move to the wrong document position if Emojis are present and this patch is not applied.
Counting chars() on macOS therefore made every offset after an astral character (emoji, musical symbols, some CJK) undershoot by one unit per such character. Windows and macOS now share the UTF-16 arm; GTK, which indexes GtkTextIter offsets by Unicode characters, keeps chars().
This also makes the two {xml,html}_table_display_length tests pass unmodified on macOS: their hardcoded expectations asserted the correct UTF-16 semantics all along, and the macOS implementation was the bug.
Side finding, not addressed here: wxTextEntry::GetStringSelection on macOS mixes UTF-16 positions into char-indexed wxString::Mid, so it returns wrong text after astral characters (upstream wx bug). The Android app similarly indexes Kotlin strings (UTF-16) with core offsets, which remain char-based on Android.