Skip to content

Add right-to-left (RTL) text support - #3049

Open
kfatehi wants to merge 5 commits into
BlueBubblesApp:masterfrom
kfatehi:rtl
Open

Add right-to-left (RTL) text support#3049
kfatehi wants to merge 5 commits into
BlueBubblesApp:masterfrom
kfatehi:rtl

Conversation

@kfatehi

@kfatehi kfatehi commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Adds right-to-left (RTL) text support for Farsi, Arabic, and Hebrew. Reimplemented on top of development (post-rewrite), per the maintainer note. Closes #3047.

What it does

  • Rendering: message bubbles, reply bubbles/previews, conversation-list titles/subtitles, and the send-animation bubble render with the correct paragraph direction (RTL text aligns right; trailing punctuation/emoji land correctly). Reaction/reply/notification text embedding an RTL message is wrapped in a Unicode first-strong isolate so it orders correctly in the surrounding sentence.
  • Input: the compose and subject fields go RTL (right-aligned, RTL base direction) as you type RTL text, via TextDirectionBuilder — which recomputes direction live but rebuilds the field only when the first-strong direction actually flips, not on every keystroke/selection change, so caret dragging works normally.
  • Emoji-corruption fix: prevents the ?? surrogate-pair corruption that RTL input could trigger (see "Emoji corruption" below).

How

  • getTextDirection() (lib/helpers/ui/text_direction_helpers.dart): UAX#9 "first strong character" detection over runes — chosen over intl's Bidi.startsWithRtl, which misclassifies emoji-leading text as LTR (the leading surrogate falls in its LTR ranges).
  • Rendering: applied as the textDirection of each message RichText/Text.rich and the conversation tile; getNotificationText wraps embedded text in a first-strong isolate (U+2068U+2069).
  • Input: each compose TextField is wrapped in TextDirectionBuilder (direction-flip-gated rebuild).
  • Caret clamp: SpellCheckTextEditingController.set value snaps a collapsed caret (or selection endpoint) off any UTF-16 surrogate-pair interior — snapSelectionOffSurrogatePairs() in lib/helpers/ui/grapheme_caret.dart — before committing the value, so a subsequent edit can never split an emoji. The caret snaps to after the cluster (the trailing edge), so when a tap lands inside a trailing emoji's glyph the caret ends up past it and a backspace deletes the emoji — in RTL, snapping to the leading edge would instead strand the caret before the emoji and delete the adjacent space.

Emoji corruption — fixed (this was the draft blocker)

Earlier this PR carried an intermittent ?? corruption while composing. With an RTL base direction, tapping on an emoji glyph can land the caret inside the emoji's UTF-16 surrogate pair; the next keystroke then splits the pair into two lone surrogates, which the Android text-input channel encodes as ? (giving ??) and which also crash ParagraphBuilder on paint.

This is a pre-existing Flutter framework bug, not something this PR's RTL approach does wrong: Flutter allows a caret to sit inside a surrogate pair and forwards that offset to the platform IME. RTL input is only the trigger — an LTR tap happens to snap the caret to the grapheme boundary, whereas an RTL tap can land it mid-pair; the standalone repro reproduces the defect with plain Latin text and no RTL at all. It's filed with a deterministic, device-free repro and fixed upstream:

This PR fixes it in-app, with no Flutter upgrade required: because EditableText forwards the controller's value to the IME, clamping the caret in our own compose controller is equivalent to the framework fix, and it's scoped to the compose field only (no risk to other fields). Verified on-device — Farsi + emoji + tap-on-the-emoji + typing no longer corrupts. The upstream Flutter PR is the clean long-term home for when the SDK pin bumps; until then the in-app clamp handles it. (The two are idempotent, so nothing conflicts if a future Flutter already carries the fix.)

Tests

The post-rewrite development branch has flutter_test commented out and no test/ directory, so this PR is kept to feature source only and adds no tests. The identical getTextDirection and snapSelectionOffSurrogatePairs logic is unit-tested in the OpenBubbles port, OpenBubbles#213 (whose base has flutter_test enabled), where the code is byte-for-byte compatible.

Note on mixed-direction text

