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

Bare-bones iOS FKA implementation #55964

Merged
merged 7 commits into from
Oct 24, 2024

Conversation

LongCatIsLooong
Copy link
Contributor

@LongCatIsLooong LongCatIsLooong commented Oct 18, 2024

A bare-bones implementation for iOS focus engine support, to enable basic full keyboard access (FKA) (except for scrolling which will be implemented in a different patch).

Partially f1xes flutter/flutter#76497

Screen.Recording.2024-10-21.at.7.11.10.PM.mov

On iOS 15 and above, FKA, if enabled, always consumes relevant key events, so the Flutter framework can't see those key events as they won't be delivered via the UIResponder chain (https://developer.apple.com/documentation/uikit/uikeycommand/3780513-wantspriorityoversystembehavior). This patch provides the basic focus-related information to the iOS focus engine, derived from the information already available in the accessibility tree, so the iOS focus engine can navigate the UI hierarchy and invoke accessibilityActivate on the current focus when the user presses the space key.

This at the moment seems to be the best option:

  • There doesn't seem to be a way to reliably prevent FKA from consuming the key events and that seems to be by design.
  • The user can remap the FKA keys in iOS system settings, but that key mapping isn't available to apps, so even if the framework can get the key events it won't be able to honor custom key maps.
  • When FKA is on, -[FlutterView isAccessibilityElement] is called without user interaction (presumably it's called when the view appears), so when the user interacts with the app using FKA, it's likely that the accessibility is already enabled, we don't have to worry detecting whether FKA is on (at least for now).

Scrolling using FKA currently does not work despite FlutterScrollableSemanticsObject conforms to UIFocusItemScrollableContainer. setContentOffset: must be implemented using a new API that informs the framework of the new contentOffset in the scroll view. accessibilityScroll does not work because it scrolls too much in most cases, and it tells the framework "how much to scroll" instead of "where to scroll to" so it tends to be jumpy.

What happens on iOS versions earlier than 15

I couldn't find iOS 14 runtime for simulators in xcode 16 so I couldn't test it. But since the key events will be delivered to the framework first regardless of whether FKA is enabled, the framework is supposed to handle keyboard focus navigation even when FKA is on.

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.

…asic full keyboard access (FKA), except for scrolling which will be implemented in a different patch.

Partially fixes #76497

On iOS 15 and above, FKA, if enabled, always consumes relevant key events, so the Flutter framework can't see those key events as they won't be delivered via the UIResponder chain (https://developer.apple.com/documentation/uikit/uikeycommand/3780513-wantspriorityoversystembehavior). This patch provides the basic focus-related information to the iOS focus engine, based on the information that is already available in the accessibility tree, so the iOS focus engine can navigate the UI hierarchy and invoke `accessibilityActivate` on the current focus when the user presses the space key.

This at the moment seems to be the best option:
- There doesn't seem to be a way to reliably prevents FKA from consuming the key events and that seems to be by design.
- The user can remap the FKA keys in iOS system settings, but that key mapping isn't available to apps, so even if the framework can get the key events it won't be able to honor custom key maps.
- When FKA is on, `-[FlutterView isAccessibilityElement]` is called without user interaction (presumably it's called when the view appears), so when the user interacts with the app using FKA, it's likely that the accessibility is already enabled, we don't have to worry detecting whether FKA is on (at least for now).

Scrolling using FKA currently does not work despite `FlutterScrollableSemanticsObject` conforms to `UIFocusItemScrollableContainer`.
`setContentOffset:` must be implemented using a new API that informs the framework of the new contentOffset in the scroll view. `accessibilityScroll` does not work because it scrolls too much in most cases.
@LongCatIsLooong LongCatIsLooong marked this pull request as draft October 18, 2024 20:59
@LongCatIsLooong LongCatIsLooong changed the title Bare-bones iOS FKA implementation for iOS focus engine support, to enable b… Bare-bones iOS FKA implementation Oct 18, 2024
@LongCatIsLooong LongCatIsLooong marked this pull request as ready for review October 18, 2024 23:15
@hannah-hyj
Copy link
Member

hannah-hyj commented Oct 21, 2024

Video looks cool! 😀

I'm curious if the a11y mode and FKA mode are both on, do a11y focus (you can still swipe to focus) and keyboard focus affect each other?

#pragma mark - UIFocusItemContainer Conformance

- (NSArray<id<UIFocusItem>>*)focusItemsInRect:(CGRect)rect {
// It seems the iOS focus system rely heavily on this method (instead of
Copy link
Member

Choose a reason for hiding this comment

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

nit: relies

nit: I know by "this method" you're referring to focusItemsInRect but because of the location of this comment, "this method" can be confused as "self.childrenInHitTestOrder"

- (NSArray<id<UIFocusItem>>*)focusItemsInRect:(CGRect)rect {
// It seems the iOS focus system rely heavily on this method (instead of
// preferredFocusEnvironments) for directional navigation.
// Whether the item order in the returned array matters is unknown.
Copy link
Member

Choose a reason for hiding this comment

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

interesting 🫨 So if you set it to return a reversed "self.childrenInHitTestOrder", the traversing order is the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I guess the iOS focus engine re-organize the returned array using the coordinates reported by frame .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh never mind. I turned FKA off on the simulator that's why everything started working magically.

@LongCatIsLooong
Copy link
Contributor Author

LongCatIsLooong commented Oct 21, 2024

Video looks cool! 😀

I'm curious if the a11y mode and FKA mode are both on, do a11y focus (you can still swipe to focus) and keyboard focus affect each other?

In practice, using voiceover (I'm not familiar with accessibility features so I don't know if there's anything else could change the accessibility focus) and FKA together should be uncommon.
It's not mentioned in the WWDC videos or the documentation afaik (they did mention that the UIResponder focus "should" be updated to follow the iOS engine focus but SemanticsObject isn't even a UIRensponder ). But FKA could affect accessibility focus, since the accessibility focus methods are defined on NSObject and a UIFocusItems have to be a NSObject.
Would that be a concern if FKA affects accessibility focus?

@hannah-hyj
Copy link
Member

hannah-hyj commented Oct 21, 2024

There's anything else could change the accessibility focus

There're some different a11y modes can change a11y focus
on iOS, there are VoiceOver , Voice Control, Switch Control
on Android, similarly, there are Android Talkback , Voice Access , Switch Access
voice over is the main a11y feature.
voice access is basically say something like "tap cancel button" to tap instead of using your fingers to tap.
switch access is using an external switch to change a11y focus.

using voiceover and FKA together should be uncommon.

We don't know that. I think we should test the behavior using voiceover and FKA together.

Would that be a concern if FKA affects accessibility focus

FKA is a new thing to me. I don't know if it should affect a11y focus or not. I think our behavior should follow the native.

@LongCatIsLooong
Copy link
Contributor Author

LongCatIsLooong commented Oct 22, 2024

I tried out some UIControls (including UITextField), none of them requested accessibility focus when the highlighted under FKA. So UIKit doesn't seem to automatically update accessibility focus based on iOS focus engine focus (or the other way around) in general.

@LongCatIsLooong
Copy link
Contributor Author

LongCatIsLooong commented Oct 22, 2024

Video looks cool! 😀

I'm curious if the a11y mode and FKA mode are both on, do a11y focus (you can still swipe to focus) and keyboard focus affect each other?

The original demo video was a lie unfortunately. I forgot to turn on FKA on that simulator so everything suddenly started working. I reverted some deleted changes. Updated the video. Scrolling is not working (as expected since I didn't implement setContentOffset).

[reversedItems
addObject:self.childrenInHitTestOrder[self.childrenInHitTestOrder.count - 1 - i]];
}
return reversedItems;
Copy link
Member

Choose a reason for hiding this comment

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

Returning a reversed array seems very unintuitive, are you doing this just to make menus and dialogs reachable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it's undocumented. I tested in an example app, popup menus couldn't be focused (instead the model barrier is the only thing focusable when a popup menu is present) if the items were in hit test order.

@hannah-hyj
Copy link
Member

I tried out some UIControls (including UITextField), none of them requested accessibility focus when the highlighted under FKA. So UIKit doesn't seem to automatically update accessibility focus based on iOS focus engine focus (or the other way around) in general.

So a11y focus, FKA focus, and keyboard input focus, can be three different thing? Does the a11y focus change if you tap a widget with FKA?

@LongCatIsLooong
Copy link
Contributor Author

I tried out some UIControls (including UITextField), none of them requested accessibility focus when the highlighted under FKA. So UIKit doesn't seem to automatically update accessibility focus based on iOS focus engine focus (or the other way around) in general.

So a11y focus, FKA focus, and keyboard input focus, can be three different thing? Does the a11y focus change if you tap a widget with FKA?

Right in flutter all three can be different. The uicontrols I tested don't request accessibility focus on [NSObject accessibilityActivate]. So most Flutter widget shouldn't do that by default either I think

@hannah-hyj
Copy link
Member

Right in flutter all three can be different. The uicontrols I tested don't request accessibility focus on [NSObject accessibilityActivate]. So most Flutter widget shouldn't do that by default either I think

The behavior (accessibilityActivate not requiring accessibility focus) is different than I would assume, but I'm happy as long as our behavior is the same as native!

Copy link
Member

@hannah-hyj hannah-hyj left a comment

Choose a reason for hiding this comment

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

LGTM

@LongCatIsLooong LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 23, 2024
@auto-submit auto-submit bot merged commit 0b56cb8 into flutter:main Oct 24, 2024
31 checks passed
@LongCatIsLooong LongCatIsLooong deleted the full-keyboard-access branch October 24, 2024 00:38
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 24, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 24, 2024
…157486)

flutter/engine@5d7caf7...0b56cb8

2024-10-24 [email protected] Bare-bones iOS FKA implementation (flutter/engine#55964)
2024-10-23 [email protected] [Impeller] libImpeller: Allow appending to the transformation stack. (flutter/engine#56072)
2024-10-23 [email protected] Add standalone 'Mac clangd' builder to replace 'Linux mac_clangd' orchestrator (flutter/engine#56014)
2024-10-23 [email protected] Roll Dart SDK from 75c42f30af7a to dd06a1e3002c (2 revisions) (flutter/engine#56070)
2024-10-23 [email protected] Roll Skia from 42f9070e6625 to 53c9663c3b83 (1 revision) (flutter/engine#56069)

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] 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
M97Chahboun pushed a commit to M97Chahboun/flutter that referenced this pull request Oct 30, 2024
…lutter#157486)

flutter/engine@5d7caf7...0b56cb8

2024-10-24 [email protected] Bare-bones iOS FKA implementation (flutter/engine#55964)
2024-10-23 [email protected] [Impeller] libImpeller: Allow appending to the transformation stack. (flutter/engine#56072)
2024-10-23 [email protected] Add standalone 'Mac clangd' builder to replace 'Linux mac_clangd' orchestrator (flutter/engine#56014)
2024-10-23 [email protected] Roll Dart SDK from 75c42f30af7a to dd06a1e3002c (2 revisions) (flutter/engine#56070)
2024-10-23 [email protected] Roll Skia from 42f9070e6625 to 53c9663c3b83 (1 revision) (flutter/engine#56069)

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] 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
auto-submit bot pushed a commit that referenced this pull request Nov 25, 2024
This PR adds basic FKA scrolling support: when the iOS focus (the focus state is maintained separately from the framework focus, see the previous PR) switches to an item in a scrollable container that is too close to the edge of the viewport, the container will scroll to make sure the next item is visible. 

Previous PR for context: #55964

https://github.com/user-attachments/assets/84ae5153-f955-4d23-9901-ce942c0e98ac

### Why the UIScrollView subclass in the focus hierarchy

The iOS focus system does not provide an API that allows apps to notify it of focus highlight changes. So if we were to keep using the transforms sent by the framework as-is and not introducing any UIViews in the focus hierarchy, the focus highlight will be positioned at the wrong location after scrolling (via FKA or via framework). That does not seem to be part of the public API and the focus system seems to only know how to properly highlight focusable UIViews.

### Things that currently may not work

1. Nested scroll views (have not tried to verify) 

The `UIScrollView`s are always subviews of the `FlutterView`. If there are nested scrollables the focus system may not be able to properly determine the focus hierarchy (in theory the iOS focus system should never depend on `UIView.parentView` but I haven't tried to verify that).

2. If the next item is too far below the bottom of the screen and there is a tab bar with focusable items, the focus will be transferred to tab bar instead of the next item in the list

Video demo (as you can see the scrolling is really finicky):

https://github.com/user-attachments/assets/51c2bfe4-d7b3-4614-aa49-4256214f8978

I've tried doing the same thing using a `UITableView` with similar configurations but it seems to have the same problem. I'll try to dig a bit deeper into this and see if there's a workaround.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
auto-submit bot added a commit that referenced this pull request Nov 26, 2024
Reverts: #56606
Initiated by: LongCatIsLooong
Reason for reverting: flutter/flutter#159456
Original PR Author: LongCatIsLooong

Reviewed By: {chunhtai, cbracken}

This change reverts the following previous change:
This PR adds basic FKA scrolling support: when the iOS focus (the focus state is maintained separately from the framework focus, see the previous PR) switches to an item in a scrollable container that is too close to the edge of the viewport, the container will scroll to make sure the next item is visible. 

Previous PR for context: #55964

https://github.com/user-attachments/assets/84ae5153-f955-4d23-9901-ce942c0e98ac

### Why the UIScrollView subclass in the focus hierarchy

The iOS focus system does not provide an API that allows apps to notify it of focus highlight changes. So if we were to keep using the transforms sent by the framework as-is and not introducing any UIViews in the focus hierarchy, the focus highlight will be positioned at the wrong location after scrolling (via FKA or via framework). That does not seem to be part of the public API and the focus system seems to only know how to properly highlight focusable UIViews.

### Things that currently may not work

1. Nested scroll views (have not tried to verify) 

The `UIScrollView`s are always subviews of the `FlutterView`. If there are nested scrollables the focus system may not be able to properly determine the focus hierarchy (in theory the iOS focus system should never depend on `UIView.parentView` but I haven't tried to verify that).

2. If the next item is too far below the bottom of the screen and there is a tab bar with focusable items, the focus will be transferred to tab bar instead of the next item in the list

Video demo (as you can see the scrolling is really finicky):

https://github.com/user-attachments/assets/51c2bfe4-d7b3-4614-aa49-4256214f8978

I've tried doing the same thing using a `UITableView` with similar configurations but it seems to have the same problem. I'll try to dig a bit deeper into this and see if there's a workaround.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
A bare-bones implementation for iOS focus engine support, to enable basic full keyboard access (FKA) (except for scrolling which will be implemented in a different patch).

Partially f1xes flutter#76497

https://github.com/user-attachments/assets/427db87e-cc15-483a-85a1-56bf1c02c285

On iOS 15 and above, FKA, if enabled, always consumes relevant key events, so the Flutter framework can't see those key events as they won't be delivered via the `UIResponder` chain (https://developer.apple.com/documentation/uikit/uikeycommand/3780513-wantspriorityoversystembehavior). This patch provides the basic focus-related information to the iOS focus engine, derived from the information already available in the accessibility tree, so the iOS focus engine can navigate the UI hierarchy and invoke `accessibilityActivate` on the current focus when the user presses the space key.

This at the moment seems to be the best option:
- There doesn't seem to be a way to reliably prevent FKA from consuming the key events and that seems to be by design.
- The user can remap the FKA keys in iOS system settings, but that key mapping isn't available to apps, so even if the framework can get the key events it won't be able to honor custom key maps.
- When FKA is on, `-[FlutterView isAccessibilityElement]` is called without user interaction (presumably it's called when the view appears), so when the user interacts with the app using FKA, it's likely that the accessibility is already enabled, we don't have to worry detecting whether FKA is on (at least for now).

Scrolling using FKA currently does not work despite `FlutterScrollableSemanticsObject` conforms to `UIFocusItemScrollableContainer`. `setContentOffset:` must be implemented using a new API that informs the framework of the new contentOffset in the scroll view. `accessibilityScroll` does not work because it scrolls too much in most cases, and it tells the framework "how much to scroll" instead of "where to scroll to" so it tends to be jumpy.

## What happens on iOS versions earlier than 15

I couldn't find iOS 14 runtime for simulators in xcode 16 so I couldn't test it. But since the key events will be delivered to the framework first regardless of whether FKA is enabled, the framework is supposed to handle keyboard focus navigation even when FKA is on.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
This PR adds basic FKA scrolling support: when the iOS focus (the focus state is maintained separately from the framework focus, see the previous PR) switches to an item in a scrollable container that is too close to the edge of the viewport, the container will scroll to make sure the next item is visible. 

Previous PR for context: flutter/engine#55964

https://github.com/user-attachments/assets/84ae5153-f955-4d23-9901-ce942c0e98ac

### Why the UIScrollView subclass in the focus hierarchy

The iOS focus system does not provide an API that allows apps to notify it of focus highlight changes. So if we were to keep using the transforms sent by the framework as-is and not introducing any UIViews in the focus hierarchy, the focus highlight will be positioned at the wrong location after scrolling (via FKA or via framework). That does not seem to be part of the public API and the focus system seems to only know how to properly highlight focusable UIViews.

### Things that currently may not work

1. Nested scroll views (have not tried to verify) 

The `UIScrollView`s are always subviews of the `FlutterView`. If there are nested scrollables the focus system may not be able to properly determine the focus hierarchy (in theory the iOS focus system should never depend on `UIView.parentView` but I haven't tried to verify that).

2. If the next item is too far below the bottom of the screen and there is a tab bar with focusable items, the focus will be transferred to tab bar instead of the next item in the list

Video demo (as you can see the scrolling is really finicky):

https://github.com/user-attachments/assets/51c2bfe4-d7b3-4614-aa49-4256214f8978

I've tried doing the same thing using a `UITableView` with similar configurations but it seems to have the same problem. I'll try to dig a bit deeper into this and see if there's a workaround.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
…er/engine#56802)

Reverts: flutter/engine#56606
Initiated by: LongCatIsLooong
Reason for reverting: flutter#159456
Original PR Author: LongCatIsLooong

Reviewed By: {chunhtai, cbracken}

This change reverts the following previous change:
This PR adds basic FKA scrolling support: when the iOS focus (the focus state is maintained separately from the framework focus, see the previous PR) switches to an item in a scrollable container that is too close to the edge of the viewport, the container will scroll to make sure the next item is visible. 

Previous PR for context: flutter/engine#55964

https://github.com/user-attachments/assets/84ae5153-f955-4d23-9901-ce942c0e98ac

### Why the UIScrollView subclass in the focus hierarchy

The iOS focus system does not provide an API that allows apps to notify it of focus highlight changes. So if we were to keep using the transforms sent by the framework as-is and not introducing any UIViews in the focus hierarchy, the focus highlight will be positioned at the wrong location after scrolling (via FKA or via framework). That does not seem to be part of the public API and the focus system seems to only know how to properly highlight focusable UIViews.

### Things that currently may not work

1. Nested scroll views (have not tried to verify) 

The `UIScrollView`s are always subviews of the `FlutterView`. If there are nested scrollables the focus system may not be able to properly determine the focus hierarchy (in theory the iOS focus system should never depend on `UIView.parentView` but I haven't tried to verify that).

2. If the next item is too far below the bottom of the screen and there is a tab bar with focusable items, the focus will be transferred to tab bar instead of the next item in the list

Video demo (as you can see the scrolling is really finicky):

https://github.com/user-attachments/assets/51c2bfe4-d7b3-4614-aa49-4256214f8978

I've tried doing the same thing using a `UITableView` with similar configurations but it seems to have the same problem. I'll try to dig a bit deeper into this and see if there's a workaround.

[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.

2 participants