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

iOS: A11y only disable during app resigning to background when voice over on #32820

Merged
merged 2 commits into from
Apr 29, 2022
Merged

iOS: A11y only disable during app resigning to background when voice over on #32820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cead1d6
only hide a11y when voice over is on
Apr 20, 2022
0fabf8a
fix test
Apr 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions shell/platform/darwin/ios/framework/Headers/FlutterViewController.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,14 @@ FLUTTER_DARWIN_EXPORT
*/
- (id<FlutterPluginRegistry>)pluginRegistry;

/**
* A wrapper around UIAccessibilityIsVoiceOverRunning().
*
* As a C function, UIAccessibilityIsVoiceOverRunning() cannot be mocked in testing. Mock
* this class method to testing features depends on UIAccessibilityIsVoiceOverRunning().
*/
+ (BOOL)isUIAccessibilityIsVoiceOverRunning;

/**
* True if at least one frame has rendered and the ViewController has appeared.
*
Expand Down
8 changes: 7 additions & 1 deletion shell/platform/darwin/ios/framework/Source/FlutterViewController.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,9 @@ - (void)applicationBecameActive:(NSNotification*)notification {

- (void)applicationWillResignActive:(NSNotification*)notification {
TRACE_EVENT0("flutter", "applicationWillResignActive");
self.view.accessibilityElementsHidden = YES;
if ([FlutterViewController isUIAccessibilityIsVoiceOverRunning]) {
self.view.accessibilityElementsHidden = YES;
Copy link
Contributor

Choose a reason for hiding this comment

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

What about when someone is using a physical switch device or voice input? Does this break things in those cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

/cc @chunhtai

Copy link
Contributor

Choose a reason for hiding this comment

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

this looks like it will break if voice control is turned on, unfortunately there isn't an API to detect whether voice control is on last time i checked

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we flip the sense of this and allow tests to pass some parameter/set some environment variable to override this behavior?

Copy link
Contributor Author

@cyanglaz cyanglaz Apr 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Do we need self.view.accessibilityElementsHidden = YES; if voice input is on? I remember self.view.accessibilityElementsHidden = YES; is to fix the exact issue where the voice over is on and the focus is jumped to flutter view.

Copy link
Contributor Author

@cyanglaz cyanglaz Apr 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

So in the current code, we always hide accessibility elements. Now, with the change in this PR, we only hide the elements when voice over is turned on. This would actually be an improvement for voice input (by not hiding a11y elements when voice over is off)

I couldn't find any concrete example for voice input to test tho.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chunhtai @dnfield Friendly ping. Did my comment make sense? I'm happy to schedule a quick VC to discuss this :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the concern is whether "showing" the a11y elements when a dialog is up will confuse voice input.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's probably fine, but it would be good to test an app that has voice input turned on while it shows a permissions dialog, and make sure you can use voice input there the same as you would be able to use it in a regular native iOS app in that situation. I'm not familiar enough with voice input to know how to test this myself.

If a manual test shows that it works ok, that's probably the best we can do. If not then we need to figure out how to change this only for tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested with voice control, it seems working fine. See below video:
https://drive.google.com/file/d/14DErLf2G_f_cfd6miU-DyJIhzMBWIW0L/view?usp=sharing

}
[self goToApplicationLifecycle:@"AppLifecycleState.inactive"];
}

Expand Down Expand Up @@ -1718,6 +1720,10 @@ - (NSString*)lookupKeyForAsset:(NSString*)asset fromPackage:(NSString*)package {
return _engine;
}

+ (BOOL)isUIAccessibilityIsVoiceOverRunning {
return UIAccessibilityIsVoiceOverRunning();
}

#pragma mark - FlutterPluginRegistry

- (NSObject<FlutterPluginRegistrar>*)registrarForPlugin:(NSString*)pluginKey {
Expand Down
29 changes: 29 additions & 0 deletions shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,9 @@ - (void)testHideA11yElements {
FlutterViewController* realVC = [[FlutterViewController alloc] initWithEngine:engine
nibName:nil
bundle:nil];
id flutterViewControllerClassMOCK = OCMClassMock([FlutterViewController class]);
[[[flutterViewControllerClassMOCK stub] andReturnValue:@YES] isUIAccessibilityIsVoiceOverRunning];

XCTAssertFalse(realVC.view.accessibilityElementsHidden);
[[NSNotificationCenter defaultCenter]
postNotificationName:UIApplicationWillResignActiveNotification
Expand All @@ -949,6 +952,32 @@ - (void)testHideA11yElements {
object:nil];
XCTAssertFalse(realVC.view.accessibilityElementsHidden);
engine.viewController = nil;

[flutterViewControllerClassMOCK stopMocking];
}

- (void)testDontHideA11yElementsWhenVoiceOverIsOff {
FlutterDartProject* project = [[FlutterDartProject alloc] init];
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:project];
[engine createShell:@"" libraryURI:@"" initialRoute:nil];
FlutterViewController* realVC = [[FlutterViewController alloc] initWithEngine:engine
nibName:nil
bundle:nil];
id flutterViewControllerClassMOCK = OCMClassMock([FlutterViewController class]);
[[[flutterViewControllerClassMOCK stub] andReturnValue:@NO] isUIAccessibilityIsVoiceOverRunning];

XCTAssertFalse(realVC.view.accessibilityElementsHidden);
[[NSNotificationCenter defaultCenter]
postNotificationName:UIApplicationWillResignActiveNotification
object:nil];
XCTAssertFalse(realVC.view.accessibilityElementsHidden);
[[NSNotificationCenter defaultCenter]
postNotificationName:UIApplicationDidBecomeActiveNotification
object:nil];
XCTAssertFalse(realVC.view.accessibilityElementsHidden);
engine.viewController = nil;

[flutterViewControllerClassMOCK stopMocking];
}

- (void)testNotifyLowMemory {
Expand Down