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

Remove UIAccessibilityTraitKeyboardKey to fix touch typing#52333

Merged
auto-submit[bot] merged 1 commit into
flutter-team-archive:mainfrom
vashworth:voiceover_touch_typing_bug
Apr 24, 2024
Merged

Remove UIAccessibilityTraitKeyboardKey to fix touch typing#52333
auto-submit[bot] merged 1 commit into
flutter-team-archive:mainfrom
vashworth:voiceover_touch_typing_bug

Conversation

@vashworth

@vashworth vashworth commented Apr 23, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

UIAccessibilityTraitKeyboardKey was added in #4575 so that TextInputSemanticsObject supported VoiceOver gestures for text editing features, such as pinch to select text, and up/down fling to move cursor. After experimenting with it, I found that those features were still available even after removing UIAccessibilityTraitKeyboardKey.

Fixes flutter/flutter#94465.

In Touch Typing Mode:

touch_typing.mov

In Standard Typing Mode:

standard_typing.mov

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.

@vashworth vashworth marked this pull request as ready for review April 23, 2024 20:00
@vashworth vashworth requested a review from chunhtai April 23, 2024 20:00
@vashworth

vashworth commented Apr 23, 2024

Copy link
Copy Markdown
Contributor Author

The original PR was made in 2018, so it could be that this changed in a newer version of iOS or something about how we do text input has changed? It's unclear to me. I only have an iOS 17 device to test with. @chunhtai Do you think it's worth wrapping in an @available to limit to iOS 17 since that's the only one I was able to test?

chunhtai
chunhtai reviewed Apr 24, 2024
View reviewed changes

@chunhtai chunhtai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you talk a bit more on why removing the trait fixes the issue? What is the observed behavior before and after?

@vashworth

vashworth commented Apr 24, 2024

Copy link
Copy Markdown
Contributor Author

Can you talk a bit more on why removing the trait fixes the issue? What is the observed behavior before and after?

Yes, the issue is that when touch typing is on, accessibilityActivate is triggered after only one tap. Whereas with standard typing, it's triggered after double tapping.

https://github.com/flutter/engine/blob/33c828fb3ff5c7628dbb01fbf40ed95831ad00d7/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.mm#L262-L267

This is happening due to Flutter adding the UIAccessibilityTraitKeyboardKey accessibility trait to TextInputSemanticsObject.

https://github.com/flutter/engine/blob/33c828fb3ff5c7628dbb01fbf40ed95831ad00d7/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.mm#L307-L312

According to Apple's documentation UIAccessibilityTraitKeyboardKey tells iOS that the "accessibility element behaves like a keyboard key". And if we look at the different typing modes, Touch typing reacts on a single tap on a keyboard key whereas Standard typing reacts on a double tap on a keyboard key:

Standard typing: Select a key by swiping left or right on the keyboard, then double-tap to enter the character.
Touch typing: Touch a key on the keyboard to select it, then lift your finger to enter the character.

So by treating TextInputSemanticsObject as a keyboard key, it reacts to a single tap when touch typing is enabled.

By removing UIAccessibilityTraitKeyboardKey, TextInputSemanticsObject is no longer treated as a keyboard key and therefore does not react on single touch when Touch typing is enabled. You can observe this by being in Touch typing mode, closing the keyboard, and then clicking on the text field. It will not open the keyboard on a single tap, which is the correct behavior. It will open on double tap. The videos I included in the description also show that the pinch to select text and up/down fling to move cursor functionality still works.

@chunhtai

chunhtai commented Apr 24, 2024

Copy link
Copy Markdown
Contributor

Thanks for explanation, yes this makes sense. According to the doc, that trait should probably not be on the Textfield semantics object.

chunhtai
chunhtai approved these changes Apr 24, 2024
View reviewed changes

@chunhtai chunhtai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@vashworth vashworth added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 24, 2024
@auto-submit auto-submit Bot merged commit b30c0a7 into flutter-team-archive:main Apr 24, 2024
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 24, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 24, 2024
@engine-flutter-autoroll
b30c0a7 Remove UIAccessibilityTraitKeyboardKey to fix touch typing (f…
dd841b1 
…lutter-team-archive/engine#52333)
auto-submit Bot pushed a commit to flutter/flutter that referenced this pull request Apr 24, 2024
@engine-flutter-autoroll
Roll Flutter Engine from b5d5832 to b30c0a7 (14 revisions) (#147336)
3bd2980 
flutter-team-archive/engine@b5d5832...b30c0a7

2024-04-24 15619084+vashworth@users.noreply.github.com Remove UIAccessibilityTraitKeyboardKey to fix touch typing (flutter-team-archive/engine#52333)
2024-04-24 bdero@google.com [Impeller] Remove libtess2 from libflutter. (flutter-team-archive/engine#52357)
2024-04-24 skia-flutter-autoroll@skia.org Roll Skia from 510b6766d907 to afcc1db27593 (2 revisions) (flutter-team-archive/engine#52367)
2024-04-24 yjbanov@google.com [web:tests] switch to new HTML DOM matcher (flutter-team-archive/engine#52354)
2024-04-24 jonahwilliams@google.com [Impeller] use spec constant for gaussian shader, rename, and reuse vertex sources. (flutter-team-archive/engine#52361)
2024-04-24 jonahwilliams@google.com [Impeller] delete points compute shader. (flutter-team-archive/engine#52346)
2024-04-24 byoungchan.lee@gmx.com [darwin] Update pixel format handling in FlutterTexture (flutter-team-archive/engine#52326)
2024-04-24 jonahwilliams@google.com [Impeller] make drawAtlas always use porterduff or vertices_uber shader (flutter-team-archive/engine#52348)
2024-04-24 magder@google.com Migrate ios_surface files to ARC (flutter-team-archive/engine#52139)
2024-04-24 skia-flutter-autoroll@skia.org Roll Dart SDK from f470eaaf6e6d to 38c43a01a51e (1 revision) (flutter-team-archive/engine#52365)
2024-04-24 skia-flutter-autoroll@skia.org Roll Skia from b5dd23bd29df to 510b6766d907 (16 revisions) (flutter-team-archive/engine#52364)
2024-04-24 jason-simmons@users.noreply.github.com Fix some warnings reported by recent versions of clang-tidy (flutter-team-archive/engine#52349)
2024-04-24 skia-flutter-autoroll@skia.org Roll Skia from e15464e6e982 to b5dd23bd29df (1 revision) (flutter-team-archive/engine#52353)
2024-04-24 skia-flutter-autoroll@skia.org Roll Dart SDK from 5227dc5103f6 to f470eaaf6e6d (1 revision) (flutter-team-archive/engine#52359)

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 jimgraham@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://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
@hellohuanlin hellohuanlin mentioned this pull request Aug 8, 2024
Merged
8 tasks
@hellohuanlin hellohuanlin mentioned this pull request Aug 19, 2024
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@chunhtai chunhtai chunhtai approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-ios

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

On iOS, VoiceOver incorrectly activated the pop-up keyboard after touching TextField

2 participants

@vashworth @chunhtai