Mixed LTR/RTL/emoji runs are laid out by Flutter's standard Unicode Bidi Algorithm — the same reordering iMessage and the platform keyboards apply for a given base direction. Intentional, not a rendering bug.

The same change is in the OpenBubbles fork: OpenBubbles#213.

@kfatehi
kfatehi marked this pull request as draft June 9, 2026 19:30
@kfatehi
kfatehi marked this pull request as ready for review June 10, 2026 00:08
@jjoelj
jjoelj changed the base branch from master to development June 24, 2026 17:23
@jjoelj

jjoelj commented Jun 24, 2026

Copy link
Copy Markdown
Member

We need this patch to be compatible with the post-rewrite state of the app in development

@kfatehi
kfatehi marked this pull request as draft June 24, 2026 17:51
@kfatehi

kfatehi commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Hi @jjoelj I was not aware of that branch. I'll look into it, thank you.

@kfatehi

kfatehi commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

There is also a bug I can't reliably repro but happens when I am using emojis where I can still manage to split/corrupt them into 2 question mark symbols. I think I'll tell AI to make a fuzzer or something and see if I can get this solid. Been using this ever since the PR and it's been perfect but when it happens it is frustrating.

@kfatehi

kfatehi commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Also once that corruption occurs it gets weirder still, like there is a big space between the cursor and the last word, when I make a space, if i delete the space, cursor goes to the correct place. I'll try to sort these bugs out.

@jjoelj

jjoelj commented Jun 24, 2026

Copy link
Copy Markdown
Member

Is this happening only on your branch?

@kfatehi

kfatehi commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

yeah only my branch.

i set both PRs (OpenBubbles and this one) to draft, and will figure it out this weekend :)

kfatehi added a commit to kfatehi/bluebubbles-app that referenced this pull request Jun 25, 2026
Reimplements PR BlueBubblesApp#3049's right-to-left support (Farsi/Arabic/Hebrew) on top
of `development` instead of the old master-based branch, per jjoelj's note.

