Unverified Commit 0e15ab54 authored by Renzo Olivares's avatar Renzo Olivares Committed by GitHub

Triple tap selection should not move beyond text boundary at the tapped location (#132357)

This PR makes sure we do not select beyond the text boundary at the tapped position unless, we tap at the end of the text which in that case we should select the previous text boundary.

```dart
    // if x is a boundary defined by `textBoundary`, most textBoundaries (except
    // LineBreaker) guarantees `x == textBoundary.getLeadingTextBoundaryAt(x)`.
    // Use x - 1 here to make sure we don't get stuck at the fixed point x.
    final int start = textBoundary.getLeadingTextBoundaryAt(extent.offset - 1) ?? 0;
```

This was originally carried over from https://github.com/flutter/flutter/blob/f468f3366c26a5092eb964a230ce7892fda8f2f8/packages/flutter/lib/src/widgets/editable_text.dart#L4167-L4179 which used this `x - 1` to be able to move to the previous word boundary when navigating with a keyboard. When selecting by tapping/clicking we do not want to move past the text boundary at the tapped position so this adjustment is not needed.

Fixes #132126
parent 2cb762d1
...@@ -2567,14 +2567,13 @@ class TextSelectionGestureDetectorBuilder { ...@@ -2567,14 +2567,13 @@ class TextSelectionGestureDetectorBuilder {
_selectTextBoundariesInRange(boundary: lineBoundary, from: from, to: to, cause: cause); _selectTextBoundariesInRange(boundary: lineBoundary, from: from, to: to, cause: cause);
} }
// Returns the closest boundary location to `extent` but not including `extent` // Returns the location of a text boundary at `extent`. When `extent` is at
// itself. // the end of the text, returns the previous text boundary's location.
TextRange _moveBeyondTextBoundary(TextPosition extent, TextBoundary textBoundary) { TextRange _moveToTextBoundary(TextPosition extent, TextBoundary textBoundary) {
assert(extent.offset >= 0); assert(extent.offset >= 0);
// if x is a boundary defined by `textBoundary`, most textBoundaries (except // Use extent.offset - 1 when `extent` is at the end of the text to retrieve
// LineBreaker) guarantees `x == textBoundary.getLeadingTextBoundaryAt(x)`. // the previous text boundary's location.
// Use x - 1 here to make sure we don't get stuck at the fixed point x. final int start = textBoundary.getLeadingTextBoundaryAt(extent.offset == editableText.textEditingValue.text.length ? extent.offset - 1 : extent.offset) ?? 0;
final int start = textBoundary.getLeadingTextBoundaryAt(extent.offset - 1) ?? 0;
final int end = textBoundary.getTrailingTextBoundaryAt(extent.offset) ?? editableText.textEditingValue.text.length; final int end = textBoundary.getTrailingTextBoundaryAt(extent.offset) ?? editableText.textEditingValue.text.length;
return TextRange(start: start, end: end); return TextRange(start: start, end: end);
} }
...@@ -2589,13 +2588,13 @@ class TextSelectionGestureDetectorBuilder { ...@@ -2589,13 +2588,13 @@ class TextSelectionGestureDetectorBuilder {
// beginning and end of a text boundary respectively. // beginning and end of a text boundary respectively.
void _selectTextBoundariesInRange({required TextBoundary boundary, required Offset from, Offset? to, SelectionChangedCause? cause}) { void _selectTextBoundariesInRange({required TextBoundary boundary, required Offset from, Offset? to, SelectionChangedCause? cause}) {
final TextPosition fromPosition = renderEditable.getPositionForPoint(from); final TextPosition fromPosition = renderEditable.getPositionForPoint(from);
final TextRange fromRange = _moveBeyondTextBoundary(fromPosition, boundary); final TextRange fromRange = _moveToTextBoundary(fromPosition, boundary);
final TextPosition toPosition = to == null final TextPosition toPosition = to == null
? fromPosition ? fromPosition
: renderEditable.getPositionForPoint(to); : renderEditable.getPositionForPoint(to);
final TextRange toRange = toPosition == fromPosition final TextRange toRange = toPosition == fromPosition
? fromRange ? fromRange
: _moveBeyondTextBoundary(toPosition, boundary); : _moveToTextBoundary(toPosition, boundary);
final bool isFromBoundaryBeforeToBoundary = fromRange.start < toRange.end; final bool isFromBoundaryBeforeToBoundary = fromRange.start < toRange.end;
final TextSelection newSelection = isFromBoundaryBeforeToBoundary final TextSelection newSelection = isFromBoundaryBeforeToBoundary
......
...@@ -3804,6 +3804,121 @@ void main() { ...@@ -3804,6 +3804,121 @@ void main() {
variant: TargetPlatformVariant.mobile(), variant: TargetPlatformVariant.mobile(),
); );
testWidgets(
'Triple click at the beginning of a line should not select the previous paragraph',
(WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/132126
final TextEditingController controller = TextEditingController();
await tester.pumpWidget(
CupertinoApp(
home: Center(
child: CupertinoTextField(
dragStartBehavior: DragStartBehavior.down,
controller: controller,
maxLines: null,
),
),
),
);
await tester.enterText(find.byType(CupertinoTextField), testValueB);
// Skip past scrolling animation.
await tester.pump();
await tester.pump(const Duration(milliseconds: 200));
expect(controller.value.text, testValueB);
final Offset thirdLinePos = textOffsetToPosition(tester, 38);
// Click on text field to gain focus, and move the selection.
final TestGesture gesture = await tester.startGesture(thirdLinePos, kind: PointerDeviceKind.mouse);
await tester.pump();
await gesture.up();
await tester.pump();
expect(controller.selection.isCollapsed, true);
expect(controller.selection.baseOffset, 38);
// Here we click on same position again, to register a double click. This will select
// the word at the clicked position.
await gesture.down(thirdLinePos);
await gesture.up();
expect(controller.selection.baseOffset, 38);
expect(controller.selection.extentOffset, 40);
// Here we click on same position again, to register a triple click. This will select
// the paragraph at the clicked position.
await gesture.down(thirdLinePos);
await tester.pump();
await gesture.up();
await tester.pump();
await tester.pumpAndSettle();
expect(controller.selection.baseOffset, 38);
expect(controller.selection.extentOffset, 57);
},
variant: TargetPlatformVariant.all(excluding: <TargetPlatform>{ TargetPlatform.linux }),
);
testWidgets(
'Triple click at the end of text should select the previous paragraph',
(WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/132126.
final TextEditingController controller = TextEditingController();
await tester.pumpWidget(
CupertinoApp(
home: Center(
child: CupertinoTextField(
dragStartBehavior: DragStartBehavior.down,
controller: controller,
maxLines: null,
),
),
),
);
await tester.enterText(find.byType(CupertinoTextField), testValueB);
// Skip past scrolling animation.
await tester.pump();
await tester.pump(const Duration(milliseconds: 200));
expect(controller.value.text, testValueB);
final Offset endOfTextPos = textOffsetToPosition(tester, 74);
// Click on text field to gain focus, and move the selection.
final TestGesture gesture = await tester.startGesture(endOfTextPos, kind: PointerDeviceKind.mouse);
await tester.pump();
await gesture.up();
await tester.pump();
expect(controller.selection.isCollapsed, true);
expect(controller.selection.baseOffset, 74);
// Here we click on same position again, to register a double click.
await gesture.down(endOfTextPos);
await tester.pump();
await gesture.up();
await tester.pump();
expect(controller.selection.baseOffset, 74);
expect(controller.selection.extentOffset, 74);
// Here we click on same position again, to register a triple click. This will select
// the paragraph at the clicked position.
await gesture.down(endOfTextPos);
await tester.pump();
await gesture.up();
await tester.pump();
await tester.pumpAndSettle();
expect(controller.selection.baseOffset, 57);
expect(controller.selection.extentOffset, 74);
},
variant: TargetPlatformVariant.all(excluding: <TargetPlatform>{ TargetPlatform.linux }),
);
testWidgets( testWidgets(
'triple tap chains work on Non-Apple mobile platforms', 'triple tap chains work on Non-Apple mobile platforms',
(WidgetTester tester) async { (WidgetTester tester) async {
......
...@@ -9771,6 +9771,117 @@ void main() { ...@@ -9771,6 +9771,117 @@ void main() {
variant: TargetPlatformVariant.mobile(), variant: TargetPlatformVariant.mobile(),
); );
testWidgets(
'Triple click at the beginning of a line should not select the previous paragraph',
(WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/132126
final TextEditingController controller = TextEditingController();
await tester.pumpWidget(
MaterialApp(
home: Material(
child: TextField(
dragStartBehavior: DragStartBehavior.down,
controller: controller,
maxLines: null,
),
),
),
);
await tester.enterText(find.byType(TextField), testValueB);
await skipPastScrollingAnimation(tester);
expect(controller.value.text, testValueB);
final Offset thirdLinePos = textOffsetToPosition(tester, 38);
// Click on text field to gain focus, and move the selection.
final TestGesture gesture = await tester.startGesture(thirdLinePos, kind: PointerDeviceKind.mouse);
await tester.pump();
await gesture.up();
await tester.pump();
expect(controller.selection.isCollapsed, true);
expect(controller.selection.baseOffset, 38);
// Here we click on same position again, to register a double click. This will select
// the word at the clicked position.
await gesture.down(thirdLinePos);
await gesture.up();
expect(controller.selection.baseOffset, 38);
expect(controller.selection.extentOffset, 40);
// Here we click on same position again, to register a triple click. This will select
// the paragraph at the clicked position.
await gesture.down(thirdLinePos);
await tester.pump();
await gesture.up();
await tester.pump();
await tester.pumpAndSettle();
expect(controller.selection.baseOffset, 38);
expect(controller.selection.extentOffset, 57);
},
variant: TargetPlatformVariant.all(excluding: <TargetPlatform>{ TargetPlatform.linux }),
);
testWidgets(
'Triple click at the end of text should select the previous paragraph',
(WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/132126.
final TextEditingController controller = TextEditingController();
await tester.pumpWidget(
MaterialApp(
home: Material(
child: TextField(
dragStartBehavior: DragStartBehavior.down,
controller: controller,
maxLines: null,
),
),
),
);
await tester.enterText(find.byType(TextField), testValueB);
await skipPastScrollingAnimation(tester);
expect(controller.value.text, testValueB);
final Offset endOfTextPos = textOffsetToPosition(tester, 74);
// Click on text field to gain focus, and move the selection.
final TestGesture gesture = await tester.startGesture(endOfTextPos, kind: PointerDeviceKind.mouse);
await tester.pump();
await gesture.up();
await tester.pump();
expect(controller.selection.isCollapsed, true);
expect(controller.selection.baseOffset, 74);
// Here we click on same position again, to register a double click.
await gesture.down(endOfTextPos);
await tester.pump();
await gesture.up();
await tester.pump();
expect(controller.selection.baseOffset, 74);
expect(controller.selection.extentOffset, 74);
// Here we click on same position again, to register a triple click. This will select
// the paragraph at the clicked position.
await gesture.down(endOfTextPos);
await tester.pump();
await gesture.up();
await tester.pump();
await tester.pumpAndSettle();
expect(controller.selection.baseOffset, 57);
expect(controller.selection.extentOffset, 74);
},
variant: TargetPlatformVariant.all(excluding: <TargetPlatform>{ TargetPlatform.linux }),
);
testWidgets( testWidgets(
'triple tap chains work on Non-Apple mobile platforms', 'triple tap chains work on Non-Apple mobile platforms',
(WidgetTester tester) async { (WidgetTester tester) async {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment