a11y improvements for iOS (#4079)

* Various iOS a11y fixes

* undo

* review comments

* review comments

lib/ui/semantics/semantics_node.h

@@ -27,6 +27,12 @@ enum class SemanticsAction : int32_t {
   kDecrease = 1 << 7,
 };
 
+const int kScrollableSemanticsActions =
+    static_cast<int32_t>(SemanticsAction::kScrollLeft) |
+    static_cast<int32_t>(SemanticsAction::kScrollRight) |
+    static_cast<int32_t>(SemanticsAction::kScrollUp) |
+    static_cast<int32_t>(SemanticsAction::kScrollDown);
+
 // Must match the SemanticsFlags enum in semantics.dart.
 enum class SemanticsFlags : int32_t {
   kHasCheckedState = 1 << 0,
@@ -45,7 +51,7 @@ struct SemanticsNode {
   int32_t flags = 0;
   int32_t actions = 0;
   std::string label;
-  int32_t textDirection = 0; // 0=unknown, 1=rtl, 2=ltr
+  int32_t textDirection = 0;  // 0=unknown, 1=rtl, 2=ltr
 
   SkRect rect = SkRect::MakeEmpty();
   SkMatrix44 transform = SkMatrix44(SkMatrix44::kIdentity_Constructor);

shell/platform/android/io/flutter/view/AccessibilityBridge.java

@@ -504,7 +504,7 @@ class AccessibilityBridge extends AccessibilityNodeProvider implements BasicMess
         }
 
         boolean isFocusable() {
-            return flags != 0 || label != null || (actions & ~SEMANTICS_ACTION_SCROLLABLE) != 0;
+            return flags != 0 || (label != null && !label.isEmpty()) || (actions & ~SEMANTICS_ACTION_SCROLLABLE) != 0;
         }
 
         void updateRecursively(float[] ancestorTransform, Set<SemanticsObject> visitedObjects, boolean forceUpdate) {

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

@@ -23,10 +23,12 @@ blink::SemanticsAction GetSemanticsActionForScrollDirection(
   // directions, which is why the following maps left to right and vice versa.
   switch (direction) {
     case UIAccessibilityScrollDirectionRight:
-    case UIAccessibilityScrollDirectionPrevious:  // TODO(abarth): Support RTL using _node.textDirection.
+    case UIAccessibilityScrollDirectionPrevious:  // TODO(abarth): Support RTL using
+                                                  // _node.textDirection.
       return blink::SemanticsAction::kScrollLeft;
     case UIAccessibilityScrollDirectionLeft:
-    case UIAccessibilityScrollDirectionNext:  // TODO(abarth): Support RTL using _node.textDirection.
+    case UIAccessibilityScrollDirectionNext:  // TODO(abarth): Support RTL using
+                                              // _node.textDirection.
       return blink::SemanticsAction::kScrollRight;
     case UIAccessibilityScrollDirectionUp:
       return blink::SemanticsAction::kScrollDown;
@@ -37,6 +39,16 @@ blink::SemanticsAction GetSemanticsActionForScrollDirection(
   return blink::SemanticsAction::kScrollUp;
 }
 
+bool GeometryComparator(SemanticsObject* a, SemanticsObject* b) {
+  // Should a go before b?
+  CGRect rectA = [a accessibilityFrame];
+  CGRect rectB = [b accessibilityFrame];
+  CGFloat top = rectA.origin.y - rectB.origin.y;
+  if (top == 0.0)
+    return rectA.origin.x - rectB.origin.x < 0.0;
+  return top < 0.0;
+}
+
 }  // namespace
 
 @implementation SemanticsObject {
@@ -92,18 +104,13 @@ blink::SemanticsAction GetSemanticsActionForScrollDirection(
   // Note: hit detection will only apply to elements that report
   // -isAccessibilityElement of YES. The framework will continue scanning the
   // entire element tree looking for such a hit.
-  return _node.HasAction(blink::SemanticsAction::kTap) || _children.empty();
+  return _node.flags != 0 || !_node.label.empty() ||
+         (_node.actions & ~blink::kScrollableSemanticsActions) != 0;
 }
 
 - (NSString*)accessibilityLabel {
-  if (_node.label.empty()) {
-    NSMutableString *label = [NSMutableString string];
-    for (auto& child : _children) {
-      [label appendString: [child accessibilityLabel]];
-      [label appendString: @"\n"];
-    }
-    return label;
-  }
+  if (_node.label.empty())
+    return nil;
   return @(_node.label.data());
 }
 
@@ -230,6 +237,10 @@ AccessibilityBridge::~AccessibilityBridge() {
 }
 
 void AccessibilityBridge::UpdateSemantics(std::vector<blink::SemanticsNode> nodes) {
+  // Children are received in paint order (inverse hit testing order). We need to bring them into
+  // traversal order (top left to bottom right, with hit testing order as tie breaker).
+  NSMutableSet<SemanticsObject*>* childOrdersToUpdate = [[[NSMutableSet alloc] init] autorelease];
+
   for (const blink::SemanticsNode& node : nodes) {
     SemanticsObject* object = GetOrCreateObject(node.id);
     [object setSemanticsNode:&node];
@@ -239,8 +250,19 @@ void AccessibilityBridge::UpdateSemantics(std::vector<blink::SemanticsNode> node
     for (size_t i = 0; i < childrenCount; ++i) {
       SemanticsObject* child = GetOrCreateObject(node.children[i]);
       child.parent = object;
-      children[i] = child;
+      // Reverting to get hit testing order (as tie breaker for sorting below).
+      children[childrenCount - i - 1] = child;
     }
+
+    [childOrdersToUpdate addObject:object];
+    if (object.parent)
+      [childOrdersToUpdate addObject:object.parent];
+  }
+
+  // Bring children into traversal order.
+  for (SemanticsObject* object in childOrdersToUpdate) {
+    std::vector<SemanticsObject*>* children = [object children];
+    std::stable_sort(children->begin(), children->end(), GeometryComparator);
   }
 
   SemanticsObject* root = objects_.get()[@(kRootNodeId)];
@@ -252,9 +274,8 @@ void AccessibilityBridge::UpdateSemantics(std::vector<blink::SemanticsNode> node
   } else {
     view_.accessibilityElements = nil;
   }
-    
-  NSMutableArray<NSNumber*>* doomed_uids =
-    [NSMutableArray arrayWithArray: [objects_.get() allKeys]];
+
+  NSMutableArray<NSNumber*>* doomed_uids = [NSMutableArray arrayWithArray:[objects_.get() allKeys]];
   if (root)
     VisitObjectsRecursivelyAndRemove(root, doomed_uids);
 
@@ -267,7 +288,7 @@ void AccessibilityBridge::UpdateSemantics(std::vector<blink::SemanticsNode> node
     }
   }
 
-  [objects_ removeObjectsForKeys: doomed_uids];
+  [objects_ removeObjectsForKeys:doomed_uids];
 
   if (focused_object_doomed) {
     // Previously focused element is no longer in the tree.
@@ -295,8 +316,8 @@ SemanticsObject* AccessibilityBridge::GetOrCreateObject(int32_t uid) {
 }
 
 void AccessibilityBridge::VisitObjectsRecursivelyAndRemove(SemanticsObject* object,
-                                                  NSMutableArray<NSNumber*>* doomed_uids) {
-  [doomed_uids removeObject: @(object.uid)];
+                                                           NSMutableArray<NSNumber*>* doomed_uids) {
+  [doomed_uids removeObject:@(object.uid)];
   for (SemanticsObject* child : *[object children])
     VisitObjectsRecursivelyAndRemove(child, doomed_uids);
 }