- getTextDirection(): first-strong (UAX#9) direction detection over runes
  (lib/helpers/ui/text_direction_helpers.dart); applied as the textDirection
  of message bubbles, reply bubbles, conversation-list title/subtitle, and the
  send animation (no LTR flash on send).
- Compose + subject TextFields wrapped in TextDirectionBuilder so they flip
  direction live but rebuild only on an actual first-strong flip (fixes the
  cursor-drag-cancel bug).
- getNotificationText() wraps embedded message text in a Unicode First-Strong
  Isolate (U+2068..U+2069) so quoted RTL renders correctly inside LTR.
- Grapheme-cluster-safe editing (lib/helpers/ui/grapheme_text_helpers.dart):
  emoji-picker insert and the controller value-setter snap to cluster
  boundaries and repair IME-split surrogate pairs (the "??" corruption); a
  repaired edit also clears the now-stale composing region (the cursor-gap bug).
- Re-enables flutter_test; adds test/rtl_detection_test.dart and
  test/grapheme_text_helpers_test.dart (detection, boundary snapping,
  insert/replace fuzz, IME-split recovery, composing-clear).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YPSpEnnJze96m53nX9k142
@kfatehi
kfatehi marked this pull request as ready for review June 25, 2026 02:34
@kfatehi
kfatehi marked this pull request as draft June 26, 2026 09:00
@kfatehi

kfatehi commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Draft again because I got the ? ? corruption bug happen again but I have no idea about repro steps. I am going to need to add logging later and export a dump next time it happens. It's otherwise usable and good but not perfect apparently.

@kfatehi
kfatehi force-pushed the rtl branch 3 times, most recently from 748a1f6 to c2abd97 Compare June 28, 2026 22:45
@kfatehi kfatehi changed the title Add right-to-left (RTL) text support Add right-to-left (RTL) text support for message rendering Jun 29, 2026
@kfatehi
kfatehi marked this pull request as ready for review June 29, 2026 01:51
@kfatehi
kfatehi marked this pull request as draft June 29, 2026 02:00
Rendering: message bubbles, reply bubbles/previews, conversation tiles,
the send-animation bubble, and embedded notification/reaction text render
with the correct paragraph direction. Direction via getTextDirection()
(UAX#9 first-strong over runes); embedded text uses a first-strong isolate.

Input: the compose and subject fields go RTL via TextDirectionBuilder,
which rebuilds only when the first-strong direction flips (so caret
dragging works). No per-keystroke direction-forcing, no grapheme-repair.

Known issue (kept draft): intermittent emoji "??" corruption while
composing, not yet reliably reproduced.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kfatehi kfatehi changed the title Add right-to-left (RTL) text support for message rendering Add right-to-left (RTL) text support Jun 29, 2026
Snap a collapsed caret (or selection endpoint) off any UTF-16 surrogate-pair
interior in the compose controller (SpellCheckTextEditingController.set value,
both exit paths) before committing the value, so a subsequent edit can't split
an emoji into lone surrogates -- which the Android text-input channel encodes
as '?' (the '??' corruption) and which crash ParagraphBuilder on paint.

App-side equivalent of the framework fix in flutter/flutter#188713 (PR
flutter/flutter#188719); needs no Flutter upgrade and is scoped to the compose
field. Source-only per this branch's convention; logic unit-tested in the
OpenBubbles port (OpenBubbles#213).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kfatehi
kfatehi marked this pull request as ready for review June 29, 2026 08:34
@kfatehi

kfatehi commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Is this happening only on your branch?

Hi @jjoelj I am following up on "is this only on your branch?" — yes, but I dug in and it's not something this PR introduces. It's an upstream Flutter bug which is now approved flutter/flutter#188719

This actually fixed a crash on windows that I didn't even know existed. Obviously AI helped significantly by writing a program to hammer inputs until it figured out tapping the center of an emoji in RTL was the secret to repro all along (there are tons of issues over the years about this but nobody knew how to repro!)

The same fix works entirely in-app — no Flutter upgrade needed. Because
EditableText forwards the controller's value to the IME, clamping the caret in our own controller
is equivalent to the framework fix. I added it to SpellCheckTextEditingController.set value (the
compose field's controller, which every keystroke/tap/IME update funnels through): if the caret
would land between the two halves of a surrogate pair, it's snapped to the pair boundary before the
value is committed. It's a few lines + a small pure helper (snapSelectionOffSurrogatePairs) with a
unit test (tests are only in OB, since we dont have it enabled in BB, so it's stripped in this PR), and it's scoped to this field only — no risk to anything else in the app, and it works on
the current Flutter pin. So this PR can carry the corruption fix itself; the upstream Flutter PR is
just the clean long-term home for when the framework version bumps.

@kfatehi
kfatehi marked this pull request as draft June 29, 2026 09:08
The compose-field caret clamp snapped a caret that lands inside an emoji's
surrogate pair to the pair start (before the emoji). In RTL a tap aiming for
the spot after a trailing emoji lands mid-glyph and got yanked before it, so
backspace deleted the adjacent space instead of the emoji and the emoji could
not be removed.

Snap to the pair end (offset + 1) instead, so the caret lands after the emoji
and backspace deletes the whole emoji. It is still a boundary, so the next edit
cannot split the pair and the "??" corruption stays fixed. Verified on-device
(Android / Gboard, forced-RTL).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FrEin3bnyFrXaxSmL9iLQM
@kfatehi
kfatehi marked this pull request as ready for review June 29, 2026 10:36
@kfatehi

kfatehi commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

And 40a31ae is the same fix for caret position snapping as that which I shipped in the approved flutter patch. This PR is good to go. And finally proper RTL support 🥳 🍾

@zlshames

zlshames commented Jul 6, 2026

Copy link
Copy Markdown
Member

First off, thanks for the contribution! Second, does it make sense to instead make this a toggle rather than a dynamic detection? From my perspective, this adds some processing overhead whenever a user types a character. As messages get bigger, it's just added processing that may not be needed. I know it might be quick, but every little bit counts.

If we make it a settings toggle, it wont be as dynamic, but it will make things less "guess-y" and reduce some processing. Does this need to be something that's dynamic? I assume that if a user is using a different language, everything will be in that language? I'm just a little worried about overhead when this processing needs to run on each message bubble/text field.

@kfatehi

kfatehi commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Hi, thanks for paying attention to the PR. It was important to me. I've been improving my Farsi and got used to the proper handling in WhatsApp and iMessage, and this PR brings the Bubbles apps into parity with them.

On the cost: it's a single scan that stops at the first strongly-directional character: O(k) where k is that character's position, which is O(1) for any real message (a longer message doesn't move it). Worst case is O(n) only for a string with no directional content at all, and even then it's dominated by the O(n) Bidi pass the RichText already runs on every layout.

A toggle is fine by me, but it'd be a deviation from what's standard. iMessage and WhatsApp just auto-detect, no setting. Happy to add one if you want it; otherwise I'd lean toward keeping it automatic.

@zlshames

Copy link
Copy Markdown
Member

@kfatehi getTextDirection(part.fullText) is called directly inside RichText in text_bubble.dart, reply_bubble.dart, send_animation.dart, and the conversation tile. Every rebuild re-scans the string. The early-exit-on-first-strong-char behavior makes this cheap for typical text, but it's a full-length scan for messages that are all neutral characters (digits, punctuation, emoji-only), and it reruns on every Obx/setState rebuild, not just when text changes. I think it might be worth caching, especially for conversation_tile.dart's title/subtitle since list tiles rebuild frequently during scroll. As well as send_animation so it's not recalculated a bunch as the message animates.

@jjoelj
jjoelj changed the base branch from development to master July 27, 2026 03:16
@kfatehi

kfatehi commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Hi, sorry I haven't replied... it's because I've been using this heavily and I have found MORE bugs and edge cases and haven't had time to chase them down yet. I'll come back to this. It is likely an upstream flutter issue, so I guess we need to park this for now. I'll come back once I know more.

@jjoelj
jjoelj marked this pull request as draft July 31, 2026 19:35
getTextDirection is called from inside build in text_bubble.dart,
reply_bubble.dart, send_animation.dart and the conversation tile, so it re-runs
on every Obx/setState rebuild rather than only when the text changes (BlueBubblesApp#3049
review). Detection early-exits on the first strongly-directional character, so
cost tracks how far in that character is, not message length: ordinary text is
8-12 ns and a 60-tile frame doing 120 detections is 0.8 us, about 0.005% of a
16.7 ms frame. But text with no strong character anywhere is scanned to the
end -- an all-neutral 200-unit message is 683-874 ns and an emoji-only message
1008-1060 ns, 60-100x worse.

Memoize on the text itself. The detection moves unchanged into a private
_detectTextDirection and every call site is untouched. No invalidation
anywhere: the direction is a pure function of the text, so an entry cannot go
stale for its own key. After: all-neutral and emoji-only both drop to ~10 ns
flat, ordinary text is unchanged within noise. On a workload of only
early-exit strings the memo is neutral, and a miss costs a scan plus an insert,
so all-distinct strings are a small net loss -- bounded by the 512 cap.

Deliberately not an LRU. Promoting a key on every hit costs a remove plus a
re-insert, measured at 34.2 ns/hit against the 13.3 ns scan it replaces, which
would make the common case slower than having no cache at all. Insertion-order
eviction keeps the hit path to a single lookup (5.9-10.7 ns); a Dart map
literal is insertion-ordered, so keys.first is the oldest and eviction is O(1).
With a 512 cap and a 120-200 string working set both policies evict the same
keys anyway.

Keyed on the string value rather than identity, because Message.fullText and
MessagePart.fullText allocate a new String on every call. Hashing costs well
under 0.1 ns/unit against the scan's ~3.75 ns/unit, so a lookup on a fresh key
still beats a scan on a fresh key (42.0 vs 802.1 ns).

The helper is byte-identical to the OpenBubbles copy, where it is covered by
8 tests including an eviction test that turns red under a move-to-end LRU.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REhKAmLTibRLmoakxNp6CW
kfatehi added a commit to kfatehi/bluebubbles-app that referenced this pull request Aug 9, 2026
getTextDirection is called from inside build in the message bubble, the reply
bubble, the send animation and the conversation tile, so it re-runs on every
Obx/setState rebuild rather than only when the text changes (BlueBubbles BlueBubblesApp#3049
review). Detection early-exits on the first strongly-directional character, so
cost tracks how far in that character is, not message length: ordinary text is
8-12 ns and a 60-tile frame doing 120 detections is 0.8 us, about 0.005% of a
16.7 ms frame. But text with no strong character anywhere is scanned to the
end -- an all-neutral 200-unit message is 683-874 ns and an emoji-only message
1008-1060 ns, 60-100x worse.

Memoize on the text itself. The detection moves unchanged into a private
_detectTextDirection and every call site is untouched. No invalidation
anywhere: the direction is a pure function of the text, so an entry cannot go
stale for its own key. After: all-neutral and emoji-only both drop to ~10 ns
flat, ordinary text is unchanged within noise. On a workload of only
early-exit strings the memo is neutral, and a miss costs a scan plus an insert,
so all-distinct strings are a small net loss -- bounded by the 512 cap.

Deliberately not an LRU. Promoting a key on every hit costs a remove plus a
re-insert, measured at 34.2 ns/hit against the 13.3 ns scan it replaces, which
would make the common case slower than having no cache at all. Insertion-order
eviction keeps the hit path to a single lookup (5.9-10.7 ns); a Dart map
literal is insertion-ordered, so keys.first is the oldest and eviction is O(1).
With a 512 cap and a 120-200 string working set both policies evict the same
keys anyway.

Keyed on the string value rather than identity, because Message.fullText and
MessagePart.fullText allocate a new String on every call. Hashing costs well
under 0.1 ns/unit against the scan's ~3.75 ns/unit, so a lookup on a fresh key
still beats a scan on a fresh key (42.0 vs 802.1 ns).

Adds test/rtl_direction_cache_test.dart (8 tests). The eviction test
discriminates: switching the memo to a move-to-end LRU turns it red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REhKAmLTibRLmoakxNp6CW
@kfatehi

kfatehi commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

It turns out the ?? bug isn't this PR, it's Skia. TextLine::getGlyphPositionAtCoordinate computes the RTL offset as getUTF16Index(clusterIndex8) + 1, one UTF-16 unit past the cluster start, where the LTR branch one line up asks for a real boundary. An emoji is two units, so tapping one in RTL returns an offset inside the surrogate pair and the next edit splits it. That's the mechanism under the centre-of-the-emoji repro I mentioned back in June. Measurements.

The caret clamp in this branch is the fix at the caller. I tried it in the framework first (flutter/flutter#188719), and the review there was that a controller making programmatic changes to the selection owns keeping them off codepoint boundaries. I agreed and closed it. The clamp stops the corruption but not the misplacement — that half is Skia's.

Caching: you were right, and it was worse than I argued in July. A string with no strongly-directional character (digits, punctuation, emoji-only) never hits the early exit and gets scanned end to end: 683-874 ns against ~10 ns for ordinary text, on every rebuild. There's a bounded memo in front of getTextDirection itself now, so every call site you listed picks it up with no change at the call site — the conversation tile during scroll and send_animation during the animation included. Every call is ~8-10 ns regardless of content, and it needs no invalidation because the direction is a pure function of the text. Insertion-order eviction rather than LRU: promoting on every hit measured 34 ns against the 13 ns scan it would replace. The numbers and the designs I threw out.

On the toggle, I've left it automatic as we left it in July, and the memo takes the overhead argument off the table anyway. For what it's worth the detection isn't a heuristic I invented: it's the first-strong rule from the Unicode bidi algorithm (UAX#9), the same one the platform text stack uses, and Flutter has an open proposal (flutter/flutter#91738) and a live PR (flutter/flutter#190179) to do it in the framework. Still happy to add a setting if you want one.

I've merged current master in, so this is up to date and merges clean. Taking it out of draft.

Everything underneath it, including the engine side: https://github.com/kfatehi/flutter-rtl-fixes

@kfatehi
kfatehi marked this pull request as ready for review August 9, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RTL (right-to-left) text is mis-ordered in message bubbles and the input field

3 participants