Fix multiple issues around accessibility handlinig

Source/Details/_ASDisplayViewAccessiblity.mm

@@ -13,15 +13,13 @@
 #import <AsyncDisplayKit/ASAvailability.h>
 #import <AsyncDisplayKit/ASCollectionNode.h>
 #import <AsyncDisplayKit/ASDisplayNodeExtras.h>
+#import <AsyncDisplayKit/ASDisplayNode+FrameworkPrivate.h>
+#import <AsyncDisplayKit/ASDisplayNode+Ancestry.h>
 #import <AsyncDisplayKit/ASDisplayNodeInternal.h>
 #import <AsyncDisplayKit/ASTableNode.h>
 
 #import <queue>
 
-NS_INLINE UIAccessibilityTraits InteractiveAccessibilityTraitsMask() {
-  return UIAccessibilityTraitLink | UIAccessibilityTraitKeyboardKey | UIAccessibilityTraitButton;
-}
-
 #pragma mark - UIAccessibilityElement
 
 @protocol ASAccessibilityElementPositioning
@@ -36,7 +34,7 @@ @protocol ASAccessibilityElementPositioning
 static void SortAccessibilityElements(NSMutableArray *elements)
 {
   ASDisplayNodeCAssertNotNil(elements, @"Should pass in a NSMutableArray");
-  
+
   static SortAccessibilityElementsComparator comparator = nil;
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
@@ -55,22 +53,25 @@ static void SortAccessibilityElements(NSMutableArray *elements)
   [elements sortUsingComparator:comparator];
 }
 
+static CGRect ASAccessibilityFrameForNode(ASDisplayNode *node) {
+  CALayer *layer = node.layer;
+  return [layer convertRect:node.bounds toLayer:ASFindWindowOfLayer(layer).layer];
+}
+
 @interface ASAccessibilityElement : UIAccessibilityElement<ASAccessibilityElementPositioning>
 
 @property (nonatomic) ASDisplayNode *node;
-@property (nonatomic) ASDisplayNode *containerNode;
 
-+ (ASAccessibilityElement *)accessibilityElementWithContainer:(UIView *)container node:(ASDisplayNode *)node containerNode:(ASDisplayNode *)containerNode;
++ (ASAccessibilityElement *)accessibilityElementWithContainer:(UIView *)container node:(ASDisplayNode *)node;
 
 @end
 
 @implementation ASAccessibilityElement
 
