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
merged 1 commit into from
Apr 24, 2024

Conversation

vashworth
Copy link
Contributor

@vashworth vashworth commented Apr 23, 2024

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
Copy link
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?

Copy link
Contributor

@chunhtai chunhtai left a comment

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
Copy link
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.

- (BOOL)accessibilityActivate {
if (![self isAccessibilityBridgeAlive]) {
return false;
}
return [[self textInputSurrogate] accessibilityActivate];
}

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

// Adding UIAccessibilityTraitKeyboardKey to the trait list so that iOS treats it like
// a keyboard entry control, thus adding support for text editing features, such as
// pinch to select text, and up/down fling to move cursor.
UIAccessibilityTraits results = [super accessibilityTraits] |
[self textInputSurrogate].accessibilityTraits |
UIAccessibilityTraitKeyboardKey;

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
Copy link
Contributor

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

Copy link
Contributor

@chunhtai chunhtai left a comment

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:main Apr 24, 2024
28 checks passed
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Apr 24, 2024
…147336)

flutter/engine@b5d5832...b30c0a7

2024-04-24 [email protected] Remove UIAccessibilityTraitKeyboardKey to fix touch typing (flutter/engine#52333)
2024-04-24 [email protected] [Impeller] Remove libtess2 from libflutter. (flutter/engine#52357)
2024-04-24 [email protected] Roll Skia from 510b6766d907 to afcc1db27593 (2 revisions) (flutter/engine#52367)
2024-04-24 [email protected] [web:tests] switch to new HTML DOM matcher (flutter/engine#52354)
2024-04-24 [email protected] [Impeller] use spec constant for gaussian shader, rename, and reuse vertex sources. (flutter/engine#52361)
2024-04-24 [email protected] [Impeller] delete points compute shader. (flutter/engine#52346)
2024-04-24 [email protected] [darwin] Update pixel format handling in FlutterTexture (flutter/engine#52326)
2024-04-24 [email protected] [Impeller] make drawAtlas always use porterduff or vertices_uber shader (flutter/engine#52348)
2024-04-24 [email protected] Migrate ios_surface files to ARC (flutter/engine#52139)
2024-04-24 [email protected] Roll Dart SDK from f470eaaf6e6d to 38c43a01a51e (1 revision) (flutter/engine#52365)
2024-04-24 [email protected] Roll Skia from b5dd23bd29df to 510b6766d907 (16 revisions) (flutter/engine#52364)
2024-04-24 [email protected] Fix some warnings reported by recent versions of clang-tidy (flutter/engine#52349)
2024-04-24 [email protected] Roll Skia from e15464e6e982 to b5dd23bd29df (1 revision) (flutter/engine#52353)
2024-04-24 [email protected] Roll Dart SDK from 5227dc5103f6 to f470eaaf6e6d (1 revision) (flutter/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 [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
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.

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