From 36d3fe023b3b9af170d400ff751c279df92bc623 Mon Sep 17 00:00:00 2001
From: ricky cancro <rickycancro@gmail.com>
Date: Tue, 5 May 2020 10:56:24 -0700
Subject: [PATCH 1/9] [ASDisplayViewAccessibility] A few accessibility
 improvements
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This diff includes a few improvements for accessibility in Texture.

* When determining a node’s accessibility elements, ignore any elements that are hidden, transparent, or out of the window. This matches UIKit’s behavior.
* When sorting the accessible elements and their origins are equal, give precedence to elements with shorter accessibility frames, followed by elements with narrower widths.
* Allow the ability to customize the comparator block that sorts the accessibility elements
* Create an experiment to stop caching accessibilityElements in `_ASDisplayView`. If we cache elements, we will require users to clear the cache (by calling `setAccessibilityElements` to get the side effect of clearing the cache) when nodes change their hidden state or opacity. This seems like a lot to request of the user. We will put this in an experiment so we can see the perf implication is both when using voice over and when not using voice over.

A few other notes:
* I got rid of the `ASAccessibilityElementPositioning` protocol in favor of passing `NSObjects` to the sort comparator. `NSObject` implements the informal `UIAccessibilityProtocol` and therefore has an `accessibilityFrame` property.
* I removed `static` from the `SortAccessibilityElements()` method definition. This allows me to declare it as `extern` in test it via unit tests.
---
 Source/ASExperimentalFeatures.h              |   1 +
 Source/ASExperimentalFeatures.mm             |   3 +-
 Source/Details/_ASDisplayViewAccessiblity.h  |  14 ++
 Source/Details/_ASDisplayViewAccessiblity.mm |  70 ++++++---
 Tests/ASDisplayViewAccessibilityTests.mm     | 154 +++++++++++++++++++
 5 files changed, 218 insertions(+), 24 deletions(-)

diff --git a/Source/ASExperimentalFeatures.h b/Source/ASExperimentalFeatures.h
index bec8614bb..82995b214 100644
--- a/Source/ASExperimentalFeatures.h
+++ b/Source/ASExperimentalFeatures.h
@@ -31,6 +31,7 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) {
   ASExperimentalDrawingGlobal = 1 << 10,                                    // exp_drawing_global
   ASExperimentalOptimizeDataControllerPipeline = 1 << 11,                   // exp_optimize_data_controller_pipeline
   ASExperimentalTraitCollectionDidChangeWithPreviousCollection = 1 << 12,   // exp_trait_collection_did_change_with_previous_collection
+  ASExperimentalDoNotCacheAccessibilityElements = 1 << 13,                  // exp_do_not_cache_accessibility_elements
   ASExperimentalFeatureAll = 0xFFFFFFFF
 };
 
diff --git a/Source/ASExperimentalFeatures.mm b/Source/ASExperimentalFeatures.mm
index eb9848292..f936525bc 100644
--- a/Source/ASExperimentalFeatures.mm
+++ b/Source/ASExperimentalFeatures.mm
@@ -24,7 +24,8 @@
                                       @"exp_oom_bg_dealloc_disable",
                                       @"exp_drawing_global",
                                       @"exp_optimize_data_controller_pipeline",
-                                      @"exp_trait_collection_did_change_with_previous_collection"]));
+                                      @"exp_trait_collection_did_change_with_previous_collection",
+                                      @"exp_do_not_cache_accessibility_elements"]));
   if (flags == ASExperimentalFeatureAll) {
     return allNames;
   }
diff --git a/Source/Details/_ASDisplayViewAccessiblity.h b/Source/Details/_ASDisplayViewAccessiblity.h
index 192b24d2c..33211cb71 100644
--- a/Source/Details/_ASDisplayViewAccessiblity.h
+++ b/Source/Details/_ASDisplayViewAccessiblity.h
@@ -14,3 +14,17 @@
 // should still work as long as accessibility is enabled, this framework provides no guarantees on
 // their correctness. For details, see
 // https://developer.apple.com/documentation/objectivec/nsobject/1615147-accessibilityelements
+
+// After recusively collecting all of the accessibility elements of a node, they get sorted. This sort determines
+// the order that a screen reader will traverse the elements. By default, we sort these elements based on their
+// origin: lower y origin comes first, then lower x origin. If 2 nodes have an equal origin, the node with the smaller
+// height is placed before the node with the smaller width. If two nodes have the exact same rect, we throw up our hands
+// and return NSOrderedSame.
+//
+// In general this seems to work fairly well. However, if you want to provide a custom sort you can do so via
+// setUserDefinedComparator(). The two elements you are comparing are NSObjects, which conforms to the informal
+// UIAccessibilityProtocol, so you can safely compare properties like accessibilityFrame.
+typedef NSComparisonResult (^ASSortAccessibilityElementsComparator)(NSObject *, NSObject *);
+
+// Use this method to supply your own custom sort comparator used to determine the order of the accessibility elements
+void setUserDefinedAccessibilitySortComparator(ASSortAccessibilityElementsComparator userDefinedComparator);
diff --git a/Source/Details/_ASDisplayViewAccessiblity.mm b/Source/Details/_ASDisplayViewAccessiblity.mm
index fa90666dd..6a1f2f482 100644
--- a/Source/Details/_ASDisplayViewAccessiblity.mm
+++ b/Source/Details/_ASDisplayViewAccessiblity.mm
@@ -10,6 +10,7 @@
 #ifndef ASDK_ACCESSIBILITY_DISABLE
 
 #import <AsyncDisplayKit/_ASDisplayView.h>
+#import <AsyncDisplayKit/_ASDisplayViewAccessiblity.h>
 #import <AsyncDisplayKit/ASAvailability.h>
 #import <AsyncDisplayKit/ASCollectionNode.h>
 #import <AsyncDisplayKit/ASDisplayNodeExtras.h>