-+ (ASAccessibilityElement *)accessibilityElementWithContainer:(UIView *)container node:(ASDisplayNode *)node containerNode:(ASDisplayNode *)containerNode
++ (ASAccessibilityElement *)accessibilityElementWithContainer:(UIView *)container node:(ASDisplayNode *)node
 {
   ASAccessibilityElement *accessibilityElement = [[ASAccessibilityElement alloc] initWithAccessibilityContainer:container];
   accessibilityElement.node = node;
-  accessibilityElement.containerNode = containerNode;
   accessibilityElement.accessibilityIdentifier = node.accessibilityIdentifier;
   accessibilityElement.accessibilityLabel = node.accessibilityLabel;
   accessibilityElement.accessibilityHint = node.accessibilityHint;
@@ -86,8 +87,7 @@ + (ASAccessibilityElement *)accessibilityElementWithContainer:(UIView *)containe
 
 - (CGRect)accessibilityFrame
 {
-  CGRect accessibilityFrame = [self.containerNode convertRect:self.node.bounds fromNode:self.node];
-  return UIAccessibilityConvertFrameToScreenCoordinates(accessibilityFrame, self.accessibilityContainer);
+  return ASAccessibilityFrameForNode(self.node);
 }
 
 @end
@@ -96,18 +96,15 @@ - (CGRect)accessibilityFrame
 
 @interface ASAccessibilityCustomAction : UIAccessibilityCustomAction<ASAccessibilityElementPositioning>
 
-@property (nonatomic) UIView *container;
 @property (nonatomic) ASDisplayNode *node;
-@property (nonatomic) ASDisplayNode *containerNode;
 
 @end
 
 @implementation ASAccessibilityCustomAction
 
 - (CGRect)accessibilityFrame
 {
-  CGRect accessibilityFrame = [self.containerNode convertRect:self.node.bounds fromNode:self.node];
-  return UIAccessibilityConvertFrameToScreenCoordinates(accessibilityFrame, self.container);
+  return ASAccessibilityFrameForNode(self.node);
 }
 
 @end
@@ -116,19 +113,26 @@ - (CGRect)accessibilityFrame
 static void CollectUIAccessibilityElementsForNode(ASDisplayNode *node, ASDisplayNode *containerNode, id container, NSMutableArray *elements)
 {
   ASDisplayNodeCAssertNotNil(elements, @"Should pass in a NSMutableArray");
-  
+
   ASDisplayNodePerformBlockOnEveryNodeBFS(node, ^(ASDisplayNode * _Nonnull currentNode) {
     // For every subnode that is layer backed or it's supernode has subtree rasterization enabled
     // we have to create a UIAccessibilityElement as no view for this node exists
     if (currentNode != containerNode && currentNode.isAccessibilityElement) {
-      UIAccessibilityElement *accessibilityElement = [ASAccessibilityElement accessibilityElementWithContainer:container node:currentNode containerNode:containerNode];
+      UIAccessibilityElement *accessibilityElement = [ASAccessibilityElement accessibilityElementWithContainer:container node:currentNode];
       [elements addObject:accessibilityElement];
     }
   });
 }
 
-static void CollectAccessibilityElementsForContainer(ASDisplayNode *container, UIView *view, NSMutableArray *elements) {
-  UIAccessibilityElement *accessiblityElement = [ASAccessibilityElement accessibilityElementWithContainer:view node:container containerNode:container];
+static void CollectAccessibilityElementsForContainer(ASDisplayNode *container, UIView *view,
+                                                     NSMutableArray *elements) {
+  ASDisplayNodeCAssertNotNil(view, @"Passed in view should not be nil");
+  if (view == nil) {
+    return;
+  }
+  UIAccessibilityElement *accessiblityElement =
+      [ASAccessibilityElement accessibilityElementWithContainer:view
+                                                           node:container];
 
   NSMutableArray<ASAccessibilityElement *> *labeledNodes = [[NSMutableArray alloc] init];
   NSMutableArray<ASAccessibilityCustomAction *> *actions = [[NSMutableArray alloc] init];
@@ -140,28 +144,28 @@ static void CollectAccessibilityElementsForContainer(ASDisplayNode *container, U
   // value and do not perform the aggregation.
   BOOL shouldAggregateSubnodeLabels =
       (container.accessibilityLabel.length == 0) ||
-      (container.accessibilityTraits & InteractiveAccessibilityTraitsMask());
+      (container.accessibilityTraits & ASInteractiveAccessibilityTraitsMask());
 
   ASDisplayNode *node = nil;
   while (!queue.empty()) {
     node = queue.front();
     queue.pop();
 
     if (node != container && node.isAccessibilityContainer) {
-      CollectAccessibilityElementsForContainer(node, view, elements);
+      UIView *containerView = node.isLayerBacked ? view : node.view;
+      CollectAccessibilityElementsForContainer(node, containerView, elements);
       continue;
     }
 
     if (node.accessibilityLabel.length > 0) {
-      if (node.accessibilityTraits & InteractiveAccessibilityTraitsMask()) {
+      if (node.accessibilityTraits & ASInteractiveAccessibilityTraitsMask()) {
         ASAccessibilityCustomAction *action = [[ASAccessibilityCustomAction alloc] initWithName:node.accessibilityLabel target:node selector:@selector(performAccessibilityCustomAction:)];
         action.node = node;
-        action.containerNode = node.supernode;
-        action.container = node.supernode.view;
         [actions addObject:action];
+
+        node.accessibilityCustomAction = action;
       } else if (node == container || shouldAggregateSubnodeLabels) {
-        // Even though not surfaced to UIKit, create a non-interactive element for purposes of building sorted aggregated label.
-        ASAccessibilityElement *nonInteractiveElement = [ASAccessibilityElement accessibilityElementWithContainer:view node:node containerNode:container];
+        ASAccessibilityElement *nonInteractiveElement = [ASAccessibilityElement accessibilityElementWithContainer:view node:node];
         [labeledNodes addObject:nonInteractiveElement];
       }
     }
@@ -195,36 +199,39 @@ static void CollectAccessibilityElementsForContainer(ASDisplayNode *container, U
 }
 
 /// Collect all accessibliity elements for a given view and view node
-static void CollectAccessibilityElementsForView(UIView *view, NSMutableArray *elements)
+static void CollectAccessibilityElements(ASDisplayNode *node, NSMutableArray *elements)
 {
   ASDisplayNodeCAssertNotNil(elements, @"Should pass in a NSMutableArray");
-
-  ASDisplayNode *node = view.asyncdisplaykit_node;
+  ASDisplayNodeCAssertFalse(node.isLayerBacked);
+  if (node.isLayerBacked) {
+    return;
+  }
 
   BOOL anySubNodeIsCollection = (nil != ASDisplayNodeFindFirstNode(node,
       ^BOOL(ASDisplayNode *nodeToCheck) {
     return ASDynamicCast(nodeToCheck, ASCollectionNode) != nil ||
            ASDynamicCast(nodeToCheck, ASTableNode) != nil;
   }));
 
+  UIView *view = node.view;
+
   if (node.isAccessibilityContainer && !anySubNodeIsCollection) {
     CollectAccessibilityElementsForContainer(node, view, elements);
     return;
   }
-  
+
   // Handle rasterize case
   if (node.rasterizesSubtree) {
     CollectUIAccessibilityElementsForNode(node, node, view, elements);
     return;
   }
-  
+
   for (ASDisplayNode *subnode in node.subnodes) {
     if (subnode.isAccessibilityElement) {
-
       // An accessiblityElement can either be a UIView or a UIAccessibilityElement
       if (subnode.isLayerBacked) {
         // No view for layer backed nodes exist. It's necessary to create a UIAccessibilityElement that represents this node
-        UIAccessibilityElement *accessiblityElement = [ASAccessibilityElement accessibilityElementWithContainer:view node:subnode containerNode:node];
+        UIAccessibilityElement *accessiblityElement = [ASAccessibilityElement accessibilityElementWithContainer:view node:subnode];
         [elements addObject:accessiblityElement];
       } else {
         // Accessiblity element is not layer backed just add the view as accessibility element
@@ -233,7 +240,7 @@ static void CollectAccessibilityElementsForView(UIView *view, NSMutableArray *el
     } else if (subnode.isLayerBacked) {
       // Go down the hierarchy of the layer backed subnode and collect all of the UIAccessibilityElement
       CollectUIAccessibilityElementsForNode(subnode, node, view, elements);
-    } else if ([subnode accessibilityElementCount] > 0) {
+    } else if (subnode.accessibilityElementCount > 0) {
       // UIView is itself a UIAccessibilityContainer just add it
       [elements addObject:subnode.view];
     }
@@ -253,13 +260,13 @@ @implementation _ASDisplayView (UIAccessibilityContainer)
 - (void)setAccessibilityElements:(NSArray *)accessibilityElements
 {
   ASDisplayNodeAssertMainThread();
-  _accessibilityElements = accessibilityElements;
+  _accessibilityElements = nil;
 }
 
 - (NSArray *)accessibilityElements
 {
   ASDisplayNodeAssertMainThread();
-  
+
   ASDisplayNode *viewNode = self.asyncdisplaykit_node;
   if (viewNode == nil) {
     return @[];
@@ -282,7 +289,7 @@ - (NSArray *)accessibilityElements
     return @[];
   }
   NSMutableArray *accessibilityElements = [[NSMutableArray alloc] init];
-  CollectAccessibilityElementsForView(self.view, accessibilityElements);
+  CollectAccessibilityElements(self, accessibilityElements);
   SortAccessibilityElements(accessibilityElements);
   return accessibilityElements;
 }

Source/Private/ASDisplayNode+FrameworkPrivate.h

@@ -154,6 +154,11 @@ __unused static NSString * _Nonnull NSStringFromASHierarchyStateChange(ASHierarc
  */
 @property (nonatomic) ASHierarchyState hierarchyState;
 
+/**
+ * Represent the current custom action in representation for the node
+ */
+@property (nonatomic, weak) UIAccessibilityCustomAction *accessibilityCustomAction;
+
 /**
  * @abstract Return if the node is range managed or not
  *
@@ -307,6 +312,14 @@ __unused static NSString * _Nonnull NSStringFromASHierarchyStateChange(ASHierarc
 
 @end
 
+/**
+ * Defines interactive accessibility traits which will be exposed as UIAccessibilityCustomActions
+ * for nodes within nodes that have isAccessibilityContainer is YES
+ */
+NS_INLINE UIAccessibilityTraits ASInteractiveAccessibilityTraitsMask() {
+  return UIAccessibilityTraitLink | UIAccessibilityTraitKeyboardKey | UIAccessibilityTraitButton;
+}
+
 @interface ASDisplayNode (AccessibilityInternal)
 - (NSArray *)accessibilityElements;
 @end;

Source/Private/ASDisplayNode+UIViewBridge.mm

@@ -7,6 +7,7 @@
 //  Licensed under Apache 2.0: http://www.apache.org/licenses/LICENSE-2.0
 //
 
+#import <AsyncDisplayKit/ASDisplayNode+FrameworkPrivate.h>
 #import <AsyncDisplayKit/_ASCoreAnimationExtras.h>
 #import <AsyncDisplayKit/_ASPendingState.h>
 #import <AsyncDisplayKit/ASInternalHelpers.h>
@@ -1100,13 +1101,25 @@ - (NSString *)accessibilityLabel
 - (void)setAccessibilityLabel:(NSString *)accessibilityLabel
 {
   _bridge_prologue_write;
+  NSString *oldAccessibilityLabel = _getFromViewOnly(accessibilityLabel);
   _setAccessibilityToViewAndProperty(_accessibilityLabel, accessibilityLabel, accessibilityLabel, accessibilityLabel);
   if (AS_AVAILABLE_IOS_TVOS(11, 11)) {
     NSAttributedString *accessibilityAttributedLabel = accessibilityLabel ? [[NSAttributedString alloc] initWithString:accessibilityLabel] : nil;
     _setAccessibilityToViewAndProperty(_accessibilityAttributedLabel, accessibilityAttributedLabel, accessibilityAttributedLabel, accessibilityAttributedLabel);
   }
+
+  // We need to update action name when it's changed to reflect the latest state.
+  // Note: Update the custom action itself won't work when a11y is inside a list of custom actions
+  // in which one action results in a name change in the next action. In that case the UIAccessibility
+  // will hold the old action strongly until a11y jumps out of the list of custom actions.
+  // Thus we can only update name in place to have the change take effect.
+  BOOL needsUpdateActionName = self.isNodeLoaded && ![oldAccessibilityLabel isEqualToString:accessibilityLabel] && (0 != (_accessibilityTraits & ASInteractiveAccessibilityTraitsMask()));
+  if (needsUpdateActionName) {
+    self.accessibilityCustomAction.name = accessibilityLabel;
+  }
 }
 
+
 - (NSAttributedString *)accessibilityAttributedLabel
 {
   _bridge_prologue_read;

Tests/ASDisplayViewAccessibilityTests.mm

@@ -18,6 +18,7 @@
 #import <AsyncDisplayKit/ASConfiguration.h>
 #import <AsyncDisplayKit/ASConfigurationInternal.h>
 #import <OCMock/OCMock.h>
+#import "ASDisplayNodeTestsHelper.h"
 
 @interface ASDisplayViewAccessibilityTests : XCTestCase
 @end
@@ -88,38 +89,74 @@ - (void)testThatContainerAccessibilityLabelOverrideStopsAggregation
                         [node.view.accessibilityElements.firstObject accessibilityLabel]);
 }
 
+- (void)testAccessibilityLayerBackedContainerWithinAccessibilityContainer
+{
+  ASDisplayNode *container = [[ASDisplayNode alloc] init];
+  container.frame = CGRectMake(50, 50, 200, 600);
+  container.isAccessibilityContainer = YES;
+
+  ASDisplayNode *subContainer = [[ASDisplayNode alloc] init];
+  subContainer.frame = CGRectMake(50, 50, 200, 600);
+
+  subContainer.layerBacked = YES;
+  subContainer.isAccessibilityContainer = YES;
+  [container addSubnode:subContainer];
+
+  ASTextNode *text1 = [[ASTextNode alloc] init];
+  text1.attributedText = [[NSAttributedString alloc] initWithString:@"hello"];
+  text1.frame = CGRectMake(50, 100, 200, 200);
+  text1.layerBacked = YES;
+  [subContainer addSubnode:text1];
+
+  ASTextNode *text2 = [[ASTextNode alloc] init];
+  text2.attributedText = [[NSAttributedString alloc] initWithString:@"world"];
+  text2.frame = CGRectMake(50, 300, 200, 200);
+  text2.layerBacked = YES;
+  [subContainer addSubnode:text2];
+
+  NSArray<UIAccessibilityElement *> *accessibilityElements = container.view.accessibilityElements;
+  XCTAssertEqual(accessibilityElements.count, 2);
+  XCTAssertEqualObjects(accessibilityElements[1].accessibilityLabel, @"hello, world");
+}
+
 - (void)testAccessibilityNonLayerbackedNodesOperationInNonContainer
 {
-  ASDisplayNode *contianer = [[ASDisplayNode alloc] init];
-  contianer.frame = CGRectMake(50, 50, 200, 600);
-  contianer.backgroundColor = [UIColor grayColor];
+  ASDisplayNode *container = [[ASDisplayNode alloc] init];
+  UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 560)];
+  [window addSubnode:container];
+  [window makeKeyAndVisible];
+
+  container.frame = CGRectMake(50, 50, 200, 600);
+  container.backgroundColor = [UIColor grayColor];
   // Do any additional setup after loading the view, typically from a nib.
   ASTextNode *text1 = [[ASTextNode alloc] init];
   text1.attributedText = [[NSAttributedString alloc] initWithString:@"hello"];
   text1.frame = CGRectMake(50, 100, 200, 200);
-  [contianer addSubnode:text1];
-  [contianer layoutIfNeeded];
-  [contianer.layer displayIfNeeded];
-  NSArray<UIAccessibilityElement *> *elements = contianer.view.accessibilityElements;
+  [container addSubnode:text1];
+  [container layoutIfNeeded];
+  [container.layer displayIfNeeded];
+  NSArray<UIAccessibilityElement *> *elements = container.view.accessibilityElements;
   XCTAssertTrue(elements.count == 1);
   XCTAssertTrue([[elements.firstObject accessibilityLabel] isEqualToString:@"hello"]);
   ASTextNode *text2 = [[ASTextNode alloc] init];
   text2.attributedText = [[NSAttributedString alloc] initWithString:@"world"];
   text2.frame = CGRectMake(50, 300, 200, 200);
-  [contianer addSubnode:text2];
-  [contianer layoutIfNeeded];
-  [contianer.layer displayIfNeeded];
-  NSArray<UIAccessibilityElement *> *updatedElements = contianer.view.accessibilityElements;
+  [container addSubnode:text2];
+  [container layoutIfNeeded];
+  [container.layer displayIfNeeded];
+   ASCATransactionQueueWait(nil);
+  NSArray<UIAccessibilityElement *> *updatedElements = container.view.accessibilityElements;
   XCTAssertTrue(updatedElements.count == 2);
   XCTAssertTrue([[updatedElements.firstObject accessibilityLabel] isEqualToString:@"hello"]);
   XCTAssertTrue([[updatedElements.lastObject accessibilityLabel] isEqualToString:@"world"]);
   ASTextNode *text3 = [[ASTextNode alloc] init];
   text3.attributedText = [[NSAttributedString alloc] initWithString:@"!!!!"];
   text3.frame = CGRectMake(50, 400, 200, 100);
   [text2 addSubnode:text3];
-  [contianer layoutIfNeeded];
-  [contianer.layer displayIfNeeded];
-  NSArray<UIAccessibilityElement *> *updatedElements2 = contianer.view.accessibilityElements;
+  [container layoutIfNeeded];
+  [container.layer displayIfNeeded];
+  ASCATransactionQueueWait(nil);
+  NSArray<UIAccessibilityElement *> *updatedElements2 = container.view.accessibilityElements;
   //text3 won't be read out cause it's overshadowed by text2
   XCTAssertTrue(updatedElements2.count == 2);
   XCTAssertTrue([[updatedElements2.firstObject accessibilityLabel] isEqualToString:@"hello"]);