Skip to content

Commit

Permalink
Fixes ios accessibility send focus request to an unfocusable node (fl…
Browse files Browse the repository at this point in the history
  • Loading branch information
chunhtai committed May 3, 2021
1 parent 9cb935f commit 223fb2e
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 37 deletions.
13 changes: 6 additions & 7 deletions shell/platform/darwin/ios/framework/Source/SemanticsObject.mm
Original file line number Diff line number Diff line change
Expand Up @@ -274,17 +274,16 @@ - (BOOL)isAccessibilityElement {
if ([self node].HasFlag(flutter::SemanticsFlags::kScopesRoute))
return false;

// If the only flag(s) set are scrolling related AND
// The only flags set are not kIsHidden OR
// The node doesn't have a label, value, or hint OR
// The only actions set are scrolling related actions.
// If the node is scrollable AND hidden OR
// The node has a label, value, or hint OR
// The node has non-scrolling related actions.
//
// The kIsHidden flag set with any other flag just means this node is now
// The kIsHidden flag set with the scrollable flag means this node is now
// hidden but still is a valid target for a11y focus in the tree, e.g. a list
// item that is currently off screen but the a11y navigation needs to know
// about.
return (([self node].flags & ~flutter::kScrollableSemanticsFlags) != 0 &&
[self node].flags != static_cast<int32_t>(flutter::SemanticsFlags::kIsHidden)) ||
return (([self node].flags & flutter::kScrollableSemanticsFlags) != 0 &&
([self node].flags & static_cast<int32_t>(flutter::SemanticsFlags::kIsHidden)) != 0) ||
![self node].label.empty() || ![self node].value.empty() || ![self node].hint.empty() ||
([self node].actions & ~flutter::kScrollableSemanticsActions) != 0;
}
Expand Down
23 changes: 23 additions & 0 deletions shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,29 @@ - (void)testPlainSemanticsObjectWithLabelHasStaticTextTrait {
XCTAssertEqual([object accessibilityTraits], UIAccessibilityTraitStaticText);
}

- (void)testNodeWithImplicitScrollIsAnAccessibilityElementWhenItisHidden {
fml::WeakPtrFactory<flutter::AccessibilityBridgeIos> factory(
new flutter::MockAccessibilityBridge());
fml::WeakPtr<flutter::AccessibilityBridgeIos> bridge = factory.GetWeakPtr();
flutter::SemanticsNode node;
node.flags = static_cast<int32_t>(flutter::SemanticsFlags::kHasImplicitScrolling) |
static_cast<int32_t>(flutter::SemanticsFlags::kIsHidden);
FlutterSemanticsObject* object = [[FlutterSemanticsObject alloc] initWithBridge:bridge uid:0];
[object setSemanticsNode:&node];
XCTAssertEqual(object.isAccessibilityElement, YES);
}

- (void)testNodeWithImplicitScrollIsNotAnAccessibilityElementWhenItisNotHidden {
fml::WeakPtrFactory<flutter::AccessibilityBridgeIos> factory(
new flutter::MockAccessibilityBridge());
fml::WeakPtr<flutter::AccessibilityBridgeIos> bridge = factory.GetWeakPtr();
flutter::SemanticsNode node;
node.flags = static_cast<int32_t>(flutter::SemanticsFlags::kHasImplicitScrolling);
FlutterSemanticsObject* object = [[FlutterSemanticsObject alloc] initWithBridge:bridge uid:0];
[object setSemanticsNode:&node];
XCTAssertEqual(object.isAccessibilityElement, NO);
}

- (void)testIntresetingSemanticsObjectWithLabelHasStaticTextTrait {
fml::WeakPtrFactory<flutter::AccessibilityBridgeIos> factory(
new flutter::MockAccessibilityBridge());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,12 @@ class AccessibilityBridge final : public AccessibilityBridgeIos {

private:
SemanticsObject* GetOrCreateObject(int32_t id, flutter::SemanticsNodeUpdates& updates);
SemanticsObject* FindFirstFocusable(SemanticsObject* object);
SemanticsObject* FindNextFocusableIfNecessary();
// Finds the first focusable SemanticsObject rooted at the parent. This includes the parent itself
// if it is a focusable SemanticsObject.
//
// If the parent is nil, this function use the root SemanticsObject as the parent.
SemanticsObject* FindFirstFocusable(SemanticsObject* parent);
void VisitObjectsRecursivelyAndRemove(SemanticsObject* object,
NSMutableArray<NSNumber*>* doomed_uids);
void HandleEvent(NSDictionary<NSString*, id>* annotatedEvent);
Expand Down
52 changes: 25 additions & 27 deletions shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -216,31 +216,14 @@ void PostAccessibilityNotification(UIAccessibilityNotifications notification,
}

if (layoutChanged) {
SemanticsObject* nextToFocus = nil;
// This property will be -1 if the focus is outside of the flutter
// application. In this case, we should not refocus anything.
if (last_focused_semantics_object_id_ != kSemanticObjectIdInvalid) {
// Tries to refocus the previous focused semantics object to avoid random jumps.
nextToFocus = [objects_.get() objectForKey:@(last_focused_semantics_object_id_)];
if (!nextToFocus && root) {
nextToFocus = FindFirstFocusable(root);
}
}
ios_delegate_->PostAccessibilityNotification(UIAccessibilityLayoutChangedNotification,
nextToFocus);
FindNextFocusableIfNecessary());
} else if (scrollOccured) {
// TODO(chunhtai): figure out what string to use for notification. At this
// point, it is guarantee the previous focused object is still in the tree
// so that we don't need to worry about focus lost. (e.g. "Screen 0 of 3")
SemanticsObject* nextToFocus = nil;
if (last_focused_semantics_object_id_ != kSemanticObjectIdInvalid) {
nextToFocus = [objects_.get() objectForKey:@(last_focused_semantics_object_id_)];
if (!nextToFocus && root) {
nextToFocus = FindFirstFocusable(root);
}
}
ios_delegate_->PostAccessibilityNotification(UIAccessibilityPageScrolledNotification,
nextToFocus);
FindNextFocusableIfNecessary());
}
}

