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

iOS accessibility ignore layout change if app is not active #32511

Closed
wants to merge 1 commit into from
Closed

iOS accessibility ignore layout change if app is not active #32511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 0 additions & 2 deletions 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 @@ -817,7 +817,6 @@ - (void)dealloc {

- (void)applicationBecameActive:(NSNotification*)notification {
TRACE_EVENT0("flutter", "applicationBecameActive");
self.view.accessibilityElementsHidden = NO;
Copy link
Contributor

Choose a reason for hiding this comment

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

I am confused at the change, it seems like the a11y was hidden when the view is not active prior to this change. and this change makes it so it does not hide the a11y when the view is not active?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code was a workaround to fix the same issue that this PR is trying to fix. Since we found the root cause and potentially a fix with this PR; we can remove the old workaround.
The workaround was introduced in 0be0303

if (_viewportMetrics.physical_width) {
[self surfaceUpdated:YES];
}
Expand All @@ -826,7 +825,6 @@ - (void)applicationBecameActive:(NSNotification*)notification {

- (void)applicationWillResignActive:(NSNotification*)notification {
TRACE_EVENT0("flutter", "applicationWillResignActive");
self.view.accessibilityElementsHidden = YES;
[self goToApplicationLifecycle:@"AppLifecycleState.inactive"];
}

Expand Down
4 changes: 2 additions & 2 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 @@ -932,7 +932,7 @@ - (void)testHideOverlay {
engine.viewController = nil;
}

- (void)testHideA11yElements {
- (void)testDoNotHideA11yElements {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should just rip out this test now that we are undoing the work that caused it.

FlutterDartProject* project = [[FlutterDartProject alloc] init];
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:project];
[engine createShell:@"" libraryURI:@"" initialRoute:nil];
Expand All @@ -943,7 +943,7 @@ - (void)testHideA11yElements {
[[NSNotificationCenter defaultCenter]
postNotificationName:UIApplicationWillResignActiveNotification
object:nil];
XCTAssertTrue(realVC.view.accessibilityElementsHidden);
XCTAssertFalse(realVC.view.accessibilityElementsHidden);
[[NSNotificationCenter defaultCenter]
postNotificationName:UIApplicationDidBecomeActiveNotification
object:nil];
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ void PostAccessibilityNotification(UIAccessibilityNotifications notification,
for (SemanticsObject* object in [objects_ allValues]) {
[object accessibilityBridgeDidFinishUpdate];
}

if (!ios_delegate_->IsFlutterViewControllerPresentingModalViewController(view_controller_)) {
if (!ios_delegate_->IsFlutterViewControllerPresentingModalViewController(view_controller_) &&
[UIApplication sharedApplication].applicationState == UIApplicationStateActive) {
Copy link
Contributor Author

@cyanglaz cyanglaz Apr 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I had a typo here that made it "worked". Looks like with a popup on the screen, the state is still active, so [UIApplication sharedApplication].applicationState should not be used as an indicator here. Will need to find another way to do it. Setting this PR to draft until I fix it

layoutChanged = layoutChanged || [doomed_uids count] > 0;

if (routeChanged) {
Expand Down
61 changes: 61 additions & 0 deletions shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1618,6 +1618,67 @@ - (void)testAnnouncesIgnoresLayoutChangeWhenModal {
XCTAssertEqual([accessibility_notifications count], 0ul);
}

- (void)testAnnouncesIgnoresLayoutChangeWhenApplicationIsInactive {
flutter::MockDelegate mock_delegate;
auto thread_task_runner = CreateNewThread("AccessibilityBridgeTest");
flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
/*platform=*/thread_task_runner,
/*raster=*/thread_task_runner,
/*ui=*/thread_task_runner,
/*io=*/thread_task_runner);
auto platform_view = std::make_unique<flutter::PlatformViewIOS>(
/*delegate=*/mock_delegate,
/*rendering_api=*/flutter::IOSRenderingAPI::kSoftware,
/*platform_views_controller=*/nil,
/*task_runners=*/runners);
id mockFlutterView = OCMClassMock([FlutterView class]);
id mockFlutterViewController = OCMClassMock([FlutterViewController class]);
OCMStub([mockFlutterViewController view]).andReturn(mockFlutterView);

NSMutableArray<NSDictionary<NSString*, id>*>* accessibility_notifications =
[[[NSMutableArray alloc] init] autorelease];
auto ios_delegate = std::make_unique<flutter::MockIosDelegate>();
ios_delegate->on_PostAccessibilityNotification_ =
[accessibility_notifications](UIAccessibilityNotifications notification, id argument) {
[accessibility_notifications addObject:@{
@"notification" : @(notification),
@"argument" : argument ? argument : [NSNull null],
}];
};
__block auto bridge =
std::make_unique<flutter::AccessibilityBridge>(/*view_controller=*/mockFlutterViewController,
/*platform_view=*/platform_view.get(),
/*platform_views_controller=*/nil,
/*ios_delegate=*/std::move(ios_delegate));
id mockApplication = OCMPartialMock([UIApplication sharedApplication]);
OCMStub([mockApplication applicationState]).andReturn(UIApplicationStateInactive);

flutter::CustomAccessibilityActionUpdates actions;
flutter::SemanticsNodeUpdates nodes;

flutter::SemanticsNode child_node;
child_node.id = 1;
child_node.label = "child_node";
nodes[child_node.id] = child_node;
flutter::SemanticsNode root_node;
root_node.id = kRootNodeId;
root_node.label = "root";
root_node.childrenInTraversalOrder = {1};
root_node.childrenInHitTestOrder = {1};
nodes[root_node.id] = root_node;
bridge->UpdateSemantics(/*nodes=*/nodes, /*actions=*/actions);

// Removes child_node to simulate a layout change.
flutter::SemanticsNodeUpdates new_nodes;
flutter::SemanticsNode new_root_node;
new_root_node.id = kRootNodeId;
new_root_node.label = "root";
new_nodes[new_root_node.id] = new_root_node;
bridge->UpdateSemantics(/*nodes=*/new_nodes, /*actions=*/actions);

XCTAssertEqual([accessibility_notifications count], 0ul);
}

- (void)testAnnouncesIgnoresScrollChangeWhenModal {
flutter::MockDelegate mock_delegate;
auto thread_task_runner = CreateNewThread("AccessibilityBridgeTest");
Expand Down