@@ -21,35 +22,46 @@
 
 #pragma mark - UIAccessibilityElement
 
-@protocol ASAccessibilityElementPositioning
-
-@property (nonatomic, readonly) CGRect accessibilityFrame;
-
-@end
-
-typedef NSComparisonResult (^SortAccessibilityElementsComparator)(id<ASAccessibilityElementPositioning>, id<ASAccessibilityElementPositioning>);
+static ASSortAccessibilityElementsComparator _userDefinedComparator = nil;
+void setUserDefinedAccessibilitySortComparator(ASSortAccessibilityElementsComparator userDefinedComparator) {
+  _userDefinedComparator = userDefinedComparator;
+}
 
 /// Sort accessiblity elements first by y and than by x origin.
-static void SortAccessibilityElements(NSMutableArray *elements)
+void SortAccessibilityElements(NSMutableArray *elements)
 {
   ASDisplayNodeCAssertNotNil(elements, @"Should pass in a NSMutableArray");
-
-  static SortAccessibilityElementsComparator comparator = nil;
+  
+  static ASSortAccessibilityElementsComparator defaultComparator = nil;
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
-      comparator = ^NSComparisonResult(id<ASAccessibilityElementPositioning> a, id<ASAccessibilityElementPositioning> b) {
-        CGPoint originA = a.accessibilityFrame.origin;
-        CGPoint originB = b.accessibilityFrame.origin;
-        if (originA.y == originB.y) {
-          if (originA.x == originB.x) {
-            return NSOrderedSame;
+    defaultComparator = ^NSComparisonResult(NSObject *a, NSObject *b) {
+      CGPoint originA = a.accessibilityFrame.origin;
+      CGPoint originB = b.accessibilityFrame.origin;
+      if (originA.y == originB.y) {
+        if (originA.x == originB.x) {
+          // if we have the same origin, favor shorter items. If heights are the same, favor thinner items. If size is the same ¯\_(ツ)_/¯
+          CGSize sizeA = a.accessibilityFrame.size;
+          CGSize sizeB = b.accessibilityFrame.size;
+          if (sizeA.height == sizeB.height) {
+            if (sizeA.width == sizeB.width) {
+              return NSOrderedSame;
+            }
+            return (sizeA.width < sizeB.width) ? NSOrderedAscending : NSOrderedDescending;
           }
-          return (originA.x < originB.x) ? NSOrderedAscending : NSOrderedDescending;
+          return (sizeA.height < sizeB.height) ? NSOrderedAscending : NSOrderedDescending;
         }
-        return (originA.y < originB.y) ? NSOrderedAscending : NSOrderedDescending;
-      };
+        return (originA.x < originB.x) ? NSOrderedAscending : NSOrderedDescending;
+      }
+      return (originA.y < originB.y) ? NSOrderedAscending : NSOrderedDescending;
+    };
   });
-  [elements sortUsingComparator:comparator];
+  
+  if (_userDefinedComparator) {
+    [elements sortUsingComparator:_userDefinedComparator];
+  } else {
+    [elements sortUsingComparator:defaultComparator];
+  }
 }
 
 static CGRect ASAccessibilityFrameForNode(ASDisplayNode *node) {
@@ -57,7 +69,7 @@ static CGRect ASAccessibilityFrameForNode(ASDisplayNode *node) {
   return [layer convertRect:node.bounds toLayer:ASFindWindowOfLayer(layer).layer];
 }
 
-@interface ASAccessibilityElement : UIAccessibilityElement<ASAccessibilityElementPositioning>
+@interface ASAccessibilityElement : UIAccessibilityElement
 
 @property (nonatomic) ASDisplayNode *node;
 
@@ -93,7 +105,7 @@ - (CGRect)accessibilityFrame
 
 #pragma mark - _ASDisplayView / UIAccessibilityContainer
 
-@interface ASAccessibilityCustomAction : UIAccessibilityCustomAction<ASAccessibilityElementPositioning>
+@interface ASAccessibilityCustomAction : UIAccessibilityCustomAction
 
 @property (nonatomic) ASDisplayNode *node;
 
@@ -226,6 +238,16 @@ static void CollectAccessibilityElements(ASDisplayNode *node, NSMutableArray *el
   }
 
   for (ASDisplayNode *subnode in node.subnodes) {
+    // If a node is hidden or has an alpha of 0.0 we should not include it
+    if (subnode.hidden || subnode.alpha == 0.0) {
+        continue;
+    }
+    // If a subnode is outside of the view's window, exclude it
+    CGRect nodeInWindowCoords = [node convertRect:subnode.frame toNode:nil];
+    if (!CGRectIntersectsRect(view.window.frame, nodeInWindowCoords)) {
+      continue;
+    }
+    
     if (subnode.isAccessibilityElement) {
       // An accessiblityElement can either be a UIView or a UIAccessibilityElement
       if (subnode.isLayerBacked) {
@@ -275,7 +297,9 @@ - (NSArray *)accessibilityElements
     return @[];
   }
 
-  if (_accessibilityElements == nil) {
+  // when items become hidden/visible we have to manually clear the _accessibilityElements in order to get an updated version
+  // Instead, let's try computing the elements every time and see how badly it affects performance.
+  if (_accessibilityElements == nil || ASActivateExperimentalFeature(ASExperimentalDoNotCacheAccessibilityElements)) {
     _accessibilityElements = [viewNode accessibilityElements];
   }
   return _accessibilityElements;
diff --git a/Tests/ASDisplayViewAccessibilityTests.mm b/Tests/ASDisplayViewAccessibilityTests.mm
index f2476a00d..fdb0832b0 100644
--- a/Tests/ASDisplayViewAccessibilityTests.mm
+++ b/Tests/ASDisplayViewAccessibilityTests.mm
@@ -13,15 +13,19 @@
 #import <XCTest/XCTest.h>
 
 #import <AsyncDisplayKit/_ASDisplayView.h>
+#import <AsyncDisplayKit/_ASDisplayViewAccessiblity.h>
 #import <AsyncDisplayKit/ASButtonNode.h>
 #import <AsyncDisplayKit/ASDisplayNode.h>
 #import <AsyncDisplayKit/ASDisplayNode+Beta.h>
 #import <AsyncDisplayKit/ASTextNode.h>
 #import <AsyncDisplayKit/ASConfiguration.h>
 #import <AsyncDisplayKit/ASConfigurationInternal.h>
+#import <AsyncDisplayKit/ASViewController.h>
 #import <OCMock/OCMock.h>
 #import "ASDisplayNodeTestsHelper.h"
 
+extern void SortAccessibilityElements(NSMutableArray *elements);
+
 @interface ASDisplayViewAccessibilityTests : XCTestCase
 @end
 
@@ -251,4 +255,154 @@ - (void)testThatAccessibilityElementsOverrideWorks {
   XCTAssertEqual(elements.firstObject, label);
 }
 
+- (void)testHiddenAccessibilityElements {
+  ASDisplayNode *containerNode = [[ASDisplayNode alloc] init];
+  containerNode.frame = CGRectMake(0, 0, 100, 200);
+
+  ASTextNode *label = [[ASTextNode alloc] init];
+  label.attributedText = [[NSAttributedString alloc] initWithString:@"test label"];
+  label.frame = CGRectMake(0, 0, 100, 20);
+
+  ASTextNode *hiddenLabel = [[ASTextNode alloc] init];
+  hiddenLabel.attributedText = [[NSAttributedString alloc] initWithString:@"hidden label"];
+  hiddenLabel.frame = CGRectMake(0, 24, 100, 20);
+  hiddenLabel.hidden = YES;
+  
+  [containerNode addSubnode:label];
+  [containerNode addSubnode:hiddenLabel];
+  
+  // force load
+  __unused UIView *view = containerNode.view;
+  
+  NSArray *elements = [containerNode accessibilityElements];
+  XCTAssertTrue(elements.count == 1);
+  XCTAssertEqual(elements.firstObject, label.view);
+}
+
+- (void)testTransparentAccessibilityElements {
+  ASDisplayNode *containerNode = [[ASDisplayNode alloc] init];
+  containerNode.frame = CGRectMake(0, 0, 100, 200);
+
+  ASTextNode *label = [[ASTextNode alloc] init];
+  label.attributedText = [[NSAttributedString alloc] initWithString:@"test label"];
+  label.frame = CGRectMake(0, 0, 100, 20);
+
+  ASTextNode *hiddenLabel = [[ASTextNode alloc] init];
+  hiddenLabel.attributedText = [[NSAttributedString alloc] initWithString:@"hidden label"];
+  hiddenLabel.frame = CGRectMake(0, 24, 100, 20);
+  hiddenLabel.alpha = 0.0;
+  
+  [containerNode addSubnode:label];
+  [containerNode addSubnode:hiddenLabel];
+  
+  // force load
+  __unused UIView *view = containerNode.view;
+  
+  NSArray *elements = [containerNode accessibilityElements];
+  XCTAssertTrue(elements.count == 1);
+  XCTAssertEqual(elements.firstObject, label.view);
+}
+
+- (void)testAccessibilityElementsNotInAppWindow {
+  
+  UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
+  ASDisplayNode *node = [[ASDisplayNode alloc] init];
+  node.automaticallyManagesSubnodes = YES;
+  
+  ASViewController *vc = [[ASViewController alloc] initWithNode:node];
+  window.rootViewController = vc;
+  [window makeKeyAndVisible];
+  [window layoutIfNeeded];
+
+  CGSize windowSize = window.frame.size;
+  ASTextNode *label = [[ASTextNode alloc] init];
+  label.attributedText = [[NSAttributedString alloc] initWithString:@"on screen"];
+  label.frame = CGRectMake(0, 0, 100, 20);
+
+  ASTextNode *partiallyOnScreenNodeY = [[ASTextNode alloc] init];
+  partiallyOnScreenNodeY.attributedText = [[NSAttributedString alloc] initWithString:@"partially on screen y"];
+  partiallyOnScreenNodeY.frame = CGRectMake(0, windowSize.height - 10, 100, 20);
+
+  ASTextNode *partiallyOnScreenNodeX = [[ASTextNode alloc] init];
+  partiallyOnScreenNodeX.attributedText = [[NSAttributedString alloc] initWithString:@"partially on screen x"];
+  partiallyOnScreenNodeX.frame = CGRectMake(0, 100, windowSize.width - 10, 20);
+
+  ASTextNode *offScreenNodeY = [[ASTextNode alloc] init];
+  offScreenNodeY.attributedText = [[NSAttributedString alloc] initWithString:@"off screen y"];
+  offScreenNodeY.frame = CGRectMake(0, windowSize.height + 10, 100, 20);
+
+  ASTextNode *offScreenNodeX = [[ASTextNode alloc] init];
+  offScreenNodeX.attributedText = [[NSAttributedString alloc] initWithString:@"off screen x"];
+  offScreenNodeX.frame = CGRectMake(windowSize.width + 1, 200, 100, 20);
+
+  ASTextNode *offScreenNode = [[ASTextNode alloc] init];
+  offScreenNode.attributedText = [[NSAttributedString alloc] initWithString:@"off screen"];
+  offScreenNode.frame = CGRectMake(windowSize.width + 1, windowSize.height + 1, 100, 20);
+
+  [node addSubnode:label];
+  [node addSubnode:partiallyOnScreenNodeY];
+  [node addSubnode:partiallyOnScreenNodeX];
+  [node addSubnode:offScreenNodeY];
+  [node addSubnode:offScreenNodeX];
+  [node addSubnode:offScreenNode];
+
+  NSArray *elements = [node accessibilityElements];
+  XCTAssertTrue(elements.count == 3);
+  XCTAssertEqual(elements[0], label.view);
+  XCTAssertEqual(elements[1], partiallyOnScreenNodeX.view);
+  XCTAssertEqual(elements[2], partiallyOnScreenNodeY.view);
+}
+
+- (void)testAccessibilitySort {
+  ASDisplayNode *node1 = [[ASDisplayNode alloc] init];
+  node1.accessibilityFrame = CGRectMake(0, 0, 50, 200);
+  
+  ASDisplayNode *node2 = [[ASDisplayNode alloc] init];
+  node2.accessibilityFrame = CGRectMake(0, 0, 100, 200);
+
+  ASDisplayNode *node3 = [[ASDisplayNode alloc] init];
+  node3.accessibilityFrame = CGRectMake(0, 1, 100, 200);
+
+  ASDisplayNode *node4 = [[ASDisplayNode alloc] init];
+  node4.accessibilityFrame = CGRectMake(1, 1, 100, 200);
+  
+  NSMutableArray *elements = [@[node2, node4, node3, node1] mutableCopy];
+  SortAccessibilityElements(elements);
+  XCTAssertEqual(elements[0], node1);
+  XCTAssertEqual(elements[1], node2);
+  XCTAssertEqual(elements[2], node3);
+  XCTAssertEqual(elements[3], node4);
+}
+
+- (void)testCustomAccessibilitySort {
+  
+  // silly custom sorter that puts items with the largest height first.
+  setUserDefinedAccessibilitySortComparator(^NSComparisonResult(NSObject *a, NSObject *b) {
+    if (a.accessibilityFrame.size.height == b.accessibilityFrame.size.height) {
+      return NSOrderedSame;
+    }
+    return a.accessibilityFrame.size.height > b.accessibilityFrame.size.height ? NSOrderedAscending : NSOrderedDescending;
+  });
+  
+  ASDisplayNode *node1 = [[ASDisplayNode alloc] init];
+  node1.accessibilityFrame = CGRectMake(0, 0, 50, 300);
+  
+  ASDisplayNode *node2 = [[ASDisplayNode alloc] init];
+  node2.accessibilityFrame = CGRectMake(0, 0, 100, 250);
+
+  ASDisplayNode *node3 = [[ASDisplayNode alloc] init];
+  node3.accessibilityFrame = CGRectMake(0, 0, 100, 200);
+
+  ASDisplayNode *node4 = [[ASDisplayNode alloc] init];
+  node4.accessibilityFrame = CGRectMake(0, 0, 100, 150);
+  
+  NSMutableArray *elements = [@[node2, node4, node3, node1] mutableCopy];
+  SortAccessibilityElements(elements);
+  XCTAssertEqual(elements[0], node1);
+  XCTAssertEqual(elements[1], node2);
+  XCTAssertEqual(elements[2], node3);
+  XCTAssertEqual(elements[3], node4);
+}
+
+
 @end

From bbbd4f8978bee7af4f566e02cb07e74994f10868 Mon Sep 17 00:00:00 2001
From: ricky cancro <rickycancro@gmail.com>
Date: Tue, 5 May 2020 13:30:03 -0700
Subject: [PATCH 2/9] fix tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In `CollectAccessibilityElements` if the view has no window, don’t do the bounds check.

Would it be better to exit completely if our view wasn’t in a window?
---
 Source/Details/_ASDisplayViewAccessiblity.h  | 2 +-
 Source/Details/_ASDisplayViewAccessiblity.mm | 2 +-
 Tests/ASDisplayViewAccessibilityTests.mm     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Source/Details/_ASDisplayViewAccessiblity.h b/Source/Details/_ASDisplayViewAccessiblity.h
index 33211cb71..2ba46f8c9 100644
--- a/Source/Details/_ASDisplayViewAccessiblity.h
+++ b/Source/Details/_ASDisplayViewAccessiblity.h
@@ -23,7 +23,7 @@
 //
 // In general this seems to work fairly well. However, if you want to provide a custom sort you can do so via
 // setUserDefinedComparator(). The two elements you are comparing are NSObjects, which conforms to the informal
-// UIAccessibilityProtocol, so you can safely compare properties like accessibilityFrame.
+// UIAccessibility protocol, so you can safely compare properties like accessibilityFrame.
 typedef NSComparisonResult (^ASSortAccessibilityElementsComparator)(NSObject *, NSObject *);
 
 // Use this method to supply your own custom sort comparator used to determine the order of the accessibility elements
diff --git a/Source/Details/_ASDisplayViewAccessiblity.mm b/Source/Details/_ASDisplayViewAccessiblity.mm
index 6a1f2f482..dd343811e 100644
--- a/Source/Details/_ASDisplayViewAccessiblity.mm
+++ b/Source/Details/_ASDisplayViewAccessiblity.mm
@@ -244,7 +244,7 @@ static void CollectAccessibilityElements(ASDisplayNode *node, NSMutableArray *el
     }
     // If a subnode is outside of the view's window, exclude it
     CGRect nodeInWindowCoords = [node convertRect:subnode.frame toNode:nil];
-    if (!CGRectIntersectsRect(view.window.frame, nodeInWindowCoords)) {
+    if (view.window && !CGRectIntersectsRect(view.window.frame, nodeInWindowCoords)) {
       continue;
     }
     
diff --git a/Tests/ASDisplayViewAccessibilityTests.mm b/Tests/ASDisplayViewAccessibilityTests.mm
index fdb0832b0..0d214b9f0 100644
--- a/Tests/ASDisplayViewAccessibilityTests.mm
+++ b/Tests/ASDisplayViewAccessibilityTests.mm
@@ -325,7 +325,7 @@ - (void)testAccessibilityElementsNotInAppWindow {
 
   ASTextNode *partiallyOnScreenNodeX = [[ASTextNode alloc] init];
   partiallyOnScreenNodeX.attributedText = [[NSAttributedString alloc] initWithString:@"partially on screen x"];
-  partiallyOnScreenNodeX.frame = CGRectMake(0, 100, windowSize.width - 10, 20);
+  partiallyOnScreenNodeX.frame = CGRectMake(windowSize.width - 10, 100, 100, 20);
 
   ASTextNode *offScreenNodeY = [[ASTextNode alloc] init];
   offScreenNodeY.attributedText = [[NSAttributedString alloc] initWithString:@"off screen y"];

From d3776b9f2c566688839d0c094c5ebd880154c837 Mon Sep 17 00:00:00 2001
From: ricky cancro <rickycancro@gmail.com>
Date: Tue, 5 May 2020 14:25:51 -0700
Subject: [PATCH 3/9] remove conditional in accessibility sort

---
 Source/Details/_ASDisplayViewAccessiblity.mm | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/Source/Details/_ASDisplayViewAccessiblity.mm b/Source/Details/_ASDisplayViewAccessiblity.mm
index dd343811e..47475420e 100644
--- a/Source/Details/_ASDisplayViewAccessiblity.mm
+++ b/Source/Details/_ASDisplayViewAccessiblity.mm
@@ -22,9 +22,11 @@
 
 #pragma mark - UIAccessibilityElement
 
-static ASSortAccessibilityElementsComparator _userDefinedComparator = nil;
+static ASSortAccessibilityElementsComparator currentAccessibilityComparator = nil;
+static ASSortAccessibilityElementsComparator defaultComparator = nil;
+
 void setUserDefinedAccessibilitySortComparator(ASSortAccessibilityElementsComparator userDefinedComparator) {
-  _userDefinedComparator = userDefinedComparator;
+  currentAccessibilityComparator = userDefinedComparator ?: defaultComparator;
 }
 
 /// Sort accessiblity elements first by y and than by x origin.
@@ -32,7 +34,6 @@ void SortAccessibilityElements(NSMutableArray *elements)
 {
   ASDisplayNodeCAssertNotNil(elements, @"Should pass in a NSMutableArray");
   
-  static ASSortAccessibilityElementsComparator defaultComparator = nil;
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
     defaultComparator = ^NSComparisonResult(NSObject *a, NSObject *b) {
@@ -55,13 +56,13 @@ void SortAccessibilityElements(NSMutableArray *elements)
       }
       return (originA.y < originB.y) ? NSOrderedAscending : NSOrderedDescending;
     };
+    
+    if (!currentAccessibilityComparator) {
+      currentAccessibilityComparator = defaultComparator;
+    }
   });
   
-  if (_userDefinedComparator) {
-    [elements sortUsingComparator:_userDefinedComparator];
-  } else {
-    [elements sortUsingComparator:defaultComparator];
-  }
+  [elements sortUsingComparator:currentAccessibilityComparator];
 }
 
 static CGRect ASAccessibilityFrameForNode(ASDisplayNode *node) {

From 666e9fc0b90962108e41931df432ef2509f3fc48 Mon Sep 17 00:00:00 2001
From: ricky cancro <rickycancro@gmail.com>
Date: Tue, 5 May 2020 14:36:39 -0700
Subject: [PATCH 4/9] try to fix the test on github

---
 Tests/ASDisplayViewAccessibilityTests.mm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Tests/ASDisplayViewAccessibilityTests.mm b/Tests/ASDisplayViewAccessibilityTests.mm
index 0d214b9f0..bd237067f 100644
--- a/Tests/ASDisplayViewAccessibilityTests.mm
+++ b/Tests/ASDisplayViewAccessibilityTests.mm
@@ -305,7 +305,7 @@ - (void)testTransparentAccessibilityElements {
 
 - (void)testAccessibilityElementsNotInAppWindow {
   
-  UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
+  UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 568)];
   ASDisplayNode *node = [[ASDisplayNode alloc] init];
   node.automaticallyManagesSubnodes = YES;
   

From 493ce021882775fde5642906216b13d20d83cedc Mon Sep 17 00:00:00 2001
From: ricky cancro <rickycancro@gmail.com>
Date: Tue, 5 May 2020 16:04:45 -0700
Subject: [PATCH 5/9] comparator name change/ tweak test

Changed name of `defaultComparator` to `defaultAccessibilityComparator`
For some reason a accessibility test keeps failing on github, but runs fine locally. Changed the tests to make sure the array contains the proper items but not enforce order.
---
 Podfile.lock                                 | 28 +++-----------------
 Source/Details/_ASDisplayViewAccessiblity.mm |  8 +++---
 Tests/ASDisplayViewAccessibilityTests.mm     |  6 ++---
 3 files changed, 10 insertions(+), 32 deletions(-)

diff --git a/Podfile.lock b/Podfile.lock
index df3c6fcb8..ce77f9d72 100644
--- a/Podfile.lock
+++ b/Podfile.lock
@@ -2,35 +2,16 @@ PODS:
   - FBSnapshotTestCase/Core (2.1.4)
   - JGMethodSwizzler (2.0.1)
   - OCMock (3.4.1)
-  - PINCache (3.0.1-beta.7):
-    - PINCache/Arc-exception-safe (= 3.0.1-beta.7)
-    - PINCache/Core (= 3.0.1-beta.7)
-  - PINCache/Arc-exception-safe (3.0.1-beta.7):
-    - PINCache/Core
-  - PINCache/Core (3.0.1-beta.7):
-    - PINOperation (~> 1.1.1)
-  - PINOperation (1.1.1)
-  - PINRemoteImage (3.0.0-beta.14):
-    - PINRemoteImage/PINCache (= 3.0.0-beta.14)
-  - PINRemoteImage/Core (3.0.0-beta.14):
-    - PINOperation
-  - PINRemoteImage/PINCache (3.0.0-beta.14):
-    - PINCache (= 3.0.1-beta.7)
-    - PINRemoteImage/Core
 
 DEPENDENCIES:
   - FBSnapshotTestCase/Core (~> 2.1)
   - JGMethodSwizzler (from `https://github.com/JonasGessner/JGMethodSwizzler`, branch `master`)
   - OCMock (= 3.4.1)
-  - PINRemoteImage (= 3.0.0-beta.14)
 
 SPEC REPOS:
-  https://github.com/cocoapods/specs.git:
+  https://github.com/CocoaPods/Specs.git:
     - FBSnapshotTestCase
     - OCMock
-    - PINCache
-    - PINOperation
-    - PINRemoteImage
 
 EXTERNAL SOURCES:
   JGMethodSwizzler:
@@ -46,10 +27,7 @@ SPEC CHECKSUMS:
   FBSnapshotTestCase: 094f9f314decbabe373b87cc339bea235a63e07a
   JGMethodSwizzler: 7328146117fffa8a4038c42eb7cd3d4c75006f97
   OCMock: 2cd0716969bab32a2283ff3a46fd26a8c8b4c5e3
-  PINCache: 7cb9ae068c8f655717f7c644ef1dff9fd573e979
-  PINOperation: a6219e6fc9db9c269eb7a7b871ac193bcf400aac
-  PINRemoteImage: 81bbff853acc71c6de9e106e9e489a791b8bbb08
 
-PODFILE CHECKSUM: 445046ac151568c694ff286684322273f0b597d6
+PODFILE CHECKSUM: 345a6700f5fdec438ef5553e1eebf62653862733
 
-COCOAPODS: 1.6.0
+COCOAPODS: 1.9.1
diff --git a/Source/Details/_ASDisplayViewAccessiblity.mm b/Source/Details/_ASDisplayViewAccessiblity.mm
index 47475420e..c6daa7c9d 100644
--- a/Source/Details/_ASDisplayViewAccessiblity.mm
+++ b/Source/Details/_ASDisplayViewAccessiblity.mm
@@ -23,10 +23,10 @@
 #pragma mark - UIAccessibilityElement
 
 static ASSortAccessibilityElementsComparator currentAccessibilityComparator = nil;
-static ASSortAccessibilityElementsComparator defaultComparator = nil;
+static ASSortAccessibilityElementsComparator defaultAccessibilityComparator = nil;
 
 void setUserDefinedAccessibilitySortComparator(ASSortAccessibilityElementsComparator userDefinedComparator) {
-  currentAccessibilityComparator = userDefinedComparator ?: defaultComparator;
+  currentAccessibilityComparator = userDefinedComparator ?: defaultAccessibilityComparator;
 }
 
 /// Sort accessiblity elements first by y and than by x origin.
@@ -36,7 +36,7 @@ void SortAccessibilityElements(NSMutableArray *elements)
   
   static dispatch_once_t onceToken;
   dispatch_once(&onceToken, ^{
-    defaultComparator = ^NSComparisonResult(NSObject *a, NSObject *b) {
+    defaultAccessibilityComparator = ^NSComparisonResult(NSObject *a, NSObject *b) {
       CGPoint originA = a.accessibilityFrame.origin;
       CGPoint originB = b.accessibilityFrame.origin;
       if (originA.y == originB.y) {
@@ -58,7 +58,7 @@ void SortAccessibilityElements(NSMutableArray *elements)
     };
     
     if (!currentAccessibilityComparator) {
-      currentAccessibilityComparator = defaultComparator;
+      currentAccessibilityComparator = defaultAccessibilityComparator;
     }
   });
   
diff --git a/Tests/ASDisplayViewAccessibilityTests.mm b/Tests/ASDisplayViewAccessibilityTests.mm
index bd237067f..fe606fe5d 100644
--- a/Tests/ASDisplayViewAccessibilityTests.mm
+++ b/Tests/ASDisplayViewAccessibilityTests.mm
@@ -348,9 +348,9 @@ - (void)testAccessibilityElementsNotInAppWindow {
 
   NSArray *elements = [node accessibilityElements];
   XCTAssertTrue(elements.count == 3);
-  XCTAssertEqual(elements[0], label.view);
-  XCTAssertEqual(elements[1], partiallyOnScreenNodeX.view);
-  XCTAssertEqual(elements[2], partiallyOnScreenNodeY.view);
+  XCTAssertTrue([elements containsObject:label.view]);
+  XCTAssertTrue([elements containsObject:partiallyOnScreenNodeX.view]);
+  XCTAssertTrue([elements containsObject:partiallyOnScreenNodeY.view]);
 }
 
 - (void)testAccessibilitySort {

From 8140403e79d8d26f8a3acf803e423dad1b202e71 Mon Sep 17 00:00:00 2001
From: ricky cancro <rickycancro@gmail.com>
Date: Tue, 5 May 2020 16:55:25 -0700
Subject: [PATCH 6/9] just making the build run again.

---
 Tests/ASDisplayViewAccessibilityTests.mm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Tests/ASDisplayViewAccessibilityTests.mm b/Tests/ASDisplayViewAccessibilityTests.mm
index fe606fe5d..f7fca3ba1 100644
--- a/Tests/ASDisplayViewAccessibilityTests.mm
+++ b/Tests/ASDisplayViewAccessibilityTests.mm
@@ -123,6 +123,7 @@ - (void)testAccessibilityLayerBackedContainerWithinAccessibilityContainer
   NSArray<UIAccessibilityElement *> *accessibilityElements = container.view.accessibilityElements;
   XCTAssertEqual(accessibilityElements.count, 2);
   XCTAssertEqualObjects(accessibilityElements[1].accessibilityLabel, @"hello, world");
+
 }
 
 - (void)testAccessibilityNonLayerbackedNodesOperationInNonContainer

From c32f52a49ba8ebbc3fdcbee87f7f0372969f00ed Mon Sep 17 00:00:00 2001
From: ricky cancro <rickycancro@gmail.com>
Date: Tue, 5 May 2020 17:11:13 -0700
Subject: [PATCH 7/9] again

This was the last error:
`fatal: unable to access 'https://github.com/pinterest/PINCache.git/': Could not resolve host: github.com`
---
 Tests/ASDisplayViewAccessibilityTests.mm | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Tests/ASDisplayViewAccessibilityTests.mm b/Tests/ASDisplayViewAccessibilityTests.mm
index f7fca3ba1..fe606fe5d 100644
--- a/Tests/ASDisplayViewAccessibilityTests.mm
+++ b/Tests/ASDisplayViewAccessibilityTests.mm
@@ -123,7 +123,6 @@ - (void)testAccessibilityLayerBackedContainerWithinAccessibilityContainer
   NSArray<UIAccessibilityElement *> *accessibilityElements = container.view.accessibilityElements;
   XCTAssertEqual(accessibilityElements.count, 2);
   XCTAssertEqualObjects(accessibilityElements[1].accessibilityLabel, @"hello, world");
-
 }
 
 - (void)testAccessibilityNonLayerbackedNodesOperationInNonContainer

From 3b2ba27530401cccc8a58b1dda3eedba80a0614c Mon Sep 17 00:00:00 2001
From: ricky cancro <rickycancro@gmail.com>
Date: Tue, 5 May 2020 22:14:59 -0700
Subject: [PATCH 8/9] added new exp to configuartion.json and
 ASConfigurationsTests

---
 Schemas/configuration.json    | 1 +
 Tests/ASConfigurationTests.mm | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Schemas/configuration.json b/Schemas/configuration.json
index e18d4380b..d2d041851 100644
--- a/Schemas/configuration.json
+++ b/Schemas/configuration.json
@@ -24,6 +24,7 @@
                     "exp_did_enter_preload_skip_asm_layout",
                     "exp_dispatch_apply",
                     "exp_oom_bg_dealloc_disable",
+                    "exp_do_not_cache_accessibility_elements",
                 ]
     		}
 		}
diff --git a/Tests/ASConfigurationTests.mm b/Tests/ASConfigurationTests.mm
index a0eefcaa2..1614f8bc4 100644
--- a/Tests/ASConfigurationTests.mm
+++ b/Tests/ASConfigurationTests.mm
@@ -53,7 +53,8 @@ + (NSArray *)names {
     @"exp_dispatch_apply",
     @"exp_oom_bg_dealloc_disable",
     @"exp_drawing_global",
-    @"exp_optimize_data_controller_pipeline"
+    @"exp_optimize_data_controller_pipeline",
+    @"exp_do_not_cache_accessibility_elements",
   ];
 }
 

From cb8bd02cbc3eeb240f132d1f44ffad363edf6d25 Mon Sep 17 00:00:00 2001
From: ricky cancro <rickycancro@gmail.com>
Date: Wed, 6 May 2020 10:30:49 -0700
Subject: [PATCH 9/9] =?UTF-8?q?Huy=E2=80=99s=20comments?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* early exit if our view has no window when creating accessibility elements (This change also requiring making sure nodes in accessibility tests were in a window)
* fixed test around experiments.
---
 Source/Details/_ASDisplayViewAccessiblity.h  |  4 +--
 Source/Details/_ASDisplayViewAccessiblity.mm |  9 ++++--
 Tests/ASConfigurationTests.mm                |  5 +++-
 Tests/ASDisplayViewAccessibilityTests.mm     | 29 ++++++++++++++++++++
 Tests/ASScrollNodeTests.mm                   |  4 +++
 5 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/Source/Details/_ASDisplayViewAccessiblity.h b/Source/Details/_ASDisplayViewAccessiblity.h
index 2ba46f8c9..7f159d1d0 100644
--- a/Source/Details/_ASDisplayViewAccessiblity.h
+++ b/Source/Details/_ASDisplayViewAccessiblity.h
@@ -22,8 +22,8 @@
 // and return NSOrderedSame.
 //
 // In general this seems to work fairly well. However, if you want to provide a custom sort you can do so via
-// setUserDefinedComparator(). The two elements you are comparing are NSObjects, which conforms to the informal
-// UIAccessibility protocol, so you can safely compare properties like accessibilityFrame.
+// setUserDefinedAccessibilitySortComparator(). The two elements you are comparing are NSObjects, which conforms to the
+// informal UIAccessibility protocol, so you can safely compare properties like accessibilityFrame.
 typedef NSComparisonResult (^ASSortAccessibilityElementsComparator)(NSObject *, NSObject *);
 
 // Use this method to supply your own custom sort comparator used to determine the order of the accessibility elements
diff --git a/Source/Details/_ASDisplayViewAccessiblity.mm b/Source/Details/_ASDisplayViewAccessiblity.mm
index c6daa7c9d..8115eea53 100644
--- a/Source/Details/_ASDisplayViewAccessiblity.mm
+++ b/Source/Details/_ASDisplayViewAccessiblity.mm
@@ -227,6 +227,11 @@ static void CollectAccessibilityElements(ASDisplayNode *node, NSMutableArray *el
 
   UIView *view = node.view;
 
+  // If we don't have a window, let's just bail out
+  if (!view.window) {
+    return;
+  }
+
   if (node.isAccessibilityContainer && !anySubNodeIsCollection) {
     CollectAccessibilityElementsForContainer(node, view, elements);
     return;
@@ -237,7 +242,7 @@ static void CollectAccessibilityElements(ASDisplayNode *node, NSMutableArray *el
     CollectUIAccessibilityElementsForNode(node, node, view, elements);
     return;
   }
-
+  
   for (ASDisplayNode *subnode in node.subnodes) {
     // If a node is hidden or has an alpha of 0.0 we should not include it
     if (subnode.hidden || subnode.alpha == 0.0) {
@@ -245,7 +250,7 @@ static void CollectAccessibilityElements(ASDisplayNode *node, NSMutableArray *el
     }
     // If a subnode is outside of the view's window, exclude it
     CGRect nodeInWindowCoords = [node convertRect:subnode.frame toNode:nil];
-    if (view.window && !CGRectIntersectsRect(view.window.frame, nodeInWindowCoords)) {
+    if (!CGRectIntersectsRect(view.window.frame, nodeInWindowCoords)) {
       continue;
     }
     
diff --git a/Tests/ASConfigurationTests.mm b/Tests/ASConfigurationTests.mm
index 1614f8bc4..67a0b0090 100644
--- a/Tests/ASConfigurationTests.mm
+++ b/Tests/ASConfigurationTests.mm
@@ -29,7 +29,9 @@
   ASExperimentalDispatchApply,
   ASExperimentalOOMBackgroundDeallocDisable,
   ASExperimentalDrawingGlobal,
-  ASExperimentalOptimizeDataControllerPipeline
+  ASExperimentalOptimizeDataControllerPipeline,
+  ASExperimentalTraitCollectionDidChangeWithPreviousCollection,
+  ASExperimentalDoNotCacheAccessibilityElements,
 };
 
 @interface ASConfigurationTests : ASTestCase <ASConfigurationDelegate>
@@ -54,6 +56,7 @@ + (NSArray *)names {
     @"exp_oom_bg_dealloc_disable",
     @"exp_drawing_global",
     @"exp_optimize_data_controller_pipeline",
+    @"exp_trait_collection_did_change_with_previous_collection",
     @"exp_do_not_cache_accessibility_elements",
   ];
 }
diff --git a/Tests/ASDisplayViewAccessibilityTests.mm b/Tests/ASDisplayViewAccessibilityTests.mm
index fe606fe5d..d52151cd6 100644
--- a/Tests/ASDisplayViewAccessibilityTests.mm
+++ b/Tests/ASDisplayViewAccessibilityTests.mm
@@ -38,6 +38,11 @@ - (void)testAccessibilityElementsAccessors
   ASDisplayNode *subnode = nil;
   node = [[ASDisplayNode alloc] init];
   subnode = [[ASDisplayNode alloc] init];
+  
+  UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 560)];
+  [window addSubnode:node];
+  [window makeKeyAndVisible];
+
   NSString *label = @"foo";
   subnode.isAccessibilityElement = YES;
   subnode.accessibilityLabel = label;
@@ -60,6 +65,10 @@ - (void)testThatSubnodeAccessibilityLabelAggregationWorks
   innerNode1 = [[ASDisplayNode alloc] init];
   innerNode2 = [[ASDisplayNode alloc] init];
 
+  UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 560)];
+  [window addSubnode:node];
+  [window makeKeyAndVisible];
+
   // Initialize nodes with relevant accessibility data
   node.isAccessibilityContainer = YES;
   innerNode1.accessibilityLabel = @"hello";
