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

Renzo-Olivares · 2024-08-21T08:01:33Z

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.

flutter/packages/flutter/lib/src/widgets/editable_text.dart

Lines 3245 to 3250 in 23883b1

    
           if (value.text != _value.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`. https://github.com/flutter/flutter/issues/108673 
        
             hideToolbar(false); 
        
           }

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

flutter/packages/flutter/lib/src/widgets/editable_text.dart

Lines 3865 to 3869 in 23883b1

    
           final Rect selectionBounds = _value.selection.isCollapsed || selectionBoxes.isEmpty 
        
                                           ? renderEditable.getLocalRectForCaret(_value.selection.extent) 
        
                                           : selectionBoxes 
        
                                               .map((TextBox box) => box.toRect()) 
        
                                               .reduce((Rect result, Rect rect) => result.expandToInclude(rect));

causing an exception to be thrown.

This PR tries to work around the issue above by hiding the toolbar when a DeleteTextIntent is received.

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is [test-exempt].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

justinmc

LGTM 👍

Is this for the software keyboard? What a roundabout bug. Thanks for fixing.

justinmc · 2024-08-21T17:38:33Z

packages/flutter/test/widgets/editable_text_test.dart

+    await tester.pumpAndSettle();
+    expect(tester.takeException(), isNull);
+    expect(state.selectionOverlay, isNotNull);
+    expect(state.selectionOverlay!.toolbarIsVisible, false);


Nit: Maybe also expect that the text has changed so that the selected word is gone? If you think it's valuable.

Renzo-Olivares · 2024-08-21T19:10:03Z

Yeah this was for the software keyboard. For some reason this bug does not affect MaterialPageRoute, but this patch achieves the correct behavior I see on native Android and iOS (the toolbar should be hidden when text is deleted), and fixes the crash.

auto-submit · 2024-08-21T20:02:22Z

auto label is removed for flutter/flutter/153822, due to - The status or check suite Linux web_long_running_tests_5_5 has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2024-08-21T21:16:57Z

auto label is removed for flutter/flutter/153822, due to - The status or check suite Linux web_long_running_tests_5_5 has failed. Please fix the issues identified (or deflake) before re-applying this label.

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

…rtinoPageRoute` throws exception (flutter#153822) Fixes flutter#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.

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

Fix bug and add test

4c24aa1

github-actions bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. labels Aug 21, 2024

Renzo-Olivares changed the title ~~Fix: Deleting text in EditableText while in CupertinoPageRoute throws exception~~ Fix: Deleting text in EditableText while in CupertinoPageRoute throws exception Aug 21, 2024

Renzo-Olivares changed the title ~~Fix: Deleting text in EditableText while in CupertinoPageRoute throws exception~~ Fix: Deleting text in EditableText while in CupertinoPageRoute throws exception Aug 21, 2024

Renzo-Olivares changed the title ~~Fix: Deleting text in EditableText while in CupertinoPageRoute throws exception~~ Fix: Deleting text in EditableText with CJK keyboard while in CupertinoPageRoute throws exception Aug 21, 2024

Renzo Olivares added 3 commits August 21, 2024 01:17

Add justification for skipping test on web

1728013

fix analyzer

1b2916d

should hide toolbar when deleting collapsed range

8ad9b37

Renzo-Olivares requested a review from justinmc August 21, 2024 16:17

fix analyzer

6214f7d

justinmc approved these changes Aug 21, 2024

View reviewed changes

address reviewer comments

46fc29c

Renzo-Olivares added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 21, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 21, 2024

Renzo-Olivares added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 21, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 21, 2024

Renzo-Olivares added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 21, 2024

auto-submit bot merged commit f6054ae into flutter:master Aug 22, 2024

engine-flutter-autoroll mentioned this pull request Aug 22, 2024

Roll Flutter from b8f89f70bf4e to 59e6c8d52f11 (20 revisions) flutter/packages#7477

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 22, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

4e5e1f9

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll mentioned this pull request Aug 22, 2024

Roll Flutter from b8f89f70bf4e to 544ce7cd34b7 (21 revisions) flutter/packages#7478

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 22, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

4d2e0f1

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll mentioned this pull request Aug 22, 2024

Roll Flutter from b8f89f70bf4e to a9e94d9045cd (22 revisions) flutter/packages#7479

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 22, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

6d25d9e

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll mentioned this pull request Aug 22, 2024

Roll Flutter from b8f89f70bf4e to 4a6fbef83f9b (23 revisions) flutter/packages#7481

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 22, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

0be7665

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

b2542c3

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

3938e98

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

ec4819b

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

deab513

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

7ccc9b7

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

9e4602d

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

366754d

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 31, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

2c01f82

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 31, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

d8b76b6

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 1, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

a61632a

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

a545439

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

0ca24f9

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

6b90bbf

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

f96cb02

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

e78aaf7

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

b5f4383

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

e042ff2

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

370d9ad

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

e9ba4bb

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

70d2312

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

fa24255

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

4677650

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

78aaec4

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

c363b1a

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

f8fc2fe

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

d2d3b9b

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

1ab35e2

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

318ab24

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024

f6054ae Fix: Deleting text in EditableText with CJK keyboard while …

075bcc4

…in `CupertinoPageRoute` throws exception (flutter/flutter#153822)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

Renzo-Olivares commented Aug 21, 2024 •

edited

Loading

Uh oh!

justinmc left a comment

Uh oh!

justinmc Aug 21, 2024

Uh oh!

Renzo-Olivares commented Aug 21, 2024

Uh oh!

auto-submit bot commented Aug 21, 2024

Uh oh!

auto-submit bot commented Aug 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if (value.text != _value.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`. https://github.com/flutter/flutter/issues/108673
	hideToolbar(false);
	}

	final Rect selectionBounds = _value.selection.isCollapsed \|\| selectionBoxes.isEmpty
	? renderEditable.getLocalRectForCaret(_value.selection.extent)
	: selectionBoxes
	.map((TextBox box) => box.toRect())
	.reduce((Rect result, Rect rect) => result.expandToInclude(rect));

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

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

Uh oh!

Conversation

Renzo-Olivares commented Aug 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Aug 21, 2024

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares commented Aug 21, 2024

Uh oh!

auto-submit bot commented Aug 21, 2024

Uh oh!

auto-submit bot commented Aug 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

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

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

Renzo-Olivares commented Aug 21, 2024 •

edited

Loading