Track and use fallback TextAffinity for null affinity platform TextSe…

…lections. (#44622)
flutter · Nov 21, 2019 · 6b66d79 · 6b66d79
1 parent 384a44d
commit 6b66d79
Show file tree

Hide file tree

Showing 7 changed files with 139 additions and 8 deletions.
diff --git a/packages/flutter/lib/src/painting/text_painter.dart b/packages/flutter/lib/src/painting/text_painter.dart
@@ -819,9 +819,14 @@ class TextPainter {
     return _paragraph.getWordBoundary(position);
   }
 
-  /// Returns the text range of the line at the given offset.
+  /// Returns the [TextRange] of the line at the given [TextPosition].
   ///
-  /// The newline, if any, is included in the range.
+  /// The newline, if any, is returned as part of the range.
+  ///
+  /// Not valid until after layout.
+  ///
+  /// This can potentially be expensive, since it needs to compute the full
+  /// layout before it is available.
   TextRange getLineBoundary(TextPosition position) {
     assert(!_needsLayout);
     return _paragraph.getLineBoundary(position);

diff --git a/packages/flutter/lib/src/rendering/editable.dart b/packages/flutter/lib/src/rendering/editable.dart
@@ -402,6 +402,12 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
   // down in a multi-line text field when selecting using the keyboard.
   bool _wasSelectingVerticallyWithKeyboard = false;
 
+  // This is the affinity we use when a platform-supplied value has null
+  // affinity.
+  //
+  // This affinity should never be null.
+  TextAffinity _fallbackAffinity = TextAffinity.downstream;
+
   // Call through to onSelectionChanged.
   void _handleSelectionChange(
     TextSelection nextSelection,
@@ -418,6 +424,17 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
     }
   }
 
+  // Sets the fallback affinity to the affinity of the selection.
+  void _setFallbackAffinity(
+    TextAffinity affinity,
+  ) {
+    assert(affinity != null);
+    // Engine-computed selections will always compute affinity when necessary.
+    // Cache this affinity in the case where the platform supplied selection
+    // does not provide an affinity.
+    _fallbackAffinity = affinity;
+  }
+
   static final Set<LogicalKeyboardKey> _movementKeys = <LogicalKeyboardKey>{
     LogicalKeyboardKey.arrowRight,
     LogicalKeyboardKey.arrowLeft,
@@ -963,7 +980,15 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
   set selection(TextSelection value) {
     if (_selection == value)
       return;
-    _selection = value;
+    // Use the _fallbackAffinity when the set selection has a null
+    // affinity. This happens when the platform does not supply affinity,
+    // in which case using the fallback affinity computed from dart:ui will
+    // be superior to simply defaulting to TextAffinity.downstream.
+    if (value.affinity == null) {
+      _selection = value.copyWith(affinity: _fallbackAffinity);
+    } else {
+      _selection = value;
+    }
     _selectionRects = null;
     markNeedsPaint();
     markNeedsSemanticsUpdate();
@@ -1566,6 +1591,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
     );
     // Call [onSelectionChanged] only when the selection actually changed.
     _handleSelectionChange(newSelection, cause);
+    _setFallbackAffinity(newSelection.affinity);
   }
 
   /// Select a word around the location of the last tap down.
@@ -1614,15 +1640,18 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin {
       return;
     }
     final TextPosition position = _textPainter.getPositionForOffset(globalToLocal(_lastTapDownPosition - _paintOffset));
+    _setFallbackAffinity(position.affinity);
     final TextRange word = _textPainter.getWordBoundary(position);
+    final TextRange lineBoundary = _textPainter.getLineBoundary(position);
+    final bool endOfLine = lineBoundary?.end == position.offset && position.affinity != null;
     if (position.offset - word.start <= 1) {
       _handleSelectionChange(
-        TextSelection.collapsed(offset: word.start, affinity: TextAffinity.downstream),
+        TextSelection.collapsed(offset: word.start, affinity: endOfLine ? position.affinity : TextAffinity.downstream),
         cause,
       );
     } else {
       _handleSelectionChange(
-        TextSelection.collapsed(offset: word.end, affinity: TextAffinity.upstream),
+        TextSelection.collapsed(offset: word.end, affinity: endOfLine ? position.affinity : TextAffinity.upstream),
         cause,
       );
     }

diff --git a/packages/flutter/lib/src/services/text_editing.dart b/packages/flutter/lib/src/services/text_editing.dart
@@ -81,7 +81,7 @@ class TextSelection extends TextRange {
   /// The position at which the selection originates.
   ///
   /// Might be larger than, smaller than, or equal to extent.
-  TextPosition get base => TextPosition(offset: baseOffset, affinity: affinity);
+  TextPosition get base => TextPosition(offset: baseOffset, affinity: affinity ?? TextAffinity.downstream);
 
   /// The position at which the selection terminates.
   ///
@@ -90,7 +90,7 @@ class TextSelection extends TextRange {
   /// side of the selection, this is the location at which to paint the caret.
   ///
   /// Might be larger than, smaller than, or equal to base.
-  TextPosition get extent => TextPosition(offset: extentOffset, affinity: affinity);
+  TextPosition get extent => TextPosition(offset: extentOffset, affinity: affinity ?? TextAffinity.downstream);
 
   @override
   String toString() {

diff --git a/packages/flutter/lib/src/services/text_input.dart b/packages/flutter/lib/src/services/text_input.dart
@@ -472,6 +472,9 @@ TextAffinity _toTextAffinity(String affinity) {
     case 'TextAffinity.upstream':
       return TextAffinity.upstream;
   }
+  // Null affinity indicates that the platform did not provide a valid
+  // affinity. Set it to null here to allow the framework to supply
+  // a fallback affinity.
   return null;
 }
 
@@ -533,7 +536,7 @@ class TextEditingValue {
       selection: TextSelection(
         baseOffset: encoded['selectionBase'] ?? -1,
         extentOffset: encoded['selectionExtent'] ?? -1,
-        affinity: _toTextAffinity(encoded['selectionAffinity']) ?? TextAffinity.downstream,
+        affinity: _toTextAffinity(encoded['selectionAffinity']),
         isDirectional: encoded['selectionIsDirectional'] ?? false,
       ),
       composing: TextRange(

diff --git a/packages/flutter/test/painting/text_painter_test.dart b/packages/flutter/test/painting/text_painter_test.dart
@@ -790,4 +790,42 @@ void main() {
     expect(lines[2].lineNumber, 2);
     expect(lines[3].lineNumber, 3);
   }, skip: !isLinux);
+
+  test('getLineBoundary', () {
+    final TextPainter painter = TextPainter()
+      ..textDirection = TextDirection.ltr;
+
+    const String text = 'test1\nhello line two really long for soft break\nfinal line 4';
+    painter.text = const TextSpan(
+      text: text,
+    );
+
+    painter.layout(maxWidth: 300);
+
+    final List<ui.LineMetrics> lines = painter.computeLineMetrics();
+
+    expect(lines.length, 4);
+
+    expect(painter.getLineBoundary(const TextPosition(offset: -1)), const TextRange(start: -1, end: -1));
+
+    expect(painter.getLineBoundary(const TextPosition(offset: 0)), const TextRange(start: 0, end: 5));
+    expect(painter.getLineBoundary(const TextPosition(offset: 1)), const TextRange(start: 0, end: 5));
+    expect(painter.getLineBoundary(const TextPosition(offset: 4)), const TextRange(start: 0, end: 5));
+    expect(painter.getLineBoundary(const TextPosition(offset: 5)), const TextRange(start: 0, end: 5));
+
+    expect(painter.getLineBoundary(const TextPosition(offset: 10)), const TextRange(start: 6, end: 28));
+    expect(painter.getLineBoundary(const TextPosition(offset: 15)), const TextRange(start: 6, end: 28));
+    expect(painter.getLineBoundary(const TextPosition(offset: 21)), const TextRange(start: 6, end: 28));
+    expect(painter.getLineBoundary(const TextPosition(offset: 28)), const TextRange(start: 6, end: 28));
+
+    expect(painter.getLineBoundary(const TextPosition(offset: 29)), const TextRange(start: 28, end: 47));
+    expect(painter.getLineBoundary(const TextPosition(offset: 47)), const TextRange(start: 28, end: 47));
+
+    expect(painter.getLineBoundary(const TextPosition(offset: 48)), const TextRange(start: 48, end: 60));
+    expect(painter.getLineBoundary(const TextPosition(offset: 49)), const TextRange(start: 48, end: 60));
+    expect(painter.getLineBoundary(const TextPosition(offset: 60)), const TextRange(start: 48, end: 60));
+
+    expect(painter.getLineBoundary(const TextPosition(offset: 61)), const TextRange(start: -1, end: -1));
+    expect(painter.getLineBoundary(const TextPosition(offset: 100)), const TextRange(start: -1, end: -1));
+  }, skip: !isLinux);
 }
diff --git a/packages/flutter/test/rendering/editable_test.dart b/packages/flutter/test/rendering/editable_test.dart
@@ -611,4 +611,29 @@ void main() {
     editable.layout(BoxConstraints.loose(const Size(1000.0, 1000.0)));
     expect(editable.maxScrollExtent, equals(10));
   }, skip: isBrowser); // TODO(yjbanov): https://github.com/flutter/flutter/issues/42772
+
+  test('selection affinity uses fallback', () {
+    final TextSelectionDelegate delegate = FakeEditableTextState();
+    EditableText.debugDeterministicCursor = true;
+
+    final RenderEditable editable = RenderEditable(
+      textDirection: TextDirection.ltr,
+      cursorColor: const Color.fromARGB(0xFF, 0xFF, 0x00, 0x00),
+      offset: ViewportOffset.zero(),
+      textSelectionDelegate: delegate,
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
+    );
+
+    expect(editable.selection, null);
+
+    const TextSelection sel1 = TextSelection(baseOffset: 10, extentOffset: 11);
+    editable.selection = sel1;
+    expect(editable.selection, sel1);
+
+    const TextSelection sel2 = TextSelection(baseOffset: 10, extentOffset: 11, affinity: null);
+    const TextSelection sel3 = TextSelection(baseOffset: 10, extentOffset: 11, affinity: TextAffinity.downstream);
+    editable.selection = sel2;
+    expect(editable.selection, sel3);
+  }, skip: isBrowser);
 }
diff --git a/packages/flutter/test/services/text_input_test.dart b/packages/flutter/test/services/text_input_test.dart
@@ -173,6 +173,37 @@ void main() {
       expect(client.latestMethodCall, 'connectionClosed');
     });
   });
