From f6054ae521bdf4c38aca0fa5bda9887596ff60a4 Mon Sep 17 00:00:00 2001 From: Renzo Olivares Date: Wed, 21 Aug 2024 20:19:24 -0700 Subject: [PATCH] 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. https://github.com/flutter/flutter/blob/23883b13d4919bed4f572e78cb60c016bb3b872f/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 https://github.com/flutter/flutter/blob/23883b13d4919bed4f572e78cb60c016bb3b872f/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. --- .../lib/src/widgets/editable_text.dart | 25 +++++- .../test/widgets/editable_text_test.dart | 81 +++++++++++++++++++ 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 1fc202c9c06..c3ca3a49de4 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -5814,6 +5814,23 @@ class _DeleteTextAction 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 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 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, ); } diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index 83616e89d86..8d561706647 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -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 navigatorKey = GlobalKey(); + + await tester.pumpWidget(MaterialApp( + navigatorKey: navigatorKey, + home: Center( + child: TextButton( + onPressed: () async { + if (navigatorKey.currentState == null) { + return; + } + await navigatorKey.currentState!.push( + CupertinoPageRoute( + 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(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;