From 2595735417fd9c743ae7f9cf0e7323fefae70121 Mon Sep 17 00:00:00 2001 From: kfatehi Date: Sun, 28 Jun 2026 15:40:13 -0700 Subject: [PATCH 1/4] Add right-to-left (RTL) text support 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) --- .../widgets/tile/conversation_tile.dart | 2 + .../widgets/message/reply/reply_bubble.dart | 2 + .../widgets/message/send_animation.dart | 1 + .../widgets/message/text/text_bubble.dart | 2 + .../text_field/conversation_text_field.dart | 14 ++- lib/helpers/helpers.dart | 1 + lib/helpers/types/helpers/message_helper.dart | 6 +- lib/helpers/ui/text_direction_helpers.dart | 87 +++++++++++++++++++ test/rtl_detection_test.dart | 27 ++++++ 9 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 lib/helpers/ui/text_direction_helpers.dart create mode 100644 test/rtl_detection_test.dart diff --git a/lib/app/layouts/conversation_list/widgets/tile/conversation_tile.dart b/lib/app/layouts/conversation_list/widgets/tile/conversation_tile.dart index de10e12257..f829b6ebcf 100644 --- a/lib/app/layouts/conversation_list/widgets/tile/conversation_tile.dart +++ b/lib/app/layouts/conversation_list/widgets/tile/conversation_tile.dart @@ -416,6 +416,7 @@ class _ChatTitleState extends CustomState { return const SizedBox.shrink(); }), if (!isChatCreator && ss.settings.enablePrivateAPI.value && ss.settings.privateSubjectLine.value && chat!.isIMessage) - TextField( + TextDirectionBuilder( + controller: subjController!, + builder: (context, direction) => TextField( + textDirection: direction, textCapitalization: TextCapitalization.sentences, focusNode: controller!.subjectFocusNode, autocorrect: true, @@ -935,7 +938,7 @@ class TextFieldComponentState extends State { controller?.subjectFocusNode.requestFocus(); }, contentInsertionConfiguration: ContentInsertionConfiguration(onContentInserted: onContentCommit), - ), + )), if (!isChatCreator && ss.settings.enablePrivateAPI.value && ss.settings.privateSubjectLine.value && chat!.isIMessage && iOS) Divider( height: 1.5, @@ -945,7 +948,10 @@ class TextFieldComponentState extends State { ), CallbackShortcuts( bindings: txtController.getShortcuts(), - child: TextField( + child: TextDirectionBuilder( + controller: txtController, + builder: (context, direction) => TextField( + textDirection: direction, textCapitalization: TextCapitalization.sentences, focusNode: controller?.focusNode ?? focusNode, autocorrect: true, @@ -1003,7 +1009,7 @@ class TextFieldComponentState extends State { sendMessage.call(); }, contentInsertionConfiguration: ContentInsertionConfiguration(onContentInserted: onContentCommit), - ), + )), ), ], ),), diff --git a/lib/helpers/helpers.dart b/lib/helpers/helpers.dart index 5793d834e4..7dac423262 100644 --- a/lib/helpers/helpers.dart +++ b/lib/helpers/helpers.dart @@ -15,5 +15,6 @@ export 'ui/async_task.dart'; export 'ui/message_widget_helpers.dart'; export 'ui/oauth_helpers.dart'; export 'ui/reaction_helpers.dart'; +export 'ui/text_direction_helpers.dart'; export 'ui/theme_helpers.dart'; export 'ui/ui_helpers.dart'; diff --git a/lib/helpers/types/helpers/message_helper.dart b/lib/helpers/types/helpers/message_helper.dart index ed5fb86d2b..b500449a9a 100644 --- a/lib/helpers/types/helpers/message_helper.dart +++ b/lib/helpers/types/helpers/message_helper.dart @@ -219,7 +219,11 @@ class MessageHelper { + (associatedMessage.text ?? ""); } } - return '$sender $verb ${attachment ? "" : "“"}$messageText${attachment ? "" : "”"}'; + // Wrap the quoted message in a Unicode First-Strong Isolate (U+2068 … U+2069) + // so an RTL message (e.g. Farsi) embedded in this LTR sentence renders as a + // self-contained bidi unit — keeps trailing emoji/punctuation on the correct + // side of the quotes instead of escaping into the surrounding text. + return '$sender $verb ${attachment ? "" : "\u2068“"}$messageText${attachment ? "" : "”\u2069"}'; } } // if we can't fetch the associated message for some reason diff --git a/lib/helpers/ui/text_direction_helpers.dart b/lib/helpers/ui/text_direction_helpers.dart new file mode 100644 index 0000000000..97116ce3bf --- /dev/null +++ b/lib/helpers/ui/text_direction_helpers.dart @@ -0,0 +1,87 @@ +import 'package:flutter/material.dart'; + +/// Rebuilds [builder] with the current text direction of [controller], but ONLY +/// when that direction actually flips — never on plain keystroke or selection +/// changes. +/// +/// A `ValueListenableBuilder` on the controller would rebuild +/// the child on every value change, and since the selection is part of the value, +/// that rebuild lands mid-cursor-drag and cancels the gesture (the caret "lets go" +/// after one step). Listening for direction changes only keeps the child +/// (e.g. a TextField/EditableText) stable during normal editing. +class TextDirectionBuilder extends StatefulWidget { + const TextDirectionBuilder({super.key, required this.controller, required this.builder}); + + final TextEditingController controller; + final Widget Function(BuildContext context, TextDirection direction) builder; + + @override + State createState() => _TextDirectionBuilderState(); +} + +class _TextDirectionBuilderState extends State { + late TextDirection _direction = getTextDirection(widget.controller.text); + + @override + void initState() { + super.initState(); + widget.controller.addListener(_onChanged); + } + + void _onChanged() { + final next = getTextDirection(widget.controller.text); + if (next != _direction) setState(() => _direction = next); + } + + @override + void didUpdateWidget(TextDirectionBuilder oldWidget) { + super.didUpdateWidget(oldWidget); + if (oldWidget.controller != widget.controller) { + oldWidget.controller.removeListener(_onChanged); + widget.controller.addListener(_onChanged); + _onChanged(); + } + } + + @override + void dispose() { + widget.controller.removeListener(_onChanged); + super.dispose(); + } + + @override + Widget build(BuildContext context) => widget.builder(context, _direction); +} + +/// Detects the paragraph direction of [text] from its first strongly-directional +/// character (UAX#9 "first strong" heuristic), so RTL languages (Farsi, Arabic, +/// Hebrew) render and align correctly. +/// +/// Implemented over runes rather than intl's [Bidi.startsWithRtl], which +/// misclassifies leading emoji as LTR (their UTF-16 surrogates fall inside its +/// LTR character ranges). +TextDirection getTextDirection(String? text) { + if (text == null) return TextDirection.ltr; + for (final rune in text.runes) { + // Strong RTL: Hebrew, Arabic, Syriac, Thaana, NKo, Samaritan..., + // Arabic/Hebrew presentation forms, and the historic/supplemental RTL planes. + if ((rune >= 0x0590 && rune <= 0x08FF) || + (rune >= 0xFB1D && rune <= 0xFDFF) || + (rune >= 0xFE70 && rune <= 0xFEFF) || + (rune >= 0x10800 && rune <= 0x10FFF) || + (rune >= 0x1E800 && rune <= 0x1EFFF)) { + return TextDirection.rtl; + } + // Strong LTR: Latin letters and the LTR script blocks below/above the RTL + // ranges. Everything else (digits, punctuation, emoji, symbols) is treated + // as neutral and skipped. + if ((rune >= 0x41 && rune <= 0x5A) || + (rune >= 0x61 && rune <= 0x7A) || + (rune >= 0x00C0 && rune <= 0x058F) || + (rune >= 0x0900 && rune <= 0x1FFF) || + (rune >= 0x2C00 && rune <= 0xD7FF)) { + return TextDirection.ltr; + } + } + return TextDirection.ltr; +} diff --git a/test/rtl_detection_test.dart b/test/rtl_detection_test.dart new file mode 100644 index 0000000000..92cb5abb8b --- /dev/null +++ b/test/rtl_detection_test.dart @@ -0,0 +1,27 @@ +import 'package:bluebubbles/helpers/ui/text_direction_helpers.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + test('farsi text detected as RTL', () { + expect(getTextDirection('سلام'), TextDirection.rtl); + expect(getTextDirection('سلام 😂'), TextDirection.rtl); + expect(getTextDirection('چطوری، خوبی؟'), TextDirection.rtl); + }); + test('emoji/punctuation-leading farsi detected as RTL (first strong char)', () { + expect(getTextDirection('😂 سلام'), TextDirection.rtl); + expect(getTextDirection('"سلام"'), TextDirection.rtl); + expect(getTextDirection('۱۲۳ سلام'), TextDirection.rtl); + }); + test('english stays LTR', () { + expect(getTextDirection('hello'), TextDirection.ltr); + expect(getTextDirection('hello 😂'), TextDirection.ltr); + expect(getTextDirection('😂 hello'), TextDirection.ltr); + expect(getTextDirection(''), TextDirection.ltr); + expect(getTextDirection(null), TextDirection.ltr); + }); + test('mixed first-strong wins', () { + expect(getTextDirection('سلام hello'), TextDirection.rtl); + expect(getTextDirection('hello سلام'), TextDirection.ltr); + }); +} From 12b865a99a1869364efa64f0bfa4137ef369dc54 Mon Sep 17 00:00:00 2001 From: kfatehi Date: Mon, 29 Jun 2026 01:33:47 -0700 Subject: [PATCH 2/4] Fix emoji ?? corruption: clamp caret off surrogate-pair interior Snap a collapsed caret (or selection endpoint) off any UTF-16 surrogate-pair interior in the compose controller (SpellCheckTextEditingController.set value) 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. Adds test/grapheme_caret_test.dart. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../custom_text_editing_controllers.dart | 6 ++ lib/helpers/ui/grapheme_caret.dart | 29 +++++++++ test/grapheme_caret_test.dart | 59 +++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 lib/helpers/ui/grapheme_caret.dart create mode 100644 test/grapheme_caret_test.dart diff --git a/lib/app/components/custom_text_editing_controllers.dart b/lib/app/components/custom_text_editing_controllers.dart index 30934ea600..59817b69a4 100644 --- a/lib/app/components/custom_text_editing_controllers.dart +++ b/lib/app/components/custom_text_editing_controllers.dart @@ -3,6 +3,7 @@ import "dart:math"; import "package:bluebubbles/app/layouts/conversation_view/dialogs/custom_mention_dialog.dart"; import "package:bluebubbles/helpers/helpers.dart"; +import "package:bluebubbles/helpers/ui/grapheme_caret.dart"; import "package:bluebubbles/database/models.dart"; import "package:bluebubbles/services/services.dart"; import 'package:bluebubbles/utils/emoji.dart'; @@ -121,6 +122,11 @@ class SpellCheckTextEditingController extends TextEditingController { _mistakeTooltip = null; } + // Never leave the caret inside a UTF-16 surrogate pair: a following edit would split the + // emoji into lone surrogates, which the Android input channel turns into "?" (the "??" + // corruption) and which crash the text painter. App-side equivalent of flutter/flutter#188713. + newValue = snapSelectionOffSurrogatePairs(newValue); + super.value = newValue; } diff --git a/lib/helpers/ui/grapheme_caret.dart b/lib/helpers/ui/grapheme_caret.dart new file mode 100644 index 0000000000..22dee9a826 --- /dev/null +++ b/lib/helpers/ui/grapheme_caret.dart @@ -0,0 +1,29 @@ +import 'package:flutter/services.dart' show TextEditingValue, TextSelection; + +/// Returns [offset] moved back to the start of a UTF-16 surrogate pair when it falls between the +/// pair's two code units; otherwise returns it unchanged. The result is never inside a pair. +int _offsetOffSurrogatePair(String text, int offset) { + if (offset <= 0 || offset >= text.length) return offset; + final int prev = text.codeUnitAt(offset - 1); + final int next = text.codeUnitAt(offset); + final bool insidePair = prev >= 0xD800 && prev <= 0xDBFF && next >= 0xDC00 && next <= 0xDFFF; + return insidePair ? offset - 1 : offset; +} + +/// Snaps both endpoints of [value]'s selection off any UTF-16 surrogate-pair interior. +/// +/// A caret left between the two halves of an emoji's surrogate pair lets the next edit split the +/// pair into lone surrogates. On Android the text-input channel then encodes each lone half as +/// `?` (the user-visible "??" corruption), and the text painter throws +/// "string is not well-formed UTF-16". This is the app-side equivalent of the framework fix in +/// flutter/flutter#188713 (PR flutter/flutter#188719); applying it in the compose controller fixes +/// the corruption without requiring a Flutter SDK upgrade, and is scoped to this field only. +TextEditingValue snapSelectionOffSurrogatePairs(TextEditingValue value) { + final TextSelection selection = value.selection; + if (!selection.isValid) return value; + final String text = value.text; + final int base = _offsetOffSurrogatePair(text, selection.baseOffset); + final int extent = _offsetOffSurrogatePair(text, selection.extentOffset); + if (base == selection.baseOffset && extent == selection.extentOffset) return value; + return value.copyWith(selection: selection.copyWith(baseOffset: base, extentOffset: extent)); +} diff --git a/test/grapheme_caret_test.dart b/test/grapheme_caret_test.dart new file mode 100644 index 0000000000..aedac0eece --- /dev/null +++ b/test/grapheme_caret_test.dart @@ -0,0 +1,59 @@ +// Unit test for the app-side caret clamp wired into SpellCheckTextEditingController.set value. +// Proves the same prevention as the framework fix (flutter/flutter#188719) without a Flutter +// upgrade: a caret inside a surrogate pair is snapped to the pair boundary, so the next edit can't +// split the emoji. +// +// Run: C:\tools\flutter\bin\flutter.bat test test/grapheme_caret_test.dart + +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:bluebubbles/helpers/ui/grapheme_caret.dart'; + +void main() { + // 'a😓b' -> code units [0061, D83D, DE13, 0062]; offset 2 is between D83D and DE13. + const String text = 'a😓b'; + + TextEditingValue v(int base, [int? extent]) => + TextEditingValue(text: text, selection: TextSelection(baseOffset: base, extentOffset: extent ?? base)); + + test('a collapsed caret inside the pair is snapped to the pair start', () { + expect(snapSelectionOffSurrogatePairs(v(2)).selection, const TextSelection.collapsed(offset: 1)); + }); + + test('boundary carets are unchanged', () { + for (final int offset in [0, 1, 3, 4]) { + expect(snapSelectionOffSurrogatePairs(v(offset)).selection.baseOffset, offset); + } + }); + + test('a selection endpoint inside the pair is snapped (both ends handled)', () { + // base inside the pair, extent at a boundary. + final TextEditingValue out = snapSelectionOffSurrogatePairs(v(2, 4)); + expect(out.selection.baseOffset, 1); + expect(out.selection.extentOffset, 4); + }); + + test('invalid/empty selection is left alone', () { + const TextEditingValue none = TextEditingValue(text: text); + expect(snapSelectionOffSurrogatePairs(none).selection, none.selection); + }); + + test('inserting at the snapped caret keeps the emoji intact', () { + final int caret = snapSelectionOffSurrogatePairs(v(2)).selection.baseOffset; // 1 + final String edited = text.replaceRange(caret, caret, 'x'); // 'ax😓b' + // No lone surrogate remains. + final List u = edited.codeUnits; + bool lone = false; + for (int i = 0; i < u.length; i++) { + final int c = u[i]; + if (c >= 0xD800 && c <= 0xDBFF) { + if (i + 1 >= u.length || !(u[i + 1] >= 0xDC00 && u[i + 1] <= 0xDFFF)) lone = true; + i++; + } else if (c >= 0xDC00 && c <= 0xDFFF) { + lone = true; + } + } + expect(lone, isFalse); + expect(edited.contains('😓'), isTrue); + }); +} From 3d33a76983fb037fb1ed191a11e596edceb605a3 Mon Sep 17 00:00:00 2001 From: kfatehi Date: Mon, 29 Jun 2026 03:15:49 -0700 Subject: [PATCH 3/4] Snap caret past surrogate-pair end so RTL backspace deletes the emoji 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) Claude-Session: https://claude.ai/code/session_01FrEin3bnyFrXaxSmL9iLQM --- lib/helpers/ui/grapheme_caret.dart | 12 ++++++++++-- test/grapheme_caret_test.dart | 10 +++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/helpers/ui/grapheme_caret.dart b/lib/helpers/ui/grapheme_caret.dart index 22dee9a826..05a69a7160 100644 --- a/lib/helpers/ui/grapheme_caret.dart +++ b/lib/helpers/ui/grapheme_caret.dart @@ -1,13 +1,21 @@ import 'package:flutter/services.dart' show TextEditingValue, TextSelection; -/// Returns [offset] moved back to the start of a UTF-16 surrogate pair when it falls between the +/// Returns [offset] moved forward to just past a UTF-16 surrogate pair when it falls between the /// pair's two code units; otherwise returns it unchanged. The result is never inside a pair. +/// +/// We snap to the end of the pair (offset + 1) rather than the start (offset - 1) so the caret +/// lands *after* the emoji. In RTL a tap aiming for the spot after a trailing emoji (visually to +/// its left) often lands mid-glyph; snapping to the start would yank the caret before the emoji, +/// where backspace deletes the wrong character (e.g. the preceding space) and the emoji can never +/// be removed. Snapping past the pair keeps a tap on a trailing emoji able to backspace it, and is +/// still a clean boundary so the next edit cannot split the pair. offset + 1 is always valid here: +/// a low surrogate at [offset] guarantees offset < text.length. int _offsetOffSurrogatePair(String text, int offset) { if (offset <= 0 || offset >= text.length) return offset; final int prev = text.codeUnitAt(offset - 1); final int next = text.codeUnitAt(offset); final bool insidePair = prev >= 0xD800 && prev <= 0xDBFF && next >= 0xDC00 && next <= 0xDFFF; - return insidePair ? offset - 1 : offset; + return insidePair ? offset + 1 : offset; } /// Snaps both endpoints of [value]'s selection off any UTF-16 surrogate-pair interior. diff --git a/test/grapheme_caret_test.dart b/test/grapheme_caret_test.dart index aedac0eece..e7f20f3f1b 100644 --- a/test/grapheme_caret_test.dart +++ b/test/grapheme_caret_test.dart @@ -16,8 +16,8 @@ void main() { TextEditingValue v(int base, [int? extent]) => TextEditingValue(text: text, selection: TextSelection(baseOffset: base, extentOffset: extent ?? base)); - test('a collapsed caret inside the pair is snapped to the pair start', () { - expect(snapSelectionOffSurrogatePairs(v(2)).selection, const TextSelection.collapsed(offset: 1)); + test('a collapsed caret inside the pair is snapped past the pair end', () { + expect(snapSelectionOffSurrogatePairs(v(2)).selection, const TextSelection.collapsed(offset: 3)); }); test('boundary carets are unchanged', () { @@ -29,7 +29,7 @@ void main() { test('a selection endpoint inside the pair is snapped (both ends handled)', () { // base inside the pair, extent at a boundary. final TextEditingValue out = snapSelectionOffSurrogatePairs(v(2, 4)); - expect(out.selection.baseOffset, 1); + expect(out.selection.baseOffset, 3); expect(out.selection.extentOffset, 4); }); @@ -39,8 +39,8 @@ void main() { }); test('inserting at the snapped caret keeps the emoji intact', () { - final int caret = snapSelectionOffSurrogatePairs(v(2)).selection.baseOffset; // 1 - final String edited = text.replaceRange(caret, caret, 'x'); // 'ax😓b' + final int caret = snapSelectionOffSurrogatePairs(v(2)).selection.baseOffset; // 3 + final String edited = text.replaceRange(caret, caret, 'x'); // 'a😓xb' // No lone surrogate remains. final List u = edited.codeUnits; bool lone = false; From 28ebbb591abcab3fc68b8ed5c08b45f5488e9d53 Mon Sep 17 00:00:00 2001 From: kfatehi Date: Thu, 6 Aug 2026 14:52:34 -0700 Subject: [PATCH 4/4] Memoize getTextDirection so message rebuilds stop rescanning text 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 #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) Claude-Session: https://claude.ai/code/session_01REhKAmLTibRLmoakxNp6CW --- lib/helpers/ui/text_direction_helpers.dart | 60 +++++++++- test/rtl_direction_cache_test.dart | 127 +++++++++++++++++++++ 2 files changed, 185 insertions(+), 2 deletions(-) create mode 100644 test/rtl_direction_cache_test.dart diff --git a/lib/helpers/ui/text_direction_helpers.dart b/lib/helpers/ui/text_direction_helpers.dart index 97116ce3bf..1e01e7b474 100644 --- a/lib/helpers/ui/text_direction_helpers.dart +++ b/lib/helpers/ui/text_direction_helpers.dart @@ -53,15 +53,71 @@ class _TextDirectionBuilderState extends State { Widget build(BuildContext context) => widget.builder(context, _direction); } +/// Upper bound on [_directionCache]. A conversation list renders on the order of +/// 60 tiles (a title and a subtitle each) and a conversation view on the order of +/// 100 message parts, so 512 holds a full working set with headroom while +/// bounding how many strings the cache keeps alive. +const int _directionCacheCapacity = 512; + +/// Memoizes [getTextDirection]. Safe with no invalidation: the direction is a +/// pure function of the text, so an entry cannot go stale for its own key. +/// +/// A Dart map literal is insertion-ordered, so the oldest key is `keys.first` and +/// eviction is O(1). Deliberately NOT a move-to-end LRU: promoting a key on every +/// hit costs a `remove` plus a re-insert, measured at 34 ns/hit against the 13 ns +/// scan it would replace for ordinary text — a true LRU makes the common case +/// slower than having no cache at all. Insertion-order eviction keeps the hit +/// path to a single lookup (~6-11 ns). +final Map _directionCache = {}; + +/// Clears the memo table. Tests only — it never needs invalidating in production +/// because [_detectTextDirection] is pure. +@visibleForTesting +void clearTextDirectionCache() => _directionCache.clear(); + +/// Number of memoized entries. Tests only. +@visibleForTesting +int get textDirectionCacheLength => _directionCache.length; + +/// The bound enforced on the memo table. Tests only. +@visibleForTesting +int get textDirectionCacheCapacity => _directionCacheCapacity; + +/// Whether [text] is currently memoized. Tests only — lets the eviction test +/// assert *which* key was dropped, not merely how many remain. +@visibleForTesting +bool textDirectionCacheContains(String text) => _directionCache.containsKey(text); + /// Detects the paragraph direction of [text] from its first strongly-directional /// character (UAX#9 "first strong" heuristic), so RTL languages (Farsi, Arabic, /// Hebrew) render and align correctly. /// +/// Memoized, because the message widgets call this from inside `build` — the +/// `RichText` in a message bubble, a reply bubble, the send animation and the +/// conversation tile — so it re-runs on every rebuild, not only when the text +/// changes. Detection early-exits on the first strongly-directional character, +/// which is cheap for ordinary text (~13 ns), but text made only of neutral +/// characters (digits, punctuation, an emoji-only message) has no such character +/// and is scanned to the end: ~640 ns for 200 units, ~1150 ns for an emoji-only +/// message. The memo turns that into one map lookup. +TextDirection getTextDirection(String? text) { + if (text == null || text.isEmpty) return TextDirection.ltr; + final TextDirection? cached = _directionCache[text]; + if (cached != null) return cached; + final TextDirection direction = _detectTextDirection(text); + _directionCache[text] = direction; + if (_directionCache.length > _directionCacheCapacity) { + _directionCache.remove(_directionCache.keys.first); + } + return direction; +} + +/// The uncached detection itself. +/// /// Implemented over runes rather than intl's [Bidi.startsWithRtl], which /// misclassifies leading emoji as LTR (their UTF-16 surrogates fall inside its /// LTR character ranges). -TextDirection getTextDirection(String? text) { - if (text == null) return TextDirection.ltr; +TextDirection _detectTextDirection(String text) { for (final rune in text.runes) { // Strong RTL: Hebrew, Arabic, Syriac, Thaana, NKo, Samaritan..., // Arabic/Hebrew presentation forms, and the historic/supplemental RTL planes. diff --git a/test/rtl_direction_cache_test.dart b/test/rtl_direction_cache_test.dart new file mode 100644 index 0000000000..83c9603a7b --- /dev/null +++ b/test/rtl_direction_cache_test.dart @@ -0,0 +1,127 @@ +import 'package:bluebubbles/helpers/ui/text_direction_helpers.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +/// Every case rtl_detection_test.dart asserts, so the memo can be checked to +/// return the same answers cold and warm. +const List<(String?, TextDirection)> _cases = <(String?, TextDirection)>[ + ('سلام', TextDirection.rtl), + ('سلام 😂', TextDirection.rtl), + ('چطوری، خوبی؟', TextDirection.rtl), + ('😂 سلام', TextDirection.rtl), + ('"سلام"', TextDirection.rtl), + ('۱۲۳ سلام', TextDirection.rtl), + ('hello', TextDirection.ltr), + ('hello 😂', TextDirection.ltr), + ('😂 hello', TextDirection.ltr), + ('', TextDirection.ltr), + (null, TextDirection.ltr), + ('سلام hello', TextDirection.rtl), + ('hello سلام', TextDirection.ltr), + // All-neutral: no strongly-directional character at all, so this is the case + // the memo exists for — the scan runs to the end of the string. + ('1234567890 !@#\$%^&*()', TextDirection.ltr), + ('😂🎉👍🏽❤️🇮🇷', TextDirection.ltr), +]; + +void main() { + setUp(clearTextDirectionCache); + + test('memoized result equals the uncached result, cold and warm', () { + for (final (String? text, TextDirection expected) in _cases) { + clearTextDirectionCache(); + expect(getTextDirection(text), expected, reason: 'cold: $text'); + // Warm: same key, now served from the memo. + expect(getTextDirection(text), expected, reason: 'warm: $text'); + // And repeatedly, to catch a memo that corrupts itself on re-read. + for (int i = 0; i < 5; i++) { + expect(getTextDirection(text), expected, reason: 'repeat $i: $text'); + } + } + }); + + test('a value equal to but not identical with the cached key still hits', () { + // The call sites pass Message.fullText, which builds a NEW String on every + // call — so the memo has to key on value, not identity. + const String subject = 'سلام'; + final String first = [subject, 'خوبی؟'].join('\n'); + final String second = [subject, 'خوبی؟'].join('\n'); + expect(identical(first, second), isFalse, reason: 'the two must be distinct instances'); + + expect(getTextDirection(first), TextDirection.rtl); + final int afterFirst = textDirectionCacheLength; + expect(getTextDirection(second), TextDirection.rtl); + expect(textDirectionCacheLength, afterFirst, reason: 'an equal string must not add a second entry'); + }); + + test('null and empty are answered without occupying the memo', () { + expect(getTextDirection(null), TextDirection.ltr); + expect(getTextDirection(''), TextDirection.ltr); + expect(textDirectionCacheLength, 0); + }); + + test('the memo is bounded at its capacity', () { + final int capacity = textDirectionCacheCapacity; + for (int i = 0; i < capacity * 2 + 7; i++) { + getTextDirection('message number $i'); + } + expect(textDirectionCacheLength, lessThanOrEqualTo(capacity)); + expect(textDirectionCacheLength, capacity, reason: 'should be full, not undersized'); + }); + + test('eviction is insertion-ordered, and a hit does NOT promote', () { + final int capacity = textDirectionCacheCapacity; + for (int i = 0; i < capacity; i++) { + getTextDirection('key $i'); + } + expect(textDirectionCacheLength, capacity); + expect(textDirectionCacheContains('key 0'), isTrue); + + // Read the oldest key. Under a move-to-end LRU this would promote it to + // most-recently-used; under insertion-order eviction it stays oldest. That + // difference is the whole point — promoting costs a remove plus a re-insert + // (34 ns/hit measured) against a 13 ns scan, so it would make ordinary text + // slower than having no memo at all. + getTextDirection('key 0'); + expect(textDirectionCacheLength, capacity, reason: 'a hit must not grow the memo'); + + // One more distinct key overflows the bound and evicts exactly one entry. + getTextDirection('overflow'); + expect(textDirectionCacheLength, capacity); + + // The evicted one is the OLDEST INSERTED, despite having just been read. + // A move-to-end LRU would have kept 'key 0' and dropped 'key 1' instead, so + // these two assertions fail against that implementation. + expect(textDirectionCacheContains('key 0'), isFalse, reason: 'oldest-inserted must be evicted even though it was just read'); + expect(textDirectionCacheContains('key 1'), isTrue, reason: 'the next-oldest must survive'); + expect(textDirectionCacheContains('overflow'), isTrue); + }); + + test('evicted entries are recomputed correctly, not lost', () { + final int capacity = textDirectionCacheCapacity; + const String farsi = 'سلام خوبی'; + expect(getTextDirection(farsi), TextDirection.rtl); + // Flood past capacity so the Farsi entry is certainly evicted. + for (int i = 0; i < capacity + 10; i++) { + getTextDirection('filler $i'); + } + // Still correct — the memo is an optimization, never the source of truth. + expect(getTextDirection(farsi), TextDirection.rtl); + }); + + test('memo does not confuse two strings with the same length', () { + expect(getTextDirection('abcd'), TextDirection.ltr); + expect(getTextDirection('سلام'), TextDirection.rtl); + expect(getTextDirection('abcd'), TextDirection.ltr); + expect(getTextDirection('سلام'), TextDirection.rtl); + expect(textDirectionCacheLength, 2); + }); + + test('clearTextDirectionCache empties the memo without changing answers', () { + expect(getTextDirection('سلام'), TextDirection.rtl); + expect(textDirectionCacheLength, 1); + clearTextDirectionCache(); + expect(textDirectionCacheLength, 0); + expect(getTextDirection('سلام'), TextDirection.rtl); + }); +}