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

handleSelectWord in MultiSelectableSelectionContainerDelegate should handle...

handleSelectWord in MultiSelectableSelectionContainerDelegate should handle rects inside of rects (#127478)

Fixes #127076

Sometimes a `Selectable`s rect may contain another `Selectable`s rect within it. In the case of `handleSelectWord` when choosing which `Selectable` to dispatch the `SelectionEvent` to, the event would be dispatched to the wrong `Selectable` causing an assertion error to be thrown. 

<img width="577" alt="Screenshot 2023-05-24 at 2 46 13 AM" src="https://github.com/flutter/flutter/assets/948037/bb246966-acad-4d81-bd87-758c3ea6ea39">

In the picture above the red outline shows the rect of a two-line piece of text. And the blue rect shows the rect of a piece of text that is on the second line of the two-line piece of text, but has been separated into its own rect for some case, for example when `TextSpan`s are separated by a `WidgetSpan`.

We should check if the text layout of the selectable that has been dispatched the SelectionEvent contains the word, if not then return `SelectionResult.next`, and continue to look through the list of selectables.
parent 44e7206a
...@@ -1476,6 +1476,11 @@ class _SelectableFragment with Selectable, ChangeNotifier implements TextLayoutM ...@@ -1476,6 +1476,11 @@ class _SelectableFragment with Selectable, ChangeNotifier implements TextLayoutM
} }
final TextRange word = paragraph.getWordBoundary(position); final TextRange word = paragraph.getWordBoundary(position);
assert(word.isNormalized); assert(word.isNormalized);
if (word.start < range.start && word.end < range.start) {
return SelectionResult.previous;
} else if (word.start > range.end && word.end > range.end) {
return SelectionResult.next;
}
// Fragments are separated by placeholder span, the word boundary shouldn't // Fragments are separated by placeholder span, the word boundary shouldn't
// expand across fragments. // expand across fragments.
assert(word.start >= range.start && word.end <= range.end); assert(word.start >= range.start && word.end <= range.end);
......
...@@ -1669,12 +1669,21 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai ...@@ -1669,12 +1669,21 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
/// ///
/// Returns positive if a is lower, negative if a is higher. /// Returns positive if a is lower, negative if a is higher.
static int _compareHorizontally(Rect a, Rect b) { static int _compareHorizontally(Rect a, Rect b) {
if (a.left - b.left < precisionErrorTolerance && a.right - b.right > - precisionErrorTolerance) {
// a encloses b. // a encloses b.
if (a.left - b.left < precisionErrorTolerance && a.right - b.right > - precisionErrorTolerance) {
// b ends before a.
if (a.right - b.right > precisionErrorTolerance) {
return 1;
}
return -1; return -1;
} }
if (b.left - a.left < precisionErrorTolerance && b.right - a.right > - precisionErrorTolerance) {
// b encloses a. // b encloses a.
if (b.left - a.left < precisionErrorTolerance && b.right - a.right > - precisionErrorTolerance) {
// a ends before b.
if (b.right - a.right > precisionErrorTolerance) {
return -1;
}
return 1; return 1;
} }
if ((a.left - b.left).abs() > precisionErrorTolerance) { if ((a.left - b.left).abs() > precisionErrorTolerance) {
...@@ -1888,13 +1897,23 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai ...@@ -1888,13 +1897,23 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
/// [SelectWordSelectionEvent.globalPosition]. /// [SelectWordSelectionEvent.globalPosition].
@protected @protected
SelectionResult handleSelectWord(SelectWordSelectionEvent event) { SelectionResult handleSelectWord(SelectWordSelectionEvent event) {
SelectionResult? lastSelectionResult;
for (int index = 0; index < selectables.length; index += 1) { for (int index = 0; index < selectables.length; index += 1) {
final Rect localRect = Rect.fromLTWH(0, 0, selectables[index].size.width, selectables[index].size.height); final Rect localRect = Rect.fromLTWH(0, 0, selectables[index].size.width, selectables[index].size.height);
final Matrix4 transform = selectables[index].getTransformTo(null); final Matrix4 transform = selectables[index].getTransformTo(null);
final Rect globalRect = MatrixUtils.transformRect(transform, localRect); final Rect globalRect = MatrixUtils.transformRect(transform, localRect);
if (globalRect.contains(event.globalPosition)) { if (globalRect.contains(event.globalPosition)) {
final SelectionGeometry existingGeometry = selectables[index].value; final SelectionGeometry existingGeometry = selectables[index].value;
dispatchSelectionEventToChild(selectables[index], event); lastSelectionResult = dispatchSelectionEventToChild(selectables[index], event);
if (lastSelectionResult == SelectionResult.next) {
continue;
}
if (index == 0 && lastSelectionResult == SelectionResult.previous) {
return SelectionResult.previous;
}
if (index == selectables.length - 1 && lastSelectionResult == SelectionResult.next) {
return SelectionResult.next;
}
if (selectables[index].value != existingGeometry) { if (selectables[index].value != existingGeometry) {
// Geometry has changed as a result of select word, need to clear the // Geometry has changed as a result of select word, need to clear the
// selection of other selectables to keep selection in sync. // selection of other selectables to keep selection in sync.
...@@ -1904,9 +1923,15 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai ...@@ -1904,9 +1923,15 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
currentSelectionStartIndex = currentSelectionEndIndex = index; currentSelectionStartIndex = currentSelectionEndIndex = index;
} }
return SelectionResult.end; return SelectionResult.end;
} else {
if (lastSelectionResult == SelectionResult.next) {
currentSelectionStartIndex = currentSelectionEndIndex = index - 1;
return SelectionResult.end;
} }
} }
return SelectionResult.none; }
assert(lastSelectionResult == null);
return SelectionResult.end;
} }
/// Removes the selection of all selectables this delegate manages. /// Removes the selection of all selectables this delegate manages.
......
...@@ -769,6 +769,48 @@ void main() { ...@@ -769,6 +769,48 @@ void main() {
skip: isBrowser, // https://github.com/flutter/flutter/issues/61020 skip: isBrowser, // https://github.com/flutter/flutter/issues/61020
); );
testWidgets(
'can select word when a selectables rect is completely inside of another selectables rect', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/127076.
final UniqueKey outerText = UniqueKey();
await tester.pumpWidget(
MaterialApp(
home: SelectableRegion(
focusNode: FocusNode(),
selectionControls: materialTextSelectionControls,
child: Scaffold(
body: Center(
child: Text.rich(
const TextSpan(
children: <InlineSpan>[
TextSpan(
text:
'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.',
),
WidgetSpan(child: Text('Some text in a WidgetSpan. ')),
TextSpan(text: 'Hello, world.'),
],
),
key: outerText,
),
),
),
),
),
);
final RenderParagraph paragraph = tester.renderObject<RenderParagraph>(find.descendant(of: find.byKey(outerText), matching: find.byType(RichText)).first);
// Right click to select word at position.
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph, 125), kind: PointerDeviceKind.mouse, buttons: kSecondaryMouseButton);
addTearDown(gesture.removePointer);
await tester.pump();
await gesture.up();
await tester.pump();
// Should select "Hello".
expect(paragraph.selections[0], const TextSelection(baseOffset: 124, extentOffset: 129));
},
skip: isBrowser, // https://github.com/flutter/flutter/issues/61020
);
testWidgets( testWidgets(
'widget span is ignored if it does not contain text - non Apple', 'widget span is ignored if it does not contain text - non Apple',
(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