Fixes ios accessibility send focus request to an unfocusable node

shell/platform/darwin/ios/framework/Source/SemanticsObject.mm

@@ -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;
 }

shell/platform/darwin/ios/framework/Source/SemanticsObjectTest.mm

@@ -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());

shell/platform/darwin/ios/framework/Source/accessibility_bridge.h

@@ -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);

shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm

@@ -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());
   }
 }
 
@@ -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) {

shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm

@@ -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");
@@ -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;
@@ -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;