Avoid using TextAffinity in TextBoundary (flutter#117446)

* Avoid affinity like the plague

* ignore lint

* clean up

* fix test

* review

* Move wordboundary to text painter

* docs

* fix tests

Author: LongCatIsLooong

packages/flutter/lib/src/painting/text_painter.dart

@@ -144,6 +144,133 @@ enum TextWidthBasis {
   longestLine,
 }
 
+/// A [TextBoundary] subclass for locating word breaks.
+///
+/// The underlying implementation uses [UAX #29](https://unicode.org/reports/tr29/)
+/// defined default word boundaries.
+///
+/// The default word break rules can be tailored to meet the requirements of
+/// different use cases. For instance, the default rule set keeps horizontal
+/// whitespaces together as a single word, which may not make sense in a
+/// word-counting context -- "hello    world" counts as 3 words instead of 2.
+/// An example is the [moveByWordBoundary] variant, which is a tailored
+/// word-break locator that more closely matches the default behavior of most
+/// platforms and editors when it comes to handling text editing keyboard
+/// shortcuts that move or delete word by word.
+class WordBoundary extends TextBoundary {
+  /// Creates a [WordBoundary] with the text and layout information.
+  WordBoundary._(this._text, this._paragraph);
+
+  final InlineSpan _text;
+  final ui.Paragraph _paragraph;
+
+  @override
+  TextRange getTextBoundaryAt(int position) => _paragraph.getWordBoundary(TextPosition(offset: max(position, 0)));
+
+  // Combines two UTF-16 code units (high surrogate + low surrogate) into a
+  // single code point that represents a supplementary character.
+  static int _codePointFromSurrogates(int highSurrogate, int lowSurrogate) {
+    assert(
+      TextPainter._isHighSurrogate(highSurrogate),
+      'U+${highSurrogate.toRadixString(16).toUpperCase().padLeft(4, "0")}) is not a high surrogate.',
+    );
+    assert(
+      TextPainter._isLowSurrogate(lowSurrogate),
+      'U+${lowSurrogate.toRadixString(16).toUpperCase().padLeft(4, "0")}) is not a low surrogate.',
+    );
+    const int base = 0x010000 - (0xD800 << 10) - 0xDC00;
+    return (highSurrogate << 10) + lowSurrogate + base;
+  }
+
+  // The Runes class does not provide random access with a code unit offset.
+  int? _codePointAt(int index) {
+    final int? codeUnitAtIndex = _text.codeUnitAt(index);
+    if (codeUnitAtIndex == null) {
+      return null;
+    }
+    switch (codeUnitAtIndex & 0xFC00) {
+      case 0xD800:
+        return _codePointFromSurrogates(codeUnitAtIndex, _text.codeUnitAt(index + 1)!);
+      case 0xDC00:
+        return _codePointFromSurrogates(_text.codeUnitAt(index - 1)!, codeUnitAtIndex);
+      default:
+        return codeUnitAtIndex;
+    }
+  }
+
+  static bool _isNewline(int codePoint) {
+    switch (codePoint) {
+      case 0x000A:
+      case 0x0085:
+      case 0x000B:
+      case 0x000C:
+      case 0x2028:
+      case 0x2029:
+        return true;
+      default:
+        return false;
+    }
+  }
+
+  bool _skipSpacesAndPunctuations(int offset, bool forward) {
+    // Use code point since some punctuations are supplementary characters.
+    // "inner" here refers to the code unit that's before the break in the
+    // search direction (`forward`).
+    final int? innerCodePoint = _codePointAt(forward ? offset - 1 : offset);
+    final int? outerCodeUnit = _text.codeUnitAt(forward ? offset : offset - 1);
+
+    // Make sure the hard break rules in UAX#29 take precedence over the ones we
+    // add below. Luckily there're only 4 hard break rules for word breaks, and
+    // dictionary based breaking does not introduce new hard breaks:
+    // https://unicode-org.github.io/icu/userguide/boundaryanalysis/break-rules.html#word-dictionaries
+    //
+    // WB1 & WB2: always break at the start or the end of the text.
+    final bool hardBreakRulesApply = innerCodePoint == null || outerCodeUnit == null
+    // WB3a & WB3b: always break before and after newlines.
+                                  || _isNewline(innerCodePoint) || _isNewline(outerCodeUnit);
+    return hardBreakRulesApply || !RegExp(r'[\p{Space_Separator}\p{Punctuation}]', unicode: true).hasMatch(String.fromCharCode(innerCodePoint));
+  }
+
+  /// Returns a [TextBoundary] suitable for handling keyboard navigation
+  /// commands that change the current selection word by word.
+  ///
+  /// This [TextBoundary] is used by text widgets in the flutter framework to
+  /// provide default implementation for text editing shortcuts, for example,
+  /// "delete to the previous word".
+  ///
+  /// The implementation applies the same set of rules [WordBoundary] uses,
+  /// except that word breaks end on a space separator or a punctuation will be
+  /// skipped, to match the behavior of most platforms. Additional rules may be
+  /// added in the future to better match platform behaviors.
+  late final TextBoundary moveByWordBoundary = _UntilTextBoundary(this, _skipSpacesAndPunctuations);
+}
+
+class _UntilTextBoundary extends TextBoundary {
+  const _UntilTextBoundary(this._textBoundary, this._predicate);
+
+  final UntilPredicate _predicate;
+  final TextBoundary _textBoundary;
+
+  @override
+  int? getLeadingTextBoundaryAt(int position) {
+    if (position < 0) {
+      return null;
+    }
+    final int? offset = _textBoundary.getLeadingTextBoundaryAt(position);
+    return offset == null || _predicate(offset, false)
+      ? offset
+      : getLeadingTextBoundaryAt(offset - 1);
+  }
+
+  @override
+  int? getTrailingTextBoundaryAt(int position) {
+    final int? offset = _textBoundary.getTrailingTextBoundaryAt(max(position, 0));
+    return offset == null || _predicate(offset, true)
+      ? offset
+      : getTrailingTextBoundaryAt(offset);
+  }
+}
+
 /// This is used to cache and pass the computed metrics regarding the
 /// caret's size and position. This is preferred due to the expensive
 /// nature of the calculation.
@@ -750,7 +877,7 @@ class TextPainter {
 
   // Creates a ui.Paragraph using the current configurations in this class and
   // assign it to _paragraph.
-  void _createParagraph() {
+  ui.Paragraph _createParagraph() {
     assert(_paragraph == null || _rebuildParagraphForPaint);
     final InlineSpan? text = this.text;
     if (text == null) {
@@ -763,8 +890,9 @@ class TextPainter {
       _debugMarkNeedsLayoutCallStack = null;
       return true;
     }());
-    _paragraph = builder.build();
+    final ui.Paragraph paragraph = _paragraph = builder.build();
     _rebuildParagraphForPaint = false;
+    return paragraph;
   }
 
   void _layoutParagraph(double minWidth, double maxWidth) {
@@ -861,13 +989,18 @@ class TextPainter {
     canvas.drawParagraph(_paragraph!, offset);
   }
 
-  // Returns true iff the given value is a valid UTF-16 surrogate. The value
+  // Returns true iff the given value is a valid UTF-16 high surrogate. The value
   // must be a UTF-16 code unit, meaning it must be in the range 0x0000-0xFFFF.
   //
   // See also:
   //   * https://en.wikipedia.org/wiki/UTF-16#Code_points_from_U+010000_to_U+10FFFF
-  static bool _isUtf16Surrogate(int value) {
-    return value & 0xF800 == 0xD800;
+  static bool _isHighSurrogate(int value) {
+    return value & 0xFC00 == 0xD800;
+  }
+
+  // Whether the given UTF-16 code unit is a low (second) surrogate.
+  static bool _isLowSurrogate(int value) {
+    return value & 0xFC00 == 0xDC00;
   }
 
   // Checks if the glyph is either [Unicode.RLM] or [Unicode.LRM]. These values take
@@ -886,7 +1019,7 @@ class TextPainter {
       return null;
     }
     // TODO(goderbauer): doesn't handle extended grapheme clusters with more than one Unicode scalar value (https://github.com/flutter/flutter/issues/13404).
-    return _isUtf16Surrogate(nextCodeUnit) ? offset + 2 : offset + 1;
+    return _isHighSurrogate(nextCodeUnit) ? offset + 2 : offset + 1;
   }
 
   /// Returns the closest offset before `offset` at which the input cursor can
@@ -897,7 +1030,7 @@ class TextPainter {
       return null;
     }
     // TODO(goderbauer): doesn't handle extended grapheme clusters with more than one Unicode scalar value (https://github.com/flutter/flutter/issues/13404).
-    return _isUtf16Surrogate(prevCodeUnit) ? offset - 2 : offset - 1;
+    return _isLowSurrogate(prevCodeUnit) ? offset - 2 : offset - 1;
   }
 
   // Unicode value for a zero width joiner character.
@@ -916,7 +1049,7 @@ class TextPainter {
     const int NEWLINE_CODE_UNIT = 10;
 
     // Check for multi-code-unit glyphs such as emojis or zero width joiner.
-    final bool needsSearch = _isUtf16Surrogate(prevCodeUnit) || _text!.codeUnitAt(offset) == _zwjUtf16 || _isUnicodeDirectionality(prevCodeUnit);
+    final bool needsSearch = _isHighSurrogate(prevCodeUnit) || _isLowSurrogate(prevCodeUnit) || _text!.codeUnitAt(offset) == _zwjUtf16 || _isUnicodeDirectionality(prevCodeUnit);
     int graphemeClusterLength = needsSearch ? 2 : 1;
     List<TextBox> boxes = <TextBox>[];
     while (boxes.isEmpty) {
@@ -966,7 +1099,7 @@ class TextPainter {
     final int nextCodeUnit = plainText.codeUnitAt(min(offset, plainTextLength - 1));
 
     // Check for multi-code-unit glyphs such as emojis or zero width joiner
-    final bool needsSearch = _isUtf16Surrogate(nextCodeUnit) || nextCodeUnit == _zwjUtf16 || _isUnicodeDirectionality(nextCodeUnit);
+    final bool needsSearch = _isHighSurrogate(nextCodeUnit) || _isLowSurrogate(nextCodeUnit) || nextCodeUnit == _zwjUtf16 || _isUnicodeDirectionality(nextCodeUnit);
     int graphemeClusterLength = needsSearch ? 2 : 1;
     List<TextBox> boxes = <TextBox>[];
     while (boxes.isEmpty) {
@@ -1141,6 +1274,18 @@ class TextPainter {
     return _paragraph!.getWordBoundary(position);
   }
 
+  /// {@template flutter.painting.TextPainter.wordBoundaries}
+  /// Returns a [TextBoundary] that can be used to perform word boundary analysis
+  /// on the current [text].
+  ///
+  /// This [TextBoundary] uses word boundary rules defined in [Unicode Standard
+  /// Annex #29](http://www.unicode.org/reports/tr29/#Word_Boundaries).
+  /// {@endtemplate}
+  ///
+  /// Currently word boundary analysis can only be performed after [layout]
+  /// has been called.
+  WordBoundary get wordBoundaries => WordBoundary._(text!, _paragraph!);
+
   /// Returns the text range of the line at the given offset.
   ///
   /// The newline (if any) is not returned as part of the range.

packages/flutter/lib/src/rendering/editable.dart

@@ -2098,6 +2098,9 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
     _setSelection(newSelection, cause);
   }
 
+  /// {@macro flutter.painting.TextPainter.wordBoundaries}
+  WordBoundary get wordBoundaries => _textPainter.wordBoundaries;
+
   /// Select a word around the location of the last tap down.
   ///
   /// {@macro flutter.rendering.RenderEditable.selectPosition}

packages/flutter/lib/src/rendering/paragraph.dart

@@ -1559,21 +1559,21 @@ class _SelectableFragment with Selectable, ChangeNotifier implements TextLayoutM
     switch (granularity) {
       case TextGranularity.character:
         final String text = range.textInside(fullText);
-        newPosition = _getNextPosition(CharacterBoundary(text), targetedEdge, forward);
+        newPosition = _moveBeyondTextBoundaryAtDirection(targetedEdge, forward, CharacterBoundary(text));
         result = SelectionResult.end;
         break;
       case TextGranularity.word:
-        final String text = range.textInside(fullText);
-        newPosition = _getNextPosition(WhitespaceBoundary(text) + WordBoundary(this), targetedEdge, forward);
+        final TextBoundary textBoundary = paragraph._textPainter.wordBoundaries.moveByWordBoundary;
+        newPosition = _moveBeyondTextBoundaryAtDirection(targetedEdge, forward, textBoundary);
         result = SelectionResult.end;
         break;
       case TextGranularity.line:
-        newPosition = _getNextPosition(LineBreak(this), targetedEdge, forward);
+        newPosition = _moveToTextBoundaryAtDirection(targetedEdge, forward, LineBoundary(this));
         result = SelectionResult.end;
         break;
       case TextGranularity.document:
         final String text = range.textInside(fullText);
-        newPosition = _getNextPosition(DocumentBoundary(text), targetedEdge, forward);
+        newPosition = _moveBeyondTextBoundaryAtDirection(targetedEdge, forward, DocumentBoundary(text));
         if (forward && newPosition.offset == range.end) {
           result = SelectionResult.next;
         } else if (!forward && newPosition.offset == range.start) {
@@ -1592,15 +1592,43 @@ class _SelectableFragment with Selectable, ChangeNotifier implements TextLayoutM
     return result;
   }
 
-  TextPosition _getNextPosition(TextBoundary boundary, TextPosition position, bool forward) {
-    if (forward) {
-      return _clampTextPosition(
-        (PushTextPosition.forward + boundary).getTrailingTextBoundaryAt(position)
-      );
+  // Move **beyond** the local boundary of the given type (unless range.start or
+  // range.end is reached). Used for most TextGranularity types except for
+  // TextGranularity.line, to ensure the selection movement doesn't get stuck at
+  // a local fixed point.
+  TextPosition _moveBeyondTextBoundaryAtDirection(TextPosition end, bool forward, TextBoundary textBoundary) {
+    final int newOffset = forward
+      ? textBoundary.getTrailingTextBoundaryAt(end.offset) ?? range.end
+      : textBoundary.getLeadingTextBoundaryAt(end.offset - 1) ?? range.start;
+    return TextPosition(offset: newOffset);
+  }
+
+  // Move **to** the local boundary of the given type. Typically used for line
+  // boundaries, such that performing "move to line start" more than once never
+  // moves the selection to the previous line.
+  TextPosition _moveToTextBoundaryAtDirection(TextPosition end, bool forward, TextBoundary textBoundary) {
+    assert(end.offset >= 0);
+    final int caretOffset;
+    switch (end.affinity) {
+      case TextAffinity.upstream:
+        if (end.offset < 1 && !forward) {
+          assert (end.offset == 0);
+          return const TextPosition(offset: 0);
+        }
+        final CharacterBoundary characterBoundary = CharacterBoundary(fullText);
+        caretOffset = math.max(
+          0,
+          characterBoundary.getLeadingTextBoundaryAt(range.start + end.offset) ?? range.start,
+        ) - 1;
+        break;
+      case TextAffinity.downstream:
+        caretOffset = end.offset;
+        break;
     }
-    return _clampTextPosition(
-      (PushTextPosition.backward + boundary).getLeadingTextBoundaryAt(position),
-    );
+    final int offset = forward
+      ? textBoundary.getTrailingTextBoundaryAt(caretOffset) ?? range.end
+      : textBoundary.getLeadingTextBoundaryAt(caretOffset) ?? range.start;
+    return TextPosition(offset: offset);
   }
 
   MapEntry<TextPosition, SelectionResult> _handleVerticalMovement(TextPosition position, {required double horizontalBaselineInParagraphCoordinates, required bool below}) {