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

hellohuanlin · 2023-10-05T18:56:51Z

According to Apple's API doc, the line range enclosing position should return nil if no such range. Since endOfDocument is exclusive, we should return nil when UIKit queries the line range enclosing it.

Note that endOfDocument with forward affinity is still queried with "delete line" voice control command, which we should still return the range of the last line.

It is unclear why iOS 17 starts to query this API, but both issues resolved seems to be related to auto-correction.

More discussion can be found in the design doc.

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

Fixes flutter/flutter#134716

Fixes flutter/flutter#132594

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.

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

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

LongCatIsLooong · 2023-10-06T22:00:13Z

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

+    // https://github.com/flutter/flutter/issues/134716
+    // 2. Auto correction candidate menu does not show up in iOS 17:
+    // https://github.com/flutter/flutter/issues/132594
+    // The end of document with forward affinity should still return the last line, which is used


What's endOfDocument supposed to return? If my text is abcd then with the current implementation endOfDocument is (4, backward), so beginningOfDocument and endOfDocument form a closed interval that describes valid cursor positions within the document?

If that's the case shouldn't we return nil when position is greater than (4, backward), and still return a non-nil value when it's (4, backward)?

According to this comment,

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

Lines 1892 to 1894 in b28032c

// This is an upstream position

closestPosition = [FlutterTextPosition positionWithIndex:position

affinity:UITextStorageDirectionBackward];

backward translates to TextAffinity.upstream. So beginningOfDocument and endOfDocument is a close interval in the current implementation. Returning nil for endOfDocument doesn't seem right (unless endOfDocument should be made exclusive).

LongCatIsLooong · 2023-10-06T22:02:09Z

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

+  // TODO(hellohuanlin): Remove iOS 17 check. The same logic should apply to older versions too.
+  if (@available(iOS 17.0, *)) {
+    // The end of document (with backward affinity) should not be part of any line, since end is
+    // exclusive. This is to fix 2 bugs:


@chunhtai is "backward" equivalent to TextAffinity.upstream? If so I guess the "end is exclusive" statement isn't true?

yes in flutter we use upstream and downstream to describe text affinity, and the upstream afinity is considered inclusive. not sure if they have different meaning or treated differently in iOS

chunhtai · 2023-10-06T22:21:36Z

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

+  // TODO(hellohuanlin): Remove iOS 17 check. The same logic should apply to older versions too.
+  if (@available(iOS 17.0, *)) {
+    // The end of document (with backward affinity) should not be part of any line, since end is
+    // exclusive. This is to fix 2 bugs:


yes in flutter we use upstream and downstream to describe text affinity, and the upstream afinity is considered inclusive. not sure if they have different meaning or treated differently in iOS

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

chunhtai · 2023-10-06T22:27:26Z

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

+    // The end of document with forward affinity should still return the last line, which is used
+    // by voice control's delete line command.
+    if ([position isEqual:[_textInputView endOfDocument]]) {
+      return nil;


What would happen if this returns the last line?

If we return the last line, we will see the 2 bugs linked in my description.

I see, if endOfDocument is exclusive and does return (text.length, upstream), then it is contradicting with how flutter handle text position.

(text.length, downstream) is a valid cursor position as well. In most cases (text.length, upstream) and (text.length, downstream) refer to the same cursor positoin, but when there's bidi text at the end of the text they refer to different locations.

I tried in UITextField. It's returning 7F for a string that's 7 code units long. So it's inclusive I think.

I can play around with it more tomorrow, but afaik apple's sample project uses exclusive endOfDocument. (It does not use affinity tho)

Yeah cursor position and text position have different end positions. I'm not sure which one UITextField is using.

chunhtai · 2023-10-06T22:40:10Z

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

+    // The end of document with forward affinity should still return the last line, which is used
+    // by voice control's delete line command.
+    if ([position isEqual:[_textInputView endOfDocument]]) {
+      return nil;


I see, if endOfDocument is exclusive and does return (text.length, upstream), then it is contradicting with how flutter handle text position.

chunhtai · 2023-10-06T22:43:29Z

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

 - (UITextRange*)lineEnclosingPosition:(UITextPosition*)position {
+  // TODO(hellohuanlin): Remove iOS 17 check. The same logic should apply to older versions too.
+  if (@available(iOS 17.0, *)) {
+    // The end of document (with backward affinity) should not be part of any line, since end is


maybe mention this is iOS expectation, since in flutter (text.length, TextAffinity.upstream) is still considered pointing to the last character due to gesture reason. e.g., we need to tell the difference between user hitting the last half of the last character vs outside of the last character to the right.

…to correction menu on iOS 17"

WithToken · 2023-10-12T02:24:06Z

I found that I couldn't reproduce this problem on iOS17.1.

castleche · 2023-10-13T06:24:38Z

Not repro on iOS 17.1 beta 3.
@hellohuanlin Please make sure your fix code works fine on iOS 17.1.

chinmaygarde · 2023-10-19T20:13:37Z

Are we making progress on this?

hellohuanlin · 2023-10-19T21:31:17Z

Are we making progress on this?

Yes, still working on it

hellohuanlin · 2023-11-01T19:23:57Z

Closing this in favor of #47566

…ges (without relying on text affinity) (#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 #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.* #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

…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

github-actions bot added the platform-ios label Oct 5, 2023

hellohuanlin force-pushed the text_input_freeze_switch_language branch from ae7db18 to 9cb0af7 Compare October 5, 2023 18:58

hellohuanlin changed the title ~~[DRAFT][ios17][text_input]fix chinese language switch freeze, and missing auto correction menu in iOS 17~~ [ios17][text_input]fix chinese language switch freeze, and missing auto correction menu in iOS 17 Oct 6, 2023

hellohuanlin marked this pull request as ready for review October 6, 2023 17:40

hellohuanlin requested review from LongCatIsLooong and stuartmorgan-g October 6, 2023 17:40

stuartmorgan-g suggested changes Oct 6, 2023

View reviewed changes

LongCatIsLooong reviewed Oct 6, 2023

View reviewed changes

chunhtai reviewed Oct 6, 2023

View reviewed changes

hellohuanlin added 2 commits October 9, 2023 11:06

[ios17][text_input]fix chinese language switch freeze, and missing au…

808c1a7

…to correction menu on iOS 17"

address some feedbacks

a0ad92d

hellohuanlin force-pushed the text_input_freeze_switch_language branch from 9cb0af7 to a0ad92d Compare October 9, 2023 18:20

hellohuanlin mentioned this pull request Nov 1, 2023

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

Merged

8 tasks

hellohuanlin closed this Nov 1, 2023

hellohuanlin mentioned this pull request Nov 14, 2023

[CP (Stable)] [ios17][text_input]fix ios 17.0 keyboard freeze when switching languages flutter/flutter#138439

Closed

	// This is an upstream position
	closestPosition = [FlutterTextPosition positionWithIndex:position
	affinity:UITextStorageDirectionBackward];

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

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

Uh oh!

Conversation

hellohuanlin commented Oct 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Oct 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WithToken commented Oct 12, 2023

Uh oh!

castleche commented Oct 13, 2023

Uh oh!

chinmaygarde commented Oct 19, 2023

Uh oh!

hellohuanlin commented Oct 19, 2023

Uh oh!

hellohuanlin commented Nov 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

hellohuanlin commented Oct 5, 2023 •

edited

Loading

LongCatIsLooong Oct 6, 2023 •

edited

Loading