Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[ios][autocorrection]disable auto-correction highlight in iOS 17 #44176

Merged

Conversation

hellohuanlin
Copy link
Contributor

@hellohuanlin hellohuanlin commented Jul 31, 2023

This PR disables the "auto-correction highlight" feature in iOS 17.

This feature was introduced in flutter/flutter#45354 and #13959 (CC: @LongCatIsLooong who was the original author)

I have created a new issue to find other approaches to re-enable this feature.

Note that "auto-correction" itself still works, it's only the "highlight" part is disabled:

Why disable this feature?

The original PR uses UITextInput::firstRectForRange API for auto-correction, since Apple does not provide any other API when auto-correction should show up, so the original PR used this API as a workaround.

In iOS 17, Apple changed a few UITextInput behaviors:

  • UIKit does not query UITextInput::firstRectForRange for text range of the auto-corrected word any more.
  • But instead, it repeatedly queries every single character of the current word (after entering or deleting a character), regardless whether the word should be auto-corrected or not.

I have tried all other UITextInput APIs that takes a text range, and none are suitable for auto-correction feature. As a result, I have to disable this feature for iOS 17 for now.

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

Fixes flutter/flutter#128406

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 Hixie said 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.

@LongCatIsLooong
Copy link
Contributor

The text highlight on iOS 17 at the top left corner looks out of place. If possible that should be fixed as well?

@hellohuanlin
Copy link
Contributor Author

@LongCatIsLooong Yes, that's the next thing I plan to do. Will do more research, but this one is more urgent.