+
+  test('TextEditingValue handles JSON affinity', () async {
+    final Map<String, dynamic> json = <String, dynamic>{};
+    json['text'] = 'Xiaomuqiao';
+
+    TextEditingValue val = TextEditingValue.fromJSON(json);
+    expect(val.text, 'Xiaomuqiao');
+    expect(val.selection.baseOffset, -1);
+    expect(val.selection.extentOffset, -1);
+    expect(val.selection.affinity, null);
+    expect(val.selection.isDirectional, false);
+    expect(val.composing.start, -1);
+    expect(val.composing.end, -1);
+
+    json['text'] = 'Xiaomuqiao';
+    json['selectionBase'] = 5;
+    json['selectionExtent'] = 6;
+    json['selectionAffinity'] = 'TextAffinity.upstream';
+    json['selectionIsDirectional'] = true;
+    json['composingBase'] = 7;
+    json['composingExtent'] = 8;
+
+    val = TextEditingValue.fromJSON(json);
+    expect(val.text, 'Xiaomuqiao');
+    expect(val.selection.baseOffset, 5);
+    expect(val.selection.extentOffset, 6);
+    expect(val.selection.affinity, TextAffinity.upstream);
+    expect(val.selection.isDirectional, true);
+    expect(val.composing.start, 7);
+    expect(val.composing.end, 8);
+  });
 }
 
 class FakeTextInputClient implements TextInputClient {