[ios] Fix text input edit rotor accessibility #54351

hellohuanlin · 2024-08-05T19:14:40Z

Fixes text input edit rotor accessibility missing edit actions.

It also fixes a few edit items that is displayed where it's not supposed to. (e.g. displaying select item while nothing to select).

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

Fixes flutter/flutter#151029

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 · 2024-08-07T18:44:16Z

Verified Select and Select All options will only be available when the text field is not empty.

hellohuanlin · 2024-08-08T21:24:21Z

Verified still need double tap gesture to reveal the edit options with this change, but it seems to be also true for native text field. I will check our behavior before this change to see if it's a regression.

hellohuanlin · 2024-08-16T20:54:21Z

After reverting #52333, I was able to confirm that it's been an existing behavior to require to double tap to bring up this rotor. So it's not a regression.

jmagman · 2024-08-20T18:50:42Z

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

@@ -462,4 +462,34 @@ - (BOOL)hasText {
  return [[self textInputSurrogate] hasText];
 }

+#pragma mark - UIResponder overrides
+
+- (void)cut:(id)sender {


I'm not sure all the gotchas, but can this class use forwardingTargetForSelector instead of directly calling every method on textInputSurrogate? It's inevitable we will miss some as they are added in future SDKs. I'm not sure it's the right behavior for this class, though, and I'm not sure if the super class UIAccessibilityElement already implements, in which case it won't work...

It turns out that iOS determines whether to show these options in accessibility rotor by checking the presence of these functions, So we can't use forwardingTargetForSelector.

hellohuanlin · 2024-08-22T23:01:01Z

Not related to objc message forwarding, but found a very interesting crash on select. Looking into it!

hellohuanlin · 2024-08-23T20:41:22Z

Surprisingly, it turns out that select: button is not supported (see flutter/flutter#107578), hence the crash. Adding select: would be out of scope of this PR.

chunhtai

LGTM

…ne#54351)

…ions) (#154153) Manual roll requested by [email protected] flutter/engine@f645ca5...b41ca79 2024-08-26 [email protected] [Impeller] fix incorrect origins for mesh gradient computation. (flutter/engine#54762) 2024-08-26 [email protected] Change the `ci/analyze.sh` script to analyze _all_ of the engine (flutter/engine#54779) 2024-08-26 [email protected] Use GNI group instead of hardcoding PNG codecs source files. (flutter/engine#54781) 2024-08-26 [email protected] [ios] Fix text input edit rotor accessibility (flutter/engine#54351) 2024-08-26 [email protected] Ensure orchestrators aren't assigned to 32 core machines (flutter/engine#54754) 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

…ions) (flutter#154153) Manual roll requested by [email protected] flutter/engine@f645ca5...b41ca79 2024-08-26 [email protected] [Impeller] fix incorrect origins for mesh gradient computation. (flutter/engine#54762) 2024-08-26 [email protected] Change the `ci/analyze.sh` script to analyze _all_ of the engine (flutter/engine#54779) 2024-08-26 [email protected] Use GNI group instead of hardcoding PNG codecs source files. (flutter/engine#54781) 2024-08-26 [email protected] [ios] Fix text input edit rotor accessibility (flutter/engine#54351) 2024-08-26 [email protected] Ensure orchestrators aren't assigned to 32 core machines (flutter/engine#54754) 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

luomo-pro · 2024-09-11T08:04:03Z

@hellohuanlin
Hello, after testing, I found that it's not perfect yet. There are still two issues:

After selecting all content, the "select all" operation is still available in the rotor.
Operations can only be performed on the text field that has the keyboard focus activated.

As I mentioned in flutter/flutter#151029 :

Suppose there are two text fields on an interface.

Double-tap to activate the first text field, and the keyboard pops up.
Then, use VoiceOver to focus on the second text field. Note: just focus on it, don't double-tap.
Finally, use the rotor to perform a paste operation.

Native behavior: The text will be pasted into the second text field.
Flutter behavior: Unable to perform the paste operation.

In other words, the native rotor operation targets the text field that VoiceOver is focused on.
However, Flutter rotor operation only targets the text field that has the keyboard activated.

hellohuanlin · 2024-09-11T17:10:39Z

@luomo-pro

After selecting all content, the "select all" operation is still available in the rotor.

I can probably put up a PR to disable selectAll in that case

Operations can only be performed on the text field that has the keyboard focus activated.

This has been the existing behavior before I reverted the PR, so not a regression. Can you create a new issue for that?

luomo-pro · 2024-09-12T02:22:25Z

@hellohuanlin
Okay, thank you very much. I will submit a separate issue for the second question

github-actions bot added the platform-ios label Aug 5, 2024

hellohuanlin force-pushed the text_input_edit_rotor_accessibility branch from c9409e9 to 05f72a9 Compare August 5, 2024 19:59

hellohuanlin force-pushed the text_input_edit_rotor_accessibility branch from 05f72a9 to c48c830 Compare August 16, 2024 20:50

hellohuanlin changed the title ~~[Draft][ios] Fix text input edit rotor accessibility~~ [ios] Fix text input edit rotor accessibility Aug 19, 2024

hellohuanlin force-pushed the text_input_edit_rotor_accessibility branch from c48c830 to 239b294 Compare August 19, 2024 17:04

hellohuanlin marked this pull request as ready for review August 19, 2024 17:07

hellohuanlin requested a review from chunhtai August 19, 2024 17:07

jmagman reviewed Aug 20, 2024

View reviewed changes

[ios]fix text input rotor accessiblity

9738113

remove select: function

bf10ef4

hellohuanlin force-pushed the text_input_edit_rotor_accessibility branch from 239b294 to bf10ef4 Compare August 23, 2024 20:44

hellohuanlin requested a review from jmagman August 23, 2024 20:45

chunhtai approved these changes Aug 23, 2024

View reviewed changes

also fix test

8fe402d

hellohuanlin added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2024

auto-submit bot merged commit c346a96 into flutter:main Aug 26, 2024
29 checks passed

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

Manual roll Flutter Engine from f645ca5d8433 to b41ca79092e8 (5 revisions) flutter/flutter#154153

Merged

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

c346a967a [ios] Fix text input edit rotor accessibility (flutter/engi…

0f09b2a

…ne#54351)

jmagman mentioned this pull request Aug 27, 2024

iOS VoiceOver accessibility regression: EditableText rotor actions only available when keyboard is focused flutter/flutter#151029

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ios] Fix text input edit rotor accessibility #54351

[ios] Fix text input edit rotor accessibility #54351

Uh oh!

hellohuanlin commented Aug 5, 2024 •

edited

Loading

Uh oh!

hellohuanlin commented Aug 7, 2024

Uh oh!

hellohuanlin commented Aug 8, 2024 •

edited

Loading

Uh oh!

hellohuanlin commented Aug 16, 2024

Uh oh!

jmagman Aug 20, 2024

Uh oh!

hellohuanlin Aug 23, 2024

Uh oh!

hellohuanlin commented Aug 22, 2024

Uh oh!

hellohuanlin commented Aug 23, 2024

Uh oh!

chunhtai left a comment

Uh oh!

Uh oh!

luomo-pro commented Sep 11, 2024 •

edited

Loading

Uh oh!

hellohuanlin commented Sep 11, 2024

Uh oh!

luomo-pro commented Sep 12, 2024

Uh oh!

Uh oh!

[ios] Fix text input edit rotor accessibility #54351

[ios] Fix text input edit rotor accessibility #54351

Uh oh!

Conversation

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

Uh oh!

Pre-launch Checklist

Uh oh!

hellohuanlin commented Aug 7, 2024

Uh oh!

hellohuanlin commented Aug 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hellohuanlin commented Aug 16, 2024

Uh oh!

jmagman Aug 20, 2024

Choose a reason for hiding this comment

Uh oh!

hellohuanlin Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

hellohuanlin commented Aug 22, 2024

Uh oh!

hellohuanlin commented Aug 23, 2024

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

luomo-pro commented Sep 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hellohuanlin commented Sep 11, 2024

Uh oh!

luomo-pro commented Sep 12, 2024

Uh oh!

Uh oh!

hellohuanlin commented Aug 5, 2024 •

edited

Loading

hellohuanlin commented Aug 8, 2024 •

edited

Loading

luomo-pro commented Sep 11, 2024 •

edited

Loading