@@ -82,6 +91,10 @@ - (void)testThatContainerAccessibilityLabelOverrideStopsAggregation
   node = [[ASDisplayNode alloc] init];
   innerNode = [[ASDisplayNode alloc] init];
 
+  UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 560)];
+  [window addSubnode:node];
+  [window makeKeyAndVisible];
+
   // Initialize nodes with relevant accessibility data
   node.isAccessibilityContainer = YES;
   node.accessibilityLabel = @"hello";
@@ -98,6 +111,10 @@ - (void)testThatContainerAccessibilityLabelOverrideStopsAggregation
 - (void)testAccessibilityLayerBackedContainerWithinAccessibilityContainer
 {
   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.isAccessibilityContainer = YES;
 
@@ -206,7 +223,12 @@ - (void)testActionForwarding {
 - (void)fakeSelector:(id)sender { }
 
 - (void)testThatAccessibilityElementsWorks {
+  
   ASDisplayNode *containerNode = [[ASDisplayNode alloc] init];
+  UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 560)];
+  [window addSubnode:containerNode];
+  [window makeKeyAndVisible];
+  
   containerNode.frame = CGRectMake(0, 0, 100, 200);
 
   ASTextNode *label = [[ASTextNode alloc] init];
@@ -257,6 +279,10 @@ - (void)testThatAccessibilityElementsOverrideWorks {
 
 - (void)testHiddenAccessibilityElements {
   ASDisplayNode *containerNode = [[ASDisplayNode alloc] init];
+  UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 560)];
+  [window addSubnode:containerNode];
+  [window makeKeyAndVisible];
+
   containerNode.frame = CGRectMake(0, 0, 100, 200);
 
   ASTextNode *label = [[ASTextNode alloc] init];
@@ -281,6 +307,9 @@ - (void)testHiddenAccessibilityElements {
 
 - (void)testTransparentAccessibilityElements {
   ASDisplayNode *containerNode = [[ASDisplayNode alloc] init];
+  UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 560)];
+  [window addSubnode:containerNode];
+  [window makeKeyAndVisible];
   containerNode.frame = CGRectMake(0, 0, 100, 200);
 
   ASTextNode *label = [[ASTextNode alloc] init];
diff --git a/Tests/ASScrollNodeTests.mm b/Tests/ASScrollNodeTests.mm
index f9dbd5152..1f7e8c176 100644
--- a/Tests/ASScrollNodeTests.mm
+++ b/Tests/ASScrollNodeTests.mm
@@ -170,6 +170,10 @@ - (void)testAutomaticallyManagesContentSizeWithInvalidCalculatedSizeForLayout
 
 - (void)testASScrollNodeAccessibility {
   ASDisplayNode *scrollNode = [[ASDisplayNode alloc] init];
+  UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 560)];
+  [window addSubnode:scrollNode];
+  [window makeKeyAndVisible];
+
   ASDisplayNode *node = [[ASDisplayNode alloc] init];
   node.isAccessibilityContainer = YES;
   node.accessibilityLabel = @"node";