[ios17][text_input]fix ios 17.0 keyboard freeze when switching languages (without relying on text affinity) #47566

hellohuanlin · 2023-11-01T19:22:31Z

After close examination of the UIKit's default string tokenizer, when querying the line enclosing the end of doc position in forward direction, we should return nil (regardless whether the position is forward or backward affinity).

This aligns with the API doc:

If the text position is at a text-unit boundary, it is considered enclosed only if the next position in the given direction is entirely enclosed.

Will cherry pick this soon. Otherwise it will be less and less important as users upgrade to iOS 17.1.

Why my previous workaround also works?

It turns out my previous workaround PR #46591 works only because our misuse of text affinity in our text input. Specifically, when adding text affinity support, we only added it to FlutterTextPosition, but not FlutterTextRange. So when getting the beginning/end position from the range, we assign arbitrary affinities.

List which issues are fixed by this PR. You must list at least one issue.

flutter/flutter#134716

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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 and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

hellohuanlin · 2023-11-09T19:02:01Z

I need to do more testing on iOS 17.0 and 17.1 (and the newly released 17.1.1), around voice control, scribble and floating cursor selection. But so far this PR is looking good and should be ready for review

stuartmorgan · 2023-11-10T13:25:13Z

when querying the line enclosing the end of doc position in forward direction, we should return nil (regardless whether the position is forward or backward affinity). This is undocumented behavior of UIKit

Is it undocumented? The docs for the method we are implementing say:

If the text position is at a text-unit boundary, it is considered enclosed only if the next position in the given direction is entirely enclosed.

Isn't the case here that we are at a boundary, and the next position in the given direction doesn't exist (in which case the next position can't be entirely enclosed)?

hellohuanlin · 2023-11-10T19:00:28Z

Isn't the case here that we are at a boundary, and the next position in the given direction doesn't exist (in which case the next position can't be entirely enclosed)?

True. It sounds indeed like what we are doing here.

stuartmorgan

LGTM with comment nit.

stuartmorgan · 2023-11-13T16:03:03Z

shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm

+                          inDirection:(UITextDirection)direction {
+  // TODO(hellohuanlin): remove iOS 17 check. The same logic should apply to older iOS version.
+  if (@available(iOS 17.0, *)) {
+    // If querying the line from end of document in forward direction, report not found.


This should link to and quote the docs I mentioned in my comment so that it's clear why.

LongCatIsLooong

LGTM. Thanks for looking into this!

…ing languages (without relying on text affinity) (flutter/engine#47566)

…ges (without relying on text affinity) (flutter#47566) After close examination of the UIKit's default string tokenizer, when querying the line enclosing the end of doc position **in forward direction**, we should return nil (regardless whether the position is forward or backward affinity). This aligns with the [API doc](https://developer.apple.com/documentation/uikit/uitextinputtokenizer/1614464-rangeenclosingposition?language=objc): > If the text position is at a text-unit boundary, it is considered enclosed only if the next position in the given direction is entirely enclosed. Will cherry pick this soon. Otherwise it will be less and less important as users upgrade to iOS 17.1. ### Why my previous workaround also works? It turns out my previous workaround PR flutter#46591 works only because our misuse of text affinity in our text input. Specifically, when adding text affinity support, we only added it to `FlutterTextPosition`, but not `FlutterTextRange`. So when getting the beginning/end position from the range, we assign arbitrary affinities. *List which issues are fixed by this PR. You must list at least one issue.* flutter#46591 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

…ing languages (without relying on text affinity) (flutter/engine#47566)

…138441) flutter/engine@f15b259...c5a067b 2023-11-14 [email protected] Roll Skia from 650913eacd01 to 15a2577b12c1 (1 revision) (flutter/engine#48036) 2023-11-14 [email protected] Roll Skia from 6feae2c274ce to 650913eacd01 (1 revision) (flutter/engine#48034) 2023-11-14 [email protected] [macOS] Eliminate unused OCMock includes (flutter/engine#48031) 2023-11-14 [email protected] [ios17][text_input]fix ios 17.0 keyboard freeze when switching languages (without relying on text affinity) (flutter/engine#47566) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…ching languages (#48041) CP for #47566 *List which issues are fixed by this PR. You must list at least one issue.* flutter/flutter#134716 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

hellohuanlin added 2 commits October 31, 2023 11:13

[ios]fix switch language freeze on ios 17 keyboard

8fc611c

only for ios 17

77f8734

github-actions bot added the platform-ios label Nov 1, 2023

hellohuanlin mentioned this pull request Nov 1, 2023

[ios17][text_input]fix chinese language switch freeze, and missing auto correction menu in iOS 17 #46591

Closed

8 tasks

out of range case

2bd9d79

hellohuanlin marked this pull request as ready for review November 9, 2023 19:01

hellohuanlin requested review from stuartmorgan and LongCatIsLooong November 9, 2023 19:01

stuartmorgan approved these changes Nov 13, 2023

View reviewed changes

LongCatIsLooong approved these changes Nov 14, 2023

View reviewed changes

comment

40bf63c

hellohuanlin added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 14, 2023

auto-submit bot merged commit ffcc5bf into flutter:main Nov 14, 2023
22 checks passed

engine-flutter-autoroll mentioned this pull request Nov 14, 2023

Roll Flutter Engine from f15b259fe98c to a79cf49e05d2 (2 revisions) flutter/flutter#138433

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 14, 2023

ffcc5bf29 [ios17][text_input]fix ios 17.0 keyboard freeze when switch…

2c157be

…ing languages (without relying on text affinity) (flutter/engine#47566)

engine-flutter-autoroll mentioned this pull request Nov 14, 2023

Roll Flutter Engine from f15b259fe98c to ddbebb48d275 (3 revisions) flutter/flutter#138435

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 14, 2023

ffcc5bf29 [ios17][text_input]fix ios 17.0 keyboard freeze when switch…

38fcef5

…ing languages (without relying on text affinity) (flutter/engine#47566)

hellohuanlin mentioned this pull request Nov 14, 2023

[CP (stable)] fix iOS 17.0 keyboard freeze when switching languages #48038

Closed

8 tasks

hellohuanlin mentioned this pull request Nov 14, 2023

[CP (beta)] [ios17][text_input]fix ios 17.0 keyboard freeze when switching languages #48041

Merged

8 tasks

engine-flutter-autoroll mentioned this pull request Nov 14, 2023

Roll Flutter Engine from f15b259fe98c to c5a067b637f4 (4 revisions) flutter/flutter#138441

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 14, 2023

ffcc5bf29 [ios17][text_input]fix ios 17.0 keyboard freeze when switch…

5eede1d

…ing languages (without relying on text affinity) (flutter/engine#47566)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ios17][text_input]fix ios 17.0 keyboard freeze when switching languages (without relying on text affinity) #47566

[ios17][text_input]fix ios 17.0 keyboard freeze when switching languages (without relying on text affinity) #47566

hellohuanlin commented Nov 1, 2023 •

edited

Loading

hellohuanlin commented Nov 9, 2023

stuartmorgan commented Nov 10, 2023

hellohuanlin commented Nov 10, 2023

stuartmorgan left a comment

stuartmorgan Nov 13, 2023

LongCatIsLooong left a comment

[ios17][text_input]fix ios 17.0 keyboard freeze when switching languages (without relying on text affinity) #47566

[ios17][text_input]fix ios 17.0 keyboard freeze when switching languages (without relying on text affinity) #47566

Conversation

hellohuanlin commented Nov 1, 2023 • edited Loading

Why my previous workaround also works?

Pre-launch Checklist

hellohuanlin commented Nov 9, 2023

stuartmorgan commented Nov 10, 2023

hellohuanlin commented Nov 10, 2023

stuartmorgan left a comment

Choose a reason for hiding this comment

stuartmorgan Nov 13, 2023

Choose a reason for hiding this comment

LongCatIsLooong left a comment

Choose a reason for hiding this comment

hellohuanlin commented Nov 1, 2023 •

edited

Loading