Fix: Deleting text in EditableText with CJK keyboard while in CupertinoPageRoute throws exception (#153822)

Fixes #153003

Pressing backspace on CJK keyboards to delete text may trigger a key event which in turns triggers the `DeleteTextIntent`. This is different than English keyboards where the updated `TextEditingValue` from the `TextInputPlugin` will come through `updateEditingValue` which allows us to hit the codepath that hides the text selection toolbar. 23883b13d4/packages/flutter/lib/src/widgets/editable_text.dart (L3245-L3250) Because CJK keyboards may not hit this codepath, when the text is long and editable text tries to bring the new selection into view by scrolling, this triggers the hide context menu scroll listener in a weird state 23883b13d4/packages/flutter/lib/src/widgets/editable_text.dart (L3865-L3869) causing an exception to be thrown.

This PR tries to work around the issue above by hiding the toolbar when a `DeleteTextIntent` is received.
This commit is contained in:
Renzo Olivares 2024-08-21 20:19:24 -07:00 committed by GitHub
parent cd988305c4
commit f6054ae521
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 104 additions and 2 deletions

View File

@ -5814,6 +5814,23 @@ class _DeleteTextAction<T extends DirectionalTextEditingIntent> extends ContextA
final TextBoundary Function() getTextBoundary;
final _ApplyTextBoundary _applyTextBoundary;
void _hideToolbarIfTextChanged(ReplaceTextIntent intent) {
if (state._selectionOverlay == null || !state.selectionOverlay!.toolbarIsVisible) {
return;
}
final TextEditingValue oldValue = intent.currentTextEditingValue;
final TextEditingValue newValue = intent.currentTextEditingValue.replaced(
intent.replacementRange,
intent.replacementText,
);
if (oldValue.text != newValue.text) {
// Hide the toolbar if the text was changed, but only hide the toolbar
// overlay; the selection handle's visibility will be handled
// by `_handleSelectionChanged`.
state.hideToolbar(false);
}
}
@override
Object? invoke(T intent, [BuildContext? context]) {
final TextSelection selection = state._value.selection;
@ -5829,9 +5846,11 @@ class _DeleteTextAction<T extends DirectionalTextEditingIntent> extends ContextA
start: atomicBoundary.getLeadingTextBoundaryAt(selection.start) ?? state._value.text.length,
end: atomicBoundary.getTrailingTextBoundaryAt(selection.end - 1) ?? 0,
);
final ReplaceTextIntent replaceTextIntent = ReplaceTextIntent(state._value, '', range, SelectionChangedCause.keyboard);
_hideToolbarIfTextChanged(replaceTextIntent);
return Actions.invoke(
context!,
ReplaceTextIntent(state._value, '', range, SelectionChangedCause.keyboard),
replaceTextIntent,
);
}
@ -5843,9 +5862,11 @@ class _DeleteTextAction<T extends DirectionalTextEditingIntent> extends ContextA
: atomicBoundary.getTrailingTextBoundaryAt(selection.baseOffset - 1) ?? 0,
extentOffset: target,
);
final ReplaceTextIntent replaceTextIntent = ReplaceTextIntent(state._value, '', rangeToDelete, SelectionChangedCause.keyboard);
_hideToolbarIfTextChanged(replaceTextIntent);
return Actions.invoke(
context!,
ReplaceTextIntent(state._value, '', rangeToDelete, SelectionChangedCause.keyboard),
replaceTextIntent,
);
}

View File

@ -9632,6 +9632,87 @@ void main() {
expect(scrollable.controller!.position.pixels, equals(renderEditable.maxScrollExtent));
});
testWidgets('Deleting text with keyboard backspace does not trigger assertion on CupertinoPageRoute', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/153003.
controller.text = testText * 20;
final ScrollController editableScrollController = ScrollController();
addTearDown(editableScrollController.dispose);
final GlobalKey<NavigatorState> navigatorKey = GlobalKey<NavigatorState>();
await tester.pumpWidget(MaterialApp(
navigatorKey: navigatorKey,
home: Center(
child: TextButton(
onPressed: () async {
if (navigatorKey.currentState == null) {
return;
}
await navigatorKey.currentState!.push(
CupertinoPageRoute<void>(
settings: const RouteSettings(name: '/TestCupertinoRoute'),
builder: (BuildContext innerContext) {
return Align(
alignment: Alignment.topLeft,
child: SizedBox(
width: 200,
height: 200,
child: EditableText(
maxLines: null,
controller: controller,
scrollController: editableScrollController,
focusNode: focusNode,
style: textStyle,
cursorColor: Colors.blue,
backgroundCursorColor: Colors.grey,
showSelectionHandles: true,
selectionControls: materialTextSelectionControls,
selectionColor: Colors.lightBlueAccent,
),
),
);
}
),
);
},
child: const Text('Push Route'),
),
),
));
// Push cupertino route.
await tester.tap(find.text('Push Route'));
await tester.pumpAndSettle();
expect(editableScrollController.offset, 0);
final EditableTextState state = tester.state<EditableTextState>(find.byType(EditableText));
state.bringIntoView(TextPosition(offset: controller.text.length));
await tester.pumpAndSettle();
expect(editableScrollController.offset, editableScrollController.position.maxScrollExtent);
// Select a word near the end of the text. And show the toolbar.
await tester.tapAt(textOffsetToPosition(tester, controller.text.length - 10));
state.renderEditable.selectWord(cause: SelectionChangedCause.longPress);
expect(state.showToolbar(), true);
await tester.pumpAndSettle();
expect(controller.selection, const TextSelection(baseOffset: 1426, extentOffset: 1431));
expect(state.selectionOverlay, isNotNull);
expect(state.selectionOverlay!.toolbarIsVisible, true);
// Send backspace key event to delete the selected word. This will cause
// the EditableText to scroll the new position into view, but this
// should not cause an exception, and the toolbar should no longer be visible.
await tester.sendKeyEvent(LogicalKeyboardKey.backspace);
await tester.pumpAndSettle();
expect(controller.selection, const TextSelection.collapsed(offset: 1426));
expect(tester.takeException(), isNull);
expect(state.selectionOverlay, isNotNull);
expect(state.selectionOverlay!.toolbarIsVisible, false);
// On web, we don't show the Flutter toolbar and instead rely on the browser
// toolbar. Until we change that, this test should remain skipped.
}, skip: kIsWeb); // [intended]
testWidgets('bringIntoView brings the caret into view when in a viewport', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/55547.
controller.text = testText * 20;