From 24db45e792fc96402c61451aa4869be117588791 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Tue, 29 Nov 2022 02:03:15 -0800 Subject: [PATCH] Disable backspace/delete handling on iOS & macOS (#115900) * Disable backspace/delete handling on iOS * fix tests * review * macOS too * review --- .../default_text_editing_shortcuts.dart | 25 +++- .../default_text_editing_shortcuts_test.dart | 85 +++++++++++++ .../widgets/editable_text_shortcuts_test.dart | 42 +++---- .../test/widgets/editable_text_test.dart | 116 +++++++++--------- 4 files changed, 188 insertions(+), 80 deletions(-) diff --git a/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart b/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart index 01f389e9f771..e57f04418d1f 100644 --- a/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart +++ b/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart @@ -19,6 +19,12 @@ import 'text_editing_intents.dart'; /// lower in the widget tree than this. See the [Action] class for an example /// of remapping an [Intent] to a custom [Action]. /// +/// The [Shortcuts] widget usually takes precedence over system keybindings. +/// Proceed with caution if the shortcut you wish to override is also used by +/// the system. For example, overriding [LogicalKeyboardKey.backspace] could +/// cause CJK input methods to discard more text than they should when the +/// backspace key is pressed during text composition on iOS. +/// /// {@tool snippet} /// /// This example shows how to use an additional [Shortcuts] widget to override @@ -440,6 +446,7 @@ class DefaultTextEditingShortcuts extends StatelessWidget { static final Map _macDisablingTextShortcuts = { ..._commonDisablingTextShortcuts, + ..._iOSDisablingTextShortcuts, const SingleActivator(LogicalKeyboardKey.escape): const DoNothingAndStopPropagationTextIntent(), const SingleActivator(LogicalKeyboardKey.tab): const DoNothingAndStopPropagationTextIntent(), const SingleActivator(LogicalKeyboardKey.tab, shift: true): const DoNothingAndStopPropagationTextIntent(), @@ -447,6 +454,20 @@ class DefaultTextEditingShortcuts extends StatelessWidget { const SingleActivator(LogicalKeyboardKey.arrowUp, shift: true, alt: true): const DoNothingAndStopPropagationTextIntent(), }; + // Hand backspace/delete events that do not depend on text layout (delete + // character and delete to the next word) back to the IME to allow it to + // update composing text properly. + static const Map _iOSDisablingTextShortcuts = { + SingleActivator(LogicalKeyboardKey.backspace): DoNothingAndStopPropagationTextIntent(), + SingleActivator(LogicalKeyboardKey.backspace, shift: true): DoNothingAndStopPropagationTextIntent(), + SingleActivator(LogicalKeyboardKey.delete): DoNothingAndStopPropagationTextIntent(), + SingleActivator(LogicalKeyboardKey.delete, shift: true): DoNothingAndStopPropagationTextIntent(), + SingleActivator(LogicalKeyboardKey.backspace, alt: true, shift: true): DoNothingAndStopPropagationTextIntent(), + SingleActivator(LogicalKeyboardKey.backspace, alt: true): DoNothingAndStopPropagationTextIntent(), + SingleActivator(LogicalKeyboardKey.delete, alt: true, shift: true): DoNothingAndStopPropagationTextIntent(), + SingleActivator(LogicalKeyboardKey.delete, alt: true): DoNothingAndStopPropagationTextIntent(), + }; + static Map get _shortcuts { switch (defaultTargetPlatform) { case TargetPlatform.android: @@ -469,13 +490,13 @@ class DefaultTextEditingShortcuts extends StatelessWidget { return _webDisablingTextShortcuts; } switch (defaultTargetPlatform) { - case TargetPlatform.android: case TargetPlatform.fuchsia: - case TargetPlatform.iOS: case TargetPlatform.linux: case TargetPlatform.windows: return null; + case TargetPlatform.iOS: + return _iOSDisablingTextShortcuts; case TargetPlatform.macOS: return _macDisablingTextShortcuts; } diff --git a/packages/flutter/test/widgets/default_text_editing_shortcuts_test.dart b/packages/flutter/test/widgets/default_text_editing_shortcuts_test.dart index 3b7f4677e0b3..82eaefb4e243 100644 --- a/packages/flutter/test/widgets/default_text_editing_shortcuts_test.dart +++ b/packages/flutter/test/widgets/default_text_editing_shortcuts_test.dart @@ -44,6 +44,87 @@ void main() { ), ); } + + group('iOS: do not delete/backspace events', () { + final TargetPlatformVariant iOS = TargetPlatformVariant.only(TargetPlatform.iOS); + final FocusNode editable = FocusNode(); + final FocusNode spy = FocusNode(); + + testWidgets('backspace with and without word modifier', (WidgetTester tester) async { + tester.binding.testTextInput.unregister(); + addTearDown(tester.binding.testTextInput.register); + + await tester.pumpWidget( + buildSpyAboveEditableText( + editableFocusNode: editable, + spyFocusNode: spy, + ), + ); + editable.requestFocus(); + await tester.pump(); + final ActionSpyState state = tester.state(find.byType(ActionSpy)); + + for (int altShiftState = 0; altShiftState < 1 << 2; altShiftState += 1) { + final bool alt = altShiftState & 0x1 != 0; + final bool shift = altShiftState & 0x2 != 0; + await sendKeyCombination(tester, SingleActivator(LogicalKeyboardKey.backspace, alt: alt, shift: shift)); + } + await tester.pump(); + + expect(state.lastIntent, isNull); + }, variant: iOS); + + testWidgets('delete with and without word modifier', (WidgetTester tester) async { + tester.binding.testTextInput.unregister(); + addTearDown(tester.binding.testTextInput.register); + + await tester.pumpWidget( + buildSpyAboveEditableText( + editableFocusNode: editable, + spyFocusNode: spy, + ), + ); + editable.requestFocus(); + await tester.pump(); + final ActionSpyState state = tester.state(find.byType(ActionSpy)); + + for (int altShiftState = 0; altShiftState < 1 << 2; altShiftState += 1) { + final bool alt = altShiftState & 0x1 != 0; + final bool shift = altShiftState & 0x2 != 0; + await sendKeyCombination(tester, SingleActivator(LogicalKeyboardKey.delete, alt: alt, shift: shift)); + } + await tester.pump(); + + expect(state.lastIntent, isNull); + }, variant: iOS); + + testWidgets('Exception: deleting to line boundary is handled by the framework', (WidgetTester tester) async { + tester.binding.testTextInput.unregister(); + addTearDown(tester.binding.testTextInput.register); + + await tester.pumpWidget( + buildSpyAboveEditableText( + editableFocusNode: editable, + spyFocusNode: spy, + ), + ); + editable.requestFocus(); + await tester.pump(); + final ActionSpyState state = tester.state(find.byType(ActionSpy)); + + for (int keyState = 0; keyState < 1 << 2; keyState += 1) { + final bool shift = keyState & 0x1 != 0; + final LogicalKeyboardKey key = keyState & 0x2 != 0 ? LogicalKeyboardKey.delete : LogicalKeyboardKey.backspace; + + state.lastIntent = null; + final SingleActivator activator = SingleActivator(key, meta: true, shift: shift); + await sendKeyCombination(tester, activator); + await tester.pump(); + expect(state.lastIntent, isA(), reason: '$activator'); + } + }, variant: iOS); + }, skip: kIsWeb); // [intended] specific tests target non-web. + group('macOS does not accept shortcuts if focus under EditableText', () { final TargetPlatformVariant macOSOnly = TargetPlatformVariant.only(TargetPlatform.macOS); @@ -400,6 +481,10 @@ class ActionSpyState extends State { ExtendSelectionVerticallyToAdjacentLineIntent: CallbackAction(onInvoke: _captureIntent), ExtendSelectionToDocumentBoundaryIntent: CallbackAction(onInvoke: _captureIntent), ExtendSelectionToNextWordBoundaryOrCaretLocationIntent: CallbackAction(onInvoke: _captureIntent), + + DeleteToLineBreakIntent: CallbackAction(onInvoke: _captureIntent), + DeleteToNextWordBoundaryIntent: CallbackAction(onInvoke: _captureIntent), + DeleteCharacterIntent: CallbackAction(onInvoke: _captureIntent), }; // ignore: use_setters_to_change_properties diff --git a/packages/flutter/test/widgets/editable_text_shortcuts_test.dart b/packages/flutter/test/widgets/editable_text_shortcuts_test.dart index c531b717a8ce..0e76e8d9b30a 100644 --- a/packages/flutter/test/widgets/editable_text_shortcuts_test.dart +++ b/packages/flutter/test/widgets/editable_text_shortcuts_test.dart @@ -150,7 +150,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 19), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('backspace readonly', (WidgetTester tester) async { controller.text = testText; @@ -215,7 +215,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 71), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('backspace inside of a cluster', (WidgetTester tester) async { controller.text = testCluster; @@ -236,7 +236,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 0), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('backspace at cluster boundary', (WidgetTester tester) async { controller.text = testCluster; @@ -257,7 +257,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 0), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); }); group('delete: ', () { @@ -287,7 +287,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 20), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('delete readonly', (WidgetTester tester) async { controller.text = testText; @@ -305,7 +305,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 20, affinity: TextAffinity.upstream), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('delete at start', (WidgetTester tester) async { controller.text = testText; @@ -328,7 +328,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 0), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('delete at end', (WidgetTester tester) async { controller.text = testText; @@ -373,7 +373,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 0), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('delete at cluster boundary', (WidgetTester tester) async { controller.text = testCluster; @@ -394,7 +394,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 8), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); }); group('Non-collapsed delete', () { @@ -420,7 +420,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 8), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('at the boundaries of a cluster', (WidgetTester tester) async { controller.text = testCluster; @@ -441,7 +441,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 8), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('cross-cluster', (WidgetTester tester) async { controller.text = testCluster; @@ -462,7 +462,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 0), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('cross-cluster obscured text', (WidgetTester tester) async { controller.text = testCluster; @@ -483,7 +483,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 1), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); }); group('word modifier + backspace', () { @@ -516,7 +516,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 24), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('readonly', (WidgetTester tester) async { controller.text = testText; @@ -581,7 +581,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 71), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('inside of a cluster', (WidgetTester tester) async { controller.text = testCluster; @@ -602,7 +602,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 0), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('at cluster boundary', (WidgetTester tester) async { controller.text = testCluster; @@ -623,7 +623,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 0), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); }); group('word modifier + delete', () { @@ -656,7 +656,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 23), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('readonly', (WidgetTester tester) async { controller.text = testText; @@ -697,7 +697,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 0), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('at end', (WidgetTester tester) async { controller.text = testText; @@ -735,7 +735,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 0), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); testWidgets('at cluster boundary', (WidgetTester tester) async { controller.text = testCluster; @@ -756,7 +756,7 @@ void main() { controller.selection, const TextSelection.collapsed(offset: 8), ); - }, variant: TargetPlatformVariant.all()); + }, variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS })); }); group('line modifier + backspace', () { diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index 79591e35b98e..5d57ab462791 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -6330,69 +6330,71 @@ void main() { expect(controller.text, equals(testText), reason: 'on $platform'); expect((await Clipboard.getData(Clipboard.kTextPlain))!.text, equals(testText)); - // Delete - await sendKeys( - tester, - [ - LogicalKeyboardKey.delete, - ], - targetPlatform: defaultTargetPlatform, - ); - expect( - selection, - equals( - const TextSelection( - baseOffset: 0, - extentOffset: 0, + if (defaultTargetPlatform != TargetPlatform.iOS) { + // Delete + await sendKeys( + tester, + [ + LogicalKeyboardKey.delete, + ], + targetPlatform: defaultTargetPlatform, + ); + expect( + selection, + equals( + const TextSelection( + baseOffset: 0, + extentOffset: 0, + ), ), - ), - reason: 'on $platform', - ); - expect(controller.text, isEmpty, reason: 'on $platform'); + reason: 'on $platform', + ); + expect(controller.text, isEmpty, reason: 'on $platform'); - controller.text = 'abc'; - controller.selection = const TextSelection(baseOffset: 2, extentOffset: 2); + controller.text = 'abc'; + controller.selection = const TextSelection(baseOffset: 2, extentOffset: 2); - // Backspace - await sendKeys( - tester, - [ - LogicalKeyboardKey.backspace, - ], - targetPlatform: defaultTargetPlatform, - ); - expect( - selection, - equals( - const TextSelection( - baseOffset: 1, - extentOffset: 1, + // Backspace + await sendKeys( + tester, + [ + LogicalKeyboardKey.backspace, + ], + targetPlatform: defaultTargetPlatform, + ); + expect( + selection, + equals( + const TextSelection( + baseOffset: 1, + extentOffset: 1, + ), ), - ), - reason: 'on $platform', - ); - expect(controller.text, 'ac', reason: 'on $platform'); + reason: 'on $platform', + ); + expect(controller.text, 'ac', reason: 'on $platform'); - // Shift-backspace (same as backspace) - await sendKeys( - tester, - [ - LogicalKeyboardKey.backspace, - ], - shift: true, - targetPlatform: defaultTargetPlatform, - ); - expect( - selection, - equals( - const TextSelection( - baseOffset: 0, - extentOffset: 0, + // Shift-backspace (same as backspace) + await sendKeys( + tester, + [ + LogicalKeyboardKey.backspace, + ], + shift: true, + targetPlatform: defaultTargetPlatform, + ); + expect( + selection, + equals( + const TextSelection( + baseOffset: 0, + extentOffset: 0, + ), ), - ), - reason: 'on $platform', - ); - expect(controller.text, 'c', reason: 'on $platform'); + reason: 'on $platform', + ); + expect(controller.text, 'c', reason: 'on $platform'); + } } testWidgets('keyboard text selection works (RawKeyEvent)', (WidgetTester tester) async {