end:end
withClient:_textInputClient];
if (@available(iOS 17.0, *)) {
// Disable auto-correction highlight feature for iOS 17.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: iOS 17+

@@ -314,6 +314,11 @@ - (void)testSettingKeyboardTypeNoneDisablesSystemKeyboard {
}

- (void)testAutocorrectionPromptRectAppears {
// Auto-correction prompt is disabled in iOS 17.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test of that.

Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to testing that it's disabled in iOS 17 if possible.

What's your plan for hiding the autocorrect highlight altogether? I guess it depends on merging this PR first? Because I would almost say that showing the highlight on the last character could be better than showing the highlight in the top left corner.

Screenshot from 2023-08-01 10-13-09

@hellohuanlin
Copy link
Contributor Author

hellohuanlin commented Aug 1, 2023

What's your plan for hiding the autocorrect highlight altogether?

@justinmc I plan to hide the top highlight as well.

While I was investigating that, I found something interesting - even in iOS 16, the top highlight shows up, but with just a single char length, rather than a word length. It's just that now in iOS 17 this problem becomes more noticeable. Still I'm surprised that no one notice it before. I created this issue with more details.

@hellohuanlin
Copy link
Contributor Author

Because I would almost say that showing the highlight on the last character could be better than showing the highlight in the top left corner.

@justinmc to clarify, the highlight on the last char shows up on every keystroke. So it is likely a "deal breaker" for iOS apps. Here's a video of it: flutter/flutter#128406 (comment)

On the other hand, the top left corner highlight only shows up for misspelled word. It's much less frequent, but still very bad though, so its the next thing in my list :)

Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for the clarification. I agree that this is an improvement then. LGTM 👍

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

As soon as this rolls into the framework tree and has been confirmed working on Flutter master, please make sure to request a cherry pick to beta.

@hellohuanlin hellohuanlin added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 2, 2023
@auto-submit auto-submit bot merged commit 42b4cfd into flutter:main Aug 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 3, 2023
fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Aug 3, 2023
…sions) (#131830)

Manual roll requested by jacksongardner@google.com

flutter/engine@9304c00...4c1157b

2023-08-03 jacksongardner@google.com Revert Android Hardware Texture PRs (flutter/engine#44310)
2023-08-03 skia-flutter-autoroll@skia.org Roll Dart SDK from 87df1bbcea5e to 3b9af2825d47 (2 revisions) (flutter/engine#44308)
2023-08-03 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from Hx7ap5qcoqRIknnnG... to WwFjJuQF_rpToCYPJ... (flutter/engine#44306)
2023-08-03 jason-simmons@users.noreply.github.com Check whether the lookup of android.hardware.HardwareBuffer found a class (flutter/engine#44304)
2023-08-02 bdero@google.com [Impeller] Add placeholder filter input. (flutter/engine#44290)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from 6c1bab070220 to 6a09e41ce6ea (1 revision) (flutter/engine#44300)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from fd5bd67d532f to c0956a252f30 (1 revision) (flutter/engine#44296)
2023-08-02 chris@bracken.jp [iOS] Fix use-after-free in setBinaryMessenger (flutter/engine#44294)
2023-08-02 louiseh0313@gmail.com Add Search Web to selection controls on iOS (flutter/engine#43324)
2023-08-02 jason-simmons@users.noreply.github.com Improve logging in the clang-tidy script (flutter/engine#44228)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from 335c6b86d70b to 6c1bab070220 (1 revision) (flutter/engine#44291)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 25f5a32367ad to fd5bd67d532f (2 revisions) (flutter/engine#44289)
2023-08-02 john@johnmccutchan.com Be sure to clear exceptions after a failed JNI lookup (flutter/engine#44293)
2023-08-02 derekx@google.com Handle deprecation of Dart_TimelineEvent Embedder API (flutter/engine#42497)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from ccc17f784e5d to 25f5a32367ad (4 revisions) (flutter/engine#44283)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from 01ee134bb223 to 335c6b86d70b (2 revisions) (flutter/engine#44287)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 7104d0e8863f to ccc17f784e5d (2 revisions) (flutter/engine#44279)
2023-08-02 41930132+hellohuanlin@users.noreply.github.com [ios][autocorrection]disable auto-correction highlight in iOS 17 (flutter/engine#44176)
2023-08-02 john@johnmccutchan.com Reland Introduce TextureRegistry.ImageTexture and HardwareBufferExternalTextureGL (flutter/engine#44278)
2023-08-02 skia-flutter-autoroll@skia.org Roll Dart SDK from afbaf4216fc8 to 87df1bbcea5e (1 revision) (flutter/engine#44276)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 93764a98b866 to 7104d0e8863f (4 revisions) (flutter/engine#44273)
2023-08-02 jason-simmons@users.noreply.github.com [Impeller] Fix leak of wrapped TextureMTL objects in the Metal embedder API (flutter/engine#44245)
2023-08-02 737941+loic-sharma@users.noreply.github.com Revert "Listen to window notifications to update application lifecycle" (flutter/engine#44275)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 514c66ce0471 to 93764a98b866 (1 revision) (flutter/engine#44270)

Also rolling transitive DEPS:
  fuchsia/sdk/core/mac-amd64 from Hx7ap5qcoqRI to WwFjJuQF_rpT

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 jacksongardner@google.com,rmistry@google.com,zra@google.com 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot pushed a commit that referenced this pull request Aug 4, 2023
Original PR: #44176

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

flutter/flutter#131890

*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
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
…tter#44176)

This PR disables the "auto-correction highlight" feature in iOS 17. 

This feature was introduced in flutter/flutter#45354 and flutter#13959 (CC: @LongCatIsLooong who was the original author)

I have created [a new issue](flutter/flutter#131622) to find other approaches to re-enable this feature. 

**Note that "auto-correction" itself still works, it's only the "highlight" part is disabled:**

- iOS 16 with highlight:
https://github.com/flutter/engine/assets/41930132/2fe7bbf6-f2db-4212-a020-e420ad8dd5e6

- iOS 17 without highlight:
https://github.com/flutter/engine/assets/41930132/34f34743-6bef-4e93-80d2-d04c92ba59bf

## Why disable this feature?

The original PR uses `UITextInput::firstRectForRange` API for auto-correction, since Apple does not provide any other API when auto-correction should show up, so the original PR used this API as a workaround. 

In iOS 17, Apple changed a few `UITextInput` behaviors:  
- UIKit does not query `UITextInput::firstRectForRange` for text range of the auto-corrected word any more. 
- But instead, it repeatedly queries every single character of the current word (after entering or deleting a character), regardless whether the word should be auto-corrected or not. 

I have tried all other `UITextInput` APIs that takes a text range, and none are suitable for auto-correction feature. As a result, I have to disable this feature for iOS 17 for now. 

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

Fixes flutter/flutter#128406

*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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-ios
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS]After entering text into the iOS 17 text input box, the cursor selects the last character
4 participants