Expand Down Expand Up @@ -324,19 +307,34 @@ static bool DidFlagChange(const flutter::SemanticsNode& oldNode,
VisitObjectsRecursivelyAndRemove(child, doomed_uids);
}

SemanticsObject* AccessibilityBridge::FindFirstFocusable(SemanticsObject* object) {
if (object.isAccessibilityElement) {
return object;
SemanticsObject* AccessibilityBridge::FindNextFocusableIfNecessary() {
// This property will be -1 if the focus is outside of the flutter
// application. In this case, we should not refocus anything.
if (last_focused_semantics_object_id_ == kSemanticObjectIdInvalid) {
return nil;
}

// Tries to refocus the previous focused semantics object to avoid random jumps.
return FindFirstFocusable([objects_.get() objectForKey:@(last_focused_semantics_object_id_)]);
}

SemanticsObject* AccessibilityBridge::FindFirstFocusable(SemanticsObject* parent) {
SemanticsObject* currentObject = parent ?: objects_.get()[@(kRootNodeId)];
;
if (!currentObject)
return nil;

if (currentObject.isAccessibilityElement) {
return currentObject;
}

SemanticsObject* candidate = nil;
for (SemanticsObject* child in [object children]) {
for (SemanticsObject* child in [currentObject children]) {
SemanticsObject* candidate = FindFirstFocusable(child);
if (candidate) {
break;
return candidate;
}
candidate = FindFirstFocusable(child);
}
return candidate;
return nil;
}

void AccessibilityBridge::HandleEvent(NSDictionary<NSString*, id>* annotatedEvent) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,88 @@ - (void)testAnnouncesRouteChanges {
UIAccessibilityScreenChangedNotification);
}

- (void)testLayoutChangeWithNonAccessibilityElement {
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));

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

flutter::SemanticsNode node1;
node1.id = 1;
node1.label = "node1";
node1.childrenInTraversalOrder = {2, 3};
node1.childrenInHitTestOrder = {2, 3};
nodes[node1.id] = node1;
flutter::SemanticsNode node2;
node2.id = 2;
node2.label = "node2";
nodes[node2.id] = node2;
flutter::SemanticsNode node3;
node3.id = 3;
node3.label = "node3";
nodes[node3.id] = node3;
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);

// Simulates the focusing on the node 1.
bridge->AccessibilityObjectDidBecomeFocused(1);

// In this update, we make node 1 unfocusable and trigger the
// layout change. The accessibility bridge should send layoutchange
// notification with the first focusable node under node 1
flutter::CustomAccessibilityActionUpdates new_actions;
flutter::SemanticsNodeUpdates new_nodes;

flutter::SemanticsNode new_node1;
new_node1.id = 1;
new_node1.childrenInTraversalOrder = {2};
new_node1.childrenInHitTestOrder = {2};
new_nodes[new_node1.id] = new_node1;
bridge->UpdateSemantics(/*nodes=*/new_nodes, /*actions=*/new_actions);

XCTAssertEqual([accessibility_notifications count], 1ul);
SemanticsObject* focusObject = accessibility_notifications[0][@"argument"];
// Since node 1 is no longer focusable (no label), it will focus node 2 instead.
XCTAssertEqual([focusObject uid], 2);
XCTAssertEqual([accessibility_notifications[0][@"notification"] unsignedIntValue],
UIAccessibilityLayoutChangedNotification);
}

- (void)testAnnouncesRouteChangesAndLayoutChangeInOneUpdate {
flutter::MockDelegate mock_delegate;
auto thread_task_runner = CreateNewThread("AccessibilityBridgeTest");
Expand Down Expand Up @@ -392,7 +474,7 @@ - (void)testAnnouncesRouteChangesAndLayoutChangeInOneUpdate {
nodes[node3.id] = node3;
flutter::SemanticsNode root_node;
root_node.id = kRootNodeId;
root_node.flags = static_cast<int32_t>(flutter::SemanticsFlags::kScopesRoute);
root_node.label = "root";
root_node.childrenInTraversalOrder = {1, 3};
root_node.childrenInHitTestOrder = {1, 3};
nodes[root_node.id] = root_node;
Expand Down Expand Up @@ -424,7 +506,7 @@ - (void)testAnnouncesRouteChangesAndLayoutChangeInOneUpdate {
new_nodes[new_node2.id] = new_node2;
flutter::SemanticsNode new_root_node;
new_root_node.id = kRootNodeId;
new_root_node.flags = static_cast<int32_t>(flutter::SemanticsFlags::kScopesRoute);
new_root_node.label = "root";
new_root_node.childrenInTraversalOrder = {1};
new_root_node.childrenInHitTestOrder = {1};
new_nodes[new_root_node.id] = new_root_node;
Expand Down

0 comments on commit 223fb2e

Please sign in to comment.