fix Selection handles position is off (#34665)

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

@@ -11,6 +11,7 @@ import 'package:flutter/semantics.dart';
 import 'package:flutter/services.dart';
 
 import 'box.dart';
+import 'layer.dart';
 import 'object.dart';
 import 'viewport_offset.dart';
 
@@ -143,6 +144,8 @@ class RenderEditable extends RenderBox {
     Color backgroundCursorColor,
     ValueNotifier<bool> showCursor,
     bool hasFocus,
+    @required LayerLink startHandleLayerLink,
+    @required LayerLink endHandleLayerLink,
     int maxLines = 1,
     int minLines,
     bool expands = false,
@@ -168,6 +171,8 @@ class RenderEditable extends RenderBox {
        assert(textDirection != null, 'RenderEditable created without a textDirection.'),
        assert(maxLines == null || maxLines > 0),
        assert(minLines == null || minLines > 0),
+       assert(startHandleLayerLink != null),
+       assert(endHandleLayerLink != null),
        assert(
          (maxLines == null) || (minLines == null) || (maxLines >= minLines),
          'minLines can\'t be greater than maxLines',
@@ -209,6 +214,8 @@ class RenderEditable extends RenderBox {
        _floatingCursorAddedMargin = floatingCursorAddedMargin,
        _enableInteractiveSelection = enableInteractiveSelection,
        _devicePixelRatio = devicePixelRatio,
+       _startHandleLayerLink = startHandleLayerLink,
+       _endHandleLayerLink = endHandleLayerLink,
        _obscureText = obscureText {
     assert(_showCursor != null);
     assert(!_showCursor.value || cursorColor != null);
@@ -903,6 +910,32 @@ class RenderEditable extends RenderBox {
     markNeedsPaint();
   }
 
+  /// The [LayerLink] of start selection handle.
+  ///
+  /// [RenderEditable] is responsible for calculating the [Offset] of this
+  /// [LayerLink], which will be used as [CompositedTransformTarget] of start handle.
+  LayerLink get startHandleLayerLink => _startHandleLayerLink;
+  LayerLink _startHandleLayerLink;
+  set startHandleLayerLink(LayerLink value) {
+    if (_startHandleLayerLink == value)
+      return;
+    _startHandleLayerLink = value;
+    markNeedsPaint();
+  }
+
+  /// The [LayerLink] of end selection handle.
+  ///
+  /// [RenderEditable] is responsible for calculating the [Offset] of this
+  /// [LayerLink], which will be used as [CompositedTransformTarget] of end handle.
+  LayerLink get endHandleLayerLink => _endHandleLayerLink;
+  LayerLink _endHandleLayerLink;
+  set endHandleLayerLink(LayerLink value) {
+    if (_endHandleLayerLink == value)
+      return;
+    _endHandleLayerLink = value;
+    markNeedsPaint();
+  }
+
   /// The padding applied to text field. Used to determine the bounds when
   /// moving the floating cursor.
   ///
@@ -1765,6 +1798,30 @@ class RenderEditable extends RenderBox {
     }
   }
 
+  void _paintHandleLayers(PaintingContext context, List<TextSelectionPoint> endpoints) {
+    Offset startPoint = endpoints[0].point;
+    startPoint = Offset(
+      startPoint.dx.clamp(0.0, size.width),
+      startPoint.dy.clamp(0.0, size.height),
+    );
+    context.pushLayer(
+      LeaderLayer(link: startHandleLayerLink, offset: startPoint),
+      super.paint,
+      Offset.zero,
+    );
+    if (endpoints.length == 2) {
+      Offset endPoint = endpoints[1].point;
+      endPoint = Offset(
+        endPoint.dx.clamp(0.0, size.width),
+        endPoint.dy.clamp(0.0, size.height),
+      );
+      context.pushLayer(
+        LeaderLayer(link: endHandleLayerLink, offset: endPoint),
+        super.paint,
+        Offset.zero,
+      );
+    }
+  }
   @override
   void paint(PaintingContext context, Offset offset) {
     _layoutText(constraints.maxWidth);
@@ -1772,6 +1829,7 @@ class RenderEditable extends RenderBox {
       context.pushClipRect(needsCompositing, offset, Offset.zero & size, _paintContents);
     else
       _paintContents(context, offset);
+    _paintHandleLayers(context, getEndpointsForSelection(selection));
   }
 
   @override

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

@@ -848,7 +848,10 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
 
   AnimationController _cursorBlinkOpacityController;
 
-  final LayerLink _layerLink = LayerLink();
+  final LayerLink _toolbarLayerLink = LayerLink();
+  final LayerLink _startHandleLayerLink = LayerLink();
+  final LayerLink _endHandleLayerLink = LayerLink();
+
   bool _didAutoFocus = false;
   FocusAttachment _focusAttachment;
 
@@ -1226,7 +1229,9 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
         context: context,
         value: _value,
         debugRequiredFor: widget,
-        layerLink: _layerLink,
+        toolbarLayerLink: _toolbarLayerLink,
+        startHandleLayerLink: _startHandleLayerLink,
+        endHandleLayerLink: _endHandleLayerLink,
         renderObject: renderObject,
         selectionControls: widget.selectionControls,
         selectionDelegate: this,
@@ -1541,13 +1546,15 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
       dragStartBehavior: widget.dragStartBehavior,
       viewportBuilder: (BuildContext context, ViewportOffset offset) {
         return CompositedTransformTarget(
-          link: _layerLink,
+          link: _toolbarLayerLink,
           child: Semantics(
             onCopy: _semanticsOnCopy(controls),
             onCut: _semanticsOnCut(controls),
             onPaste: _semanticsOnPaste(controls),
             child: _Editable(
               key: _editableKey,
+              startHandleLayerLink: _startHandleLayerLink,
+              endHandleLayerLink: _endHandleLayerLink,
               textSpan: buildTextSpan(),
               value: _value,
               cursorColor: _cursorColor,
@@ -1624,6 +1631,8 @@ class _Editable extends LeafRenderObjectWidget {
     Key key,
     this.textSpan,
     this.value,
+    this.startHandleLayerLink,
+    this.endHandleLayerLink,
     this.cursorColor,
     this.backgroundCursorColor,
     this.showCursor,
@@ -1657,6 +1666,8 @@ class _Editable extends LeafRenderObjectWidget {
   final TextSpan textSpan;
   final TextEditingValue value;
   final Color cursorColor;
+  final LayerLink startHandleLayerLink;
+  final LayerLink endHandleLayerLink;
   final Color backgroundCursorColor;
   final ValueNotifier<bool> showCursor;
   final bool hasFocus;
@@ -1688,6 +1699,8 @@ class _Editable extends LeafRenderObjectWidget {
     return RenderEditable(
       text: textSpan,
       cursorColor: cursorColor,
+      startHandleLayerLink: startHandleLayerLink,
+      endHandleLayerLink: endHandleLayerLink,
       backgroundCursorColor: backgroundCursorColor,
       showCursor: showCursor,
       hasFocus: hasFocus,
@@ -1721,6 +1734,8 @@ class _Editable extends LeafRenderObjectWidget {
     renderObject
       ..text = textSpan
       ..cursorColor = cursorColor
+      ..startHandleLayerLink = startHandleLayerLink
+      ..endHandleLayerLink = endHandleLayerLink
       ..showCursor = showCursor
       ..hasFocus = hasFocus
       ..maxLines = maxLines

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

@@ -269,7 +269,9 @@ class TextSelectionOverlay {
     @required TextEditingValue value,
     @required this.context,
     this.debugRequiredFor,
-    @required this.layerLink,
+    @required this.toolbarLayerLink,
+    @required this.startHandleLayerLink,
+    @required this.endHandleLayerLink,
     @required this.renderObject,
     this.selectionControls,
     bool handlesVisible = false,
@@ -300,7 +302,15 @@ class TextSelectionOverlay {
 
   /// The object supplied to the [CompositedTransformTarget] that wraps the text
   /// field.
-  final LayerLink layerLink;
+  final LayerLink toolbarLayerLink;
+
+  /// The objects supplied to the [CompositedTransformTarget] that wraps the
+  /// location of start selection handle.
+  final LayerLink startHandleLayerLink;
+
+  /// The objects supplied to the [CompositedTransformTarget] that wraps the
+  /// location of end selection handle.
+  final LayerLink endHandleLayerLink;
 
   // TODO(mpcomplete): what if the renderObject is removed or replaced, or
   // moves? Not sure what cases I need to handle, or how to handle them.
@@ -499,7 +509,8 @@ class TextSelectionOverlay {
       child: _TextSelectionHandleOverlay(
         onSelectionHandleChanged: (TextSelection newSelection) { _handleSelectionHandleChanged(newSelection, position); },
         onSelectionHandleTapped: onSelectionHandleTapped,
-        layerLink: layerLink,
+        startHandleLayerLink: startHandleLayerLink,
+        endHandleLayerLink: endHandleLayerLink,
         renderObject: renderObject,
         selection: _selection,
         selectionControls: selectionControls,
@@ -539,7 +550,7 @@ class TextSelectionOverlay {
     return FadeTransition(
       opacity: _toolbarOpacity,
       child: CompositedTransformFollower(
-        link: layerLink,
+        link: toolbarLayerLink,
         showWhenUnlinked: false,
         offset: -editingRegion.topLeft,
         child: selectionControls.buildToolbar(
@@ -575,7 +586,8 @@ class _TextSelectionHandleOverlay extends StatefulWidget {
     Key key,
     @required this.selection,
     @required this.position,
-    @required this.layerLink,
+    @required this.startHandleLayerLink,
+    @required this.endHandleLayerLink,
     @required this.renderObject,
     @required this.onSelectionHandleChanged,
     @required this.onSelectionHandleTapped,
@@ -585,7 +597,8 @@ class _TextSelectionHandleOverlay extends StatefulWidget {
 
   final TextSelection selection;
   final _TextSelectionHandlePosition position;
-  final LayerLink layerLink;
+  final LayerLink startHandleLayerLink;
+  final LayerLink endHandleLayerLink;
   final RenderEditable renderObject;
   final ValueChanged<TextSelection> onSelectionHandleChanged;
   final VoidCallback onSelectionHandleTapped;
@@ -696,30 +709,30 @@ class _TextSelectionHandleOverlayState
 
   @override
   Widget build(BuildContext context) {
-    final List<TextSelectionPoint> endpoints = widget.renderObject.getEndpointsForSelection(widget.selection);
-    Offset point;
+    LayerLink layerLink;
     TextSelectionHandleType type;
 
     switch (widget.position) {
       case _TextSelectionHandlePosition.start:
-        point = endpoints[0].point;
-        type = _chooseType(endpoints[0], TextSelectionHandleType.left, TextSelectionHandleType.right);
+        layerLink = widget.startHandleLayerLink;
+        type = _chooseType(
+          widget.renderObject.textDirection,
+          TextSelectionHandleType.left,
+          TextSelectionHandleType.right,
+        );
         break;
       case _TextSelectionHandlePosition.end:
-        // [endpoints] will only contain 1 point for collapsed selections, in
-        // which case we shouldn't be building the [end] handle.
-        assert(endpoints.length == 2);
-        point = endpoints[1].point;
-        type = _chooseType(endpoints[1], TextSelectionHandleType.right, TextSelectionHandleType.left);
+        // For collapsed selections, we shouldn't be building the [end] handle.
+        assert(!widget.selection.isCollapsed);
+        layerLink = widget.endHandleLayerLink;
+        type = _chooseType(
+          widget.renderObject.textDirection,
+          TextSelectionHandleType.right,
+          TextSelectionHandleType.left,
+        );
         break;
     }
 
-    final Size viewport = widget.renderObject.size;
-    point = Offset(
-      point.dx.clamp(0.0, viewport.width),
-      point.dy.clamp(0.0, viewport.height),
-    );
-
     final Offset handleAnchor = widget.selectionControls.getHandleAnchor(
       type,
       widget.renderObject.preferredLineHeight,
@@ -727,10 +740,10 @@ class _TextSelectionHandleOverlayState
     final Size handleSize = widget.selectionControls.getHandleSize(
       widget.renderObject.preferredLineHeight,
     );
+
     final Rect handleRect = Rect.fromLTWH(
-      // Put handleAnchor on top of point
-      point.dx - handleAnchor.dx,
-      point.dy - handleAnchor.dy,
+      -handleAnchor.dx,
+      -handleAnchor.dy,
       handleSize.width,
       handleSize.height,
     );
@@ -747,7 +760,7 @@ class _TextSelectionHandleOverlayState
     );
 
     return CompositedTransformFollower(
-      link: widget.layerLink,
+      link: layerLink,
       offset: interactiveRect.topLeft,
       showWhenUnlinked: false,
       child: FadeTransition(
@@ -782,15 +795,15 @@ class _TextSelectionHandleOverlayState
   }
 
   TextSelectionHandleType _chooseType(
-    TextSelectionPoint endpoint,
+    TextDirection textDirection,
     TextSelectionHandleType ltrType,
     TextSelectionHandleType rtlType,
   ) {
     if (widget.selection.isCollapsed)
       return TextSelectionHandleType.collapsed;
 
-    assert(endpoint.direction != null);
-    switch (endpoint.direction) {
+    assert(textDirection != null);
+    switch (textDirection) {
       case TextDirection.ltr:
         return ltrType;
       case TextDirection.rtl:

packages/flutter/test/rendering/editable_test.dart

@@ -34,6 +34,8 @@ void main() {
         style: TextStyle(height: 1.0, fontSize: 10.0, fontFamily: 'Ahem'),
         text: '12345',
       ),
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
       textAlign: TextAlign.start,
       textDirection: TextDirection.ltr,
       locale: const Locale('ja', 'JP'),
@@ -84,11 +86,16 @@ void main() {
         style: TextStyle(height: 1.0, fontSize: 10.0, fontFamily: 'Ahem'),
         text: 'A',
       ),
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
       textAlign: TextAlign.start,
       textDirection: TextDirection.ltr,
       locale: const Locale('en', 'US'),
       offset: ViewportOffset.fixed(10.0),
       textSelectionDelegate: delegate,
+      selection: const TextSelection.collapsed(
+        offset: 0,
+      ),
     );
     editable.layout(BoxConstraints.loose(const Size(1000.0, 1000.0)));
     expect(
@@ -114,6 +121,8 @@ void main() {
           height: 1.0, fontSize: 10.0, fontFamily: 'Ahem',
         ),
       ),
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
       selection: const TextSelection.collapsed(
         offset: 4,
         affinity: TextAffinity.upstream,
@@ -187,6 +196,8 @@ void main() {
           height: 1.0, fontSize: 10.0, fontFamily: 'Ahem',
         ),
       ),
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
       selection: const TextSelection.collapsed(
         offset: 4,
         affinity: TextAffinity.upstream,
@@ -258,6 +269,8 @@ void main() {
           height: 1.0, fontSize: 10.0, fontFamily: 'Ahem',
         ),
       ),
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
       selection: const TextSelection(
         baseOffset: 0,
         extentOffset: 3,
@@ -297,6 +310,8 @@ void main() {
           height: 1.0, fontSize: 10.0, fontFamily: 'Ahem',
         ),
       ),
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
       selection: const TextSelection.collapsed(
         offset: 2,
         affinity: TextAffinity.upstream,
@@ -345,12 +360,17 @@ void main() {
       onSelectionChanged: (TextSelection selection, RenderEditable renderObject, SelectionChangedCause cause) {
         currentSelection = selection;
       },
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
       text: const TextSpan(
         text: 'test\ntest',
         style: TextStyle(
           height: 1.0, fontSize: 10.0, fontFamily: 'Ahem',
         ),
       ),
+      selection: const TextSelection.collapsed(
+        offset: 4,
+      ),
     );
 
     layout(editable);
@@ -429,6 +449,8 @@ void main() {
           height: 1.0, fontSize: 10.0, fontFamily: 'Ahem',
         ),
       ),
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
     );
 
     layout(editable);
@@ -461,6 +483,8 @@ void main() {
         selectionChangedCount++;
         updatedSelection = selection;
       },
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
       text: text,
     );
 
@@ -483,6 +507,8 @@ void main() {
         updatedSelection = selection;
       },
       text: text,
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
     );
 
     layout(editable2);
@@ -510,6 +536,8 @@ void main() {
       offset: ViewportOffset.zero(),
       textSelectionDelegate: delegate,
       hasFocus: true,
+      startHandleLayerLink: LayerLink(),
+      endHandleLayerLink: LayerLink(),
     );
 
     expect(editable.hasFocus, true);

packages/flutter/test/widgets/editable_text_test.dart

@@ -268,6 +268,66 @@ void main() {
         equals('TextInputAction.newline'));
   });
 
+  testWidgets('selection overlay will update when text grow bigger', (WidgetTester tester) async {
+    final TextEditingController controller = TextEditingController.fromValue(
+        const TextEditingValue(
+          text: 'initial value',
+        )
+    );
+    Future<void> pumpEditableTextWithTextStyle(TextStyle style) async {
+      await tester.pumpWidget(
+        MaterialApp(
+          home: EditableText(
+            backgroundCursorColor: Colors.grey,
+            controller: controller,
+            focusNode: focusNode,
+            style: style,
+            cursorColor: cursorColor,
+            selectionControls: materialTextSelectionControls,
+            showSelectionHandles: true,
+          ),
+        ),
+      );
+    }
+
+    await pumpEditableTextWithTextStyle(const TextStyle(fontSize: 18));
+    final EditableTextState state = tester.state<EditableTextState>(find.byType(EditableText));
+    state.renderEditable.selectWordsInRange(
+      from: const Offset(0, 0),
+      cause: SelectionChangedCause.longPress,
+    );
+    await tester.pumpAndSettle();
+    await tester.idle();
+
+    List<RenderBox> handles = List<RenderBox>.from(
+      tester.renderObjectList<RenderBox>(
+        find.descendant(
+          of: find.byType(CompositedTransformFollower),
+          matching: find.byType(GestureDetector),
+        ),
+      ),
+    );
+
+    expect(handles[0].localToGlobal(Offset.zero), const Offset(-35.0, 5.0));
+    expect(handles[1].localToGlobal(Offset.zero), const Offset(113.0, 5.0));
+
+    await pumpEditableTextWithTextStyle(const TextStyle(fontSize: 30));
+    await tester.pumpAndSettle();
+
+    // Handles should be updated with bigger font size.
+    handles = List<RenderBox>.from(
+      tester.renderObjectList<RenderBox>(
+        find.descendant(
+          of: find.byType(CompositedTransformFollower),
+          matching: find.byType(GestureDetector),
+        ),
+      ),
+    );
+    // First handle should have the same dx but bigger dy.
+    expect(handles[0].localToGlobal(Offset.zero), const Offset(-35.0, 17.0));
+    expect(handles[1].localToGlobal(Offset.zero), const Offset(197.0, 17.0));
+  });
+
   testWidgets('Multiline keyboard with newline action is requested when maxLines = null', (WidgetTester tester) async {
     await tester.pumpWidget(
       MediaQuery(
@@ -1908,7 +1968,7 @@ void main() {
     ));
 
     final EditableTextState state =
-        tester.state<EditableTextState>(find.byType(EditableText));
+      tester.state<EditableTextState>(find.byType(EditableText));
     final RenderEditable renderEditable = state.renderEditable;
     final Scrollable scrollable = tester.widget<Scrollable>(find.byType(Scrollable));
 
@@ -1963,12 +2023,14 @@ void main() {
 
       // Check that the handles' positions are correct.
 
-      final List<CompositedTransformFollower> container =
-        find.byType(CompositedTransformFollower)
-          .evaluate()
-          .map((Element e) => e.widget)
-          .cast<CompositedTransformFollower>()
-          .toList();
+      final List<RenderBox> handles = List<RenderBox>.from(
+        tester.renderObjectList<RenderBox>(
+          find.descendant(
+            of: find.byType(CompositedTransformFollower),
+            matching: find.byType(GestureDetector),
+          ),
+        ),
+      );
 
       final Size viewport = renderEditable.size;
 
@@ -2006,8 +2068,8 @@ void main() {
         }
       }
       expect(state.selectionOverlay.handlesAreVisible, isTrue);
-      testPosition(container[0].offset.dx, leftPosition);
-      testPosition(container[1].offset.dx, rightPosition);
+      testPosition(handles[0].localToGlobal(Offset.zero).dx, leftPosition);
+      testPosition(handles[1].localToGlobal(Offset.zero).dx, rightPosition);
     }
 
     // Select the first word. Both handles should be visible.
@@ -2080,21 +2142,23 @@ void main() {
     await tester.tapAt(const Offset(20, 10));
     state.renderEditable.selectWord(cause: SelectionChangedCause.longPress);
     await tester.pump();
-    final List<CompositedTransformFollower> container =
-      find.byType(CompositedTransformFollower)
-        .evaluate()
-        .map((Element e) => e.widget)
-        .cast<CompositedTransformFollower>()
-        .toList();
+    final List<RenderBox> handles = List<RenderBox>.from(
+      tester.renderObjectList<RenderBox>(
+        find.descendant(
+          of: find.byType(CompositedTransformFollower),
+          matching: find.byType(GestureDetector),
+        ),
+      ),
+    );
     expect(
-      container[0].offset.dx,
+      handles[0].localToGlobal(Offset.zero).dx,
       inExclusiveRange(
         -kMinInteractiveSize,
         kMinInteractiveSize,
       ),
     );
     expect(
-      container[1].offset.dx,
+      handles[1].localToGlobal(Offset.zero).dx,
       inExclusiveRange(
         70.0 - kMinInteractiveSize,
         70.0 + kMinInteractiveSize,
@@ -2188,12 +2252,14 @@ void main() {
 
       // Check that the handles' positions are correct.
 
-      final List<CompositedTransformFollower> container =
-        find.byType(CompositedTransformFollower)
-          .evaluate()
-          .map((Element e) => e.widget)
-          .cast<CompositedTransformFollower>()
-          .toList();
+      final List<RenderBox> handles = List<RenderBox>.from(
+        tester.renderObjectList<RenderBox>(
+          find.descendant(
+            of: find.byType(CompositedTransformFollower),
+            matching: find.byType(GestureDetector),
+          ),
+        ),
+      );
 
       final Size viewport = renderEditable.size;
 
@@ -2231,8 +2297,8 @@ void main() {
         }
       }
       expect(state.selectionOverlay.handlesAreVisible, isTrue);
-      testPosition(container[0].offset.dx, leftPosition);
-      testPosition(container[1].offset.dx, rightPosition);
+      testPosition(handles[0].localToGlobal(Offset.zero).dx, leftPosition);
+      testPosition(handles[1].localToGlobal(Offset.zero).dx, rightPosition);
     }
 
     // Select the first word. Both handles should be visible.