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/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/grapheme_caret.dart b/lib/helpers/ui/grapheme_caret.dart new file mode 100644 index 0000000000..05a69a7160 --- /dev/null +++ b/lib/helpers/ui/grapheme_caret.dart @@ -0,0 +1,37 @@ +import 'package:flutter/services.dart' show TextEditingValue, TextSelection; + +/// 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; +} + +/// 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/lib/helpers/ui/text_direction_helpers.dart b/lib/helpers/ui/text_direction_helpers.dart new file mode 100644 index 0000000000..1e01e7b474 --- /dev/null +++ b/lib/helpers/ui/text_direction_helpers.dart @@ -0,0 +1,143 @@ +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); +} + +/// 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 _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. + 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/grapheme_caret_test.dart b/test/grapheme_caret_test.dart new file mode 100644 index 0000000000..e7f20f3f1b --- /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 past the pair end', () { + expect(snapSelectionOffSurrogatePairs(v(2)).selection, const TextSelection.collapsed(offset: 3)); + }); + + 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, 3); + 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; // 3 + final String edited = text.replaceRange(caret, caret, 'x'); // 'a😓xb' + // 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); + }); +} 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); + }); +} 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); + }); +}