review

Author: LongCatIsLooong

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

@@ -1583,7 +1583,7 @@ class _SelectableFragment with Selectable, ChangeNotifier implements TextLayoutM
     switch (granularity) {
       case TextGranularity.character:
         final String text = range.textInside(fullText);
-        newPosition = _beyondTextBoundary(targetedEdge, forward, CharacterBoundary(text));
+        newPosition = _moveBeyondTextBoundaryAtDirection(targetedEdge, forward, CharacterBoundary(text));
         result = SelectionResult.end;
         break;
       case TextGranularity.word:
@@ -1592,16 +1592,16 @@ class _SelectableFragment with Selectable, ChangeNotifier implements TextLayoutM
           return (forward ? offset >= text.length : offset == 0)
               || !TextLayoutMetrics.isWhitespace(text.codeUnitAt(forward ? offset - 1 : offset));
         });
-        newPosition = _beyondTextBoundary(targetedEdge, forward, textBoundary);
+        newPosition = _moveBeyondTextBoundaryAtDirection(targetedEdge, forward, textBoundary);
         result = SelectionResult.end;
         break;
       case TextGranularity.line:
-        newPosition = _toTextBoundary(targetedEdge, forward, LineBoundary(this));
+        newPosition = _moveToTextBoundaryAtDirection(targetedEdge, forward, LineBoundary(this));
         result = SelectionResult.end;
         break;
       case TextGranularity.document:
         final String text = range.textInside(fullText);
-        newPosition = _beyondTextBoundary(targetedEdge, forward, DocumentBoundary(text));
+        newPosition = _moveBeyondTextBoundaryAtDirection(targetedEdge, forward, DocumentBoundary(text));
         if (forward && newPosition.offset == range.end) {
           result = SelectionResult.next;
         } else if (!forward && newPosition.offset == range.start) {
@@ -1620,26 +1620,37 @@ class _SelectableFragment with Selectable, ChangeNotifier implements TextLayoutM
     return result;
   }
 
-  TextPosition _beyondTextBoundary(TextPosition extent, bool forward, TextBoundary textBoundary) {
+  // 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(extent.offset) ?? range.end
-      : textBoundary.getLeadingTextBoundaryAt(extent.offset - 1) ?? range.start;
+      ? textBoundary.getTrailingTextBoundaryAt(end.offset) ?? range.end
+      : textBoundary.getLeadingTextBoundaryAt(end.offset - 1) ?? range.start;
     return TextPosition(offset: newOffset);
   }
 
-  TextPosition _toTextBoundary(TextPosition extent, bool forward, TextBoundary textBoundary) {
-    assert(extent.offset >= 0);
+  // 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 (extent.affinity) {
+    switch (end.affinity) {
       case TextAffinity.upstream:
-        if (extent.offset < 1 && !forward) {
-          assert (extent.offset == 0);
+        if (end.offset < 1 && !forward) {
+          assert (end.offset == 0);
           return const TextPosition(offset: 0);
         }
-        caretOffset = math.max(0, extent.offset - 1);
+        final CharacterBoundary characterBoundary = CharacterBoundary(fullText);
+        caretOffset = math.max(
+          0,
+          characterBoundary.getLeadingTextBoundaryAt(range.start + end.offset) ?? range.start,
+        ) - 1;
         break;
       case TextAffinity.downstream:
-        caretOffset = extent.offset;
+        caretOffset = end.offset;
         break;
     }
     final int offset = forward

packages/flutter/lib/src/widgets/editable_text.dart

@@ -69,6 +69,10 @@ typedef EditableTextContextMenuBuilder = Widget Function(
   EditableTextState editableTextState,
 );
 
+// Signature for a function that determines the target location of the given
+// [TextPosition] after applying the given [TextBoundary].
+typedef _ApplyTextBoundary = TextPosition Function(TextPosition, bool, TextBoundary);
+
 // The time it takes for the cursor to fade from fully opaque to fully
 // transparent and vice versa. A full cursor blink, from transparent to opaque
 // to transparent, is twice this duration.
@@ -4716,11 +4720,11 @@ class _CodeUnitBoundary extends TextBoundary {
 
 // -------------------------------  Text Actions -------------------------------
 class _DeleteTextAction<T extends DirectionalTextEditingIntent> extends ContextAction<T> {
-  _DeleteTextAction(this.state, this.textBoundary, this._applyTextBoundary);
+  _DeleteTextAction(this.state, this.getTextBoundary, this._applyTextBoundary);
 
   final EditableTextState state;
-  final TextBoundary Function() textBoundary;
-  final TextPosition Function(TextPosition, bool, TextBoundary) _applyTextBoundary;
+  final TextBoundary Function() getTextBoundary;
+  final _ApplyTextBoundary _applyTextBoundary;
 
   @override
   Object? invoke(T intent, [BuildContext? context]) {
@@ -4743,7 +4747,7 @@ class _DeleteTextAction<T extends DirectionalTextEditingIntent> extends ContextA
       );
     }
 
-    final int target = _applyTextBoundary(selection.base, intent.forward, textBoundary()).offset;
+    final int target = _applyTextBoundary(selection.base, intent.forward, getTextBoundary()).offset;
 
     final TextRange rangeToDelete = TextSelection(
       baseOffset: intent.forward
@@ -4764,7 +4768,7 @@ class _DeleteTextAction<T extends DirectionalTextEditingIntent> extends ContextA
 class _UpdateTextSelectionAction<T extends DirectionalCaretMovementIntent> extends ContextAction<T> {
   _UpdateTextSelectionAction(
     this.state,
-    this.textBoundary,
+    this.getTextBoundary,
     this.applyTextBoundary, {
     required this.ignoreNonCollapsedSelection,
     this.isExpand = false,
@@ -4775,8 +4779,8 @@ class _UpdateTextSelectionAction<T extends DirectionalCaretMovementIntent> exten
   final bool ignoreNonCollapsedSelection;
   final bool isExpand;
   final bool extentAtIndex;
-  final TextBoundary Function() textBoundary;
-  final TextPosition Function(TextPosition, bool, TextBoundary) applyTextBoundary;
+  final TextBoundary Function() getTextBoundary;
+  final _ApplyTextBoundary applyTextBoundary;
 
   static const int NEWLINE_CODE_UNIT = 10;
 
@@ -4832,7 +4836,7 @@ class _UpdateTextSelectionAction<T extends DirectionalCaretMovementIntent> exten
     }
 
     final bool shouldTargetBase = isExpand && (intent.forward ? selection.baseOffset > selection.extentOffset : selection.baseOffset < selection.extentOffset);
-    final TextPosition newExtent = applyTextBoundary(shouldTargetBase ? selection.base : extent, intent.forward, textBoundary());
+    final TextPosition newExtent = applyTextBoundary(shouldTargetBase ? selection.base : extent, intent.forward, getTextBoundary());
     final TextSelection newSelection = collapseSelection || (!isExpand && newExtent.offset == selection.baseOffset)
       ? TextSelection.fromPosition(newExtent)
       : isExpand ? selection.expandTo(newExtent, extentAtIndex || selection.isCollapsed) : selection.extendTo(newExtent);