[Accessibility] Do not exclude elements outside the window’s rect that are subviews of UIScrollView

.github/workflows/ci-master-only.yml

@@ -8,11 +8,11 @@ on:
 jobs:
   cocoapods-lint:
     env: 
-        DEVELOPER_DIR: /Applications/Xcode_11.app/Contents/Developer
+        DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
     name: Verify that podspec lints
     runs-on: macOS-latest
     steps:
     - name: Checkout the Git repository
-      uses: actions/checkout@v1
+      uses: actions/checkout@v2
     - name: Run build.sh with cocoapods-lint mode
       run: ./build.sh cocoapods-lint

.github/workflows/ci-pull-requests-only.yml

@@ -8,7 +8,7 @@ on:
 jobs:
   buildsh:
     env:
-        DEVELOPER_DIR: /Applications/Xcode_11.app/Contents/Developer
+        DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
     strategy:
       matrix:
         mode: [cocoapods-lint-default-subspecs, cocoapods-lint-other-subspecs]
@@ -21,6 +21,6 @@ jobs:
     runs-on: macOS-latest
     steps:
     - name: Checkout the Git repository
-      uses: actions/checkout@v1
+      uses: actions/checkout@v2
     - name: Run build script
       run: ./build.sh ${{ matrix.mode }}

.github/workflows/ci.yml

@@ -5,7 +5,7 @@ on: [push, pull_request]
 jobs:
   buildsh:
     env:
-        DEVELOPER_DIR: /Applications/Xcode_11.app/Contents/Developer
+        DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
     strategy:
       matrix:
         mode: [tests, framework, life-without-cocoapods, carthage, examples-pt1, examples-pt2, examples-pt3, examples-pt4]
@@ -30,6 +30,6 @@ jobs:
     runs-on: macOS-latest
     steps:
     - name: Checkout the Git repository
-      uses: actions/checkout@v1
+      uses: actions/checkout@v2
     - name: Run build script
       run: ./build.sh ${{ matrix.mode }}

.github/workflows/danger.yml

@@ -4,8 +4,8 @@ on: [pull_request]
 
 jobs:
   buildsh:
-    env: 
-        DEVELOPER_DIR: /Applications/Xcode_11.app/Contents/Developer
+    env:
+        DEVELOPER_DIR: /Applications/Xcode_11.4.1.app/Contents/Developer
     strategy:
       matrix:
         mode: [danger]

Source/Details/ASGraphicsContext.h

@@ -30,6 +30,23 @@ NS_ASSUME_NONNULL_BEGIN
  */
 AS_EXTERN UIImage *ASGraphicsCreateImageWithOptions(CGSize size, BOOL opaque, CGFloat scale, UIImage * _Nullable sourceImage, asdisplaynode_iscancelled_block_t NS_NOESCAPE _Nullable isCancelled, void (NS_NOESCAPE ^work)(void)) ASDISPLAYNODE_DEPRECATED_MSG("Use ASGraphicsCreateImageWithTraitCollectionAndOptions instead");
 
+/**
+* A wrapper for the UIKit drawing APIs. If you are in ASExperimentalDrawingGlobal, and you have iOS >= 10, we will create
+* a UIGraphicsRenderer with an appropriate format. Otherwise, we will use UIGraphicsBeginImageContext et al.
+*
+* @param traitCollection Trait collection. The `work` block will be executed with this trait collection, so it will affect dynamic colors, etc.
+* @param size The size of the context.
+* @param opaque Whether the context should be opaque or not.
+* @param scale The scale of the context. 0 uses main screen scale.
+* @param sourceImage If you are planning to render a UIImage into this context, provide it here and we will use its
+*   preferred renderer format if we are using UIGraphicsImageRenderer.
+* @param isCancelled An optional block for canceling the drawing before forming the image.
+* @param work A block, wherein the current UIGraphics context is set based on the arguments.
+*
+* @return The rendered image. You can also render intermediary images using UIGraphicsGetImageFromCurrentImageContext.
+*/
+AS_EXTERN UIImage *ASGraphicsCreateImage(ASPrimitiveTraitCollection traitCollection, CGSize size, BOOL opaque, CGFloat scale, UIImage * _Nullable sourceImage, asdisplaynode_iscancelled_block_t _Nullable NS_NOESCAPE isCancelled, void (NS_NOESCAPE ^work)(void));
+
 /**
 * A wrapper for the UIKit drawing APIs.
 *

Source/Details/ASGraphicsContext.mm

@@ -14,7 +14,7 @@
 
 
 #if AS_AT_LEAST_IOS13
-#define PERFORM_WORK_WITH_TRAIT_COLLECTION(work, traitCollection) \
+#define ASPerformBlockWithTraitCollection(work, traitCollection) \
     if (@available(iOS 13.0, *)) { \
       UITraitCollection *uiTraitCollection = ASPrimitiveTraitCollectionToUITraitCollection(traitCollection); \
       [uiTraitCollection performAsCurrentTraitCollection:^{ \
@@ -24,7 +24,7 @@
       work(); \
     }
 #else
-#define PERFORM_WORK_WITH_TRAIT_COLLECTION(work, traitCollection) work();
+#define ASPerformBlockWithTraitCollection(work, traitCollection) work();
 #endif
 
 
@@ -43,10 +43,10 @@ NS_INLINE void ASConfigureExtendedRange(UIGraphicsImageRendererFormat *format)
                                           asdisplaynode_iscancelled_block_t NS_NOESCAPE isCancelled,
                                           void (^NS_NOESCAPE work)())
 {
-  return ASGraphicsCreateImageWithTraitCollectionAndOptions(ASPrimitiveTraitCollectionMakeDefault(), size, opaque, scale, sourceImage, work);
+  return ASGraphicsCreateImage(ASPrimitiveTraitCollectionMakeDefault(), size, opaque, scale, sourceImage, isCancelled, work);
 }
 
-UIImage *ASGraphicsCreateImageWithTraitCollectionAndOptions(ASPrimitiveTraitCollection traitCollection, CGSize size, BOOL opaque, CGFloat scale, UIImage * sourceImage, void (NS_NOESCAPE ^work)()) {
+UIImage *ASGraphicsCreateImage(ASPrimitiveTraitCollection traitCollection, CGSize size, BOOL opaque, CGFloat scale, UIImage * sourceImage, asdisplaynode_iscancelled_block_t NS_NOESCAPE isCancelled, void (NS_NOESCAPE ^work)()) {
   if (AS_AVAILABLE_IOS_TVOS(10, 10)) {
     if (ASActivateExperimentalFeature(ASExperimentalDrawingGlobal)) {
       // If they used default scale, reuse one of two preferred formats.
@@ -98,17 +98,39 @@ NS_INLINE void ASConfigureExtendedRange(UIGraphicsImageRendererFormat *format)
         ASConfigureExtendedRange(format);
       }
 
-      return [[[UIGraphicsImageRenderer alloc] initWithSize:size format:format] imageWithActions:^(UIGraphicsImageRendererContext *rendererContext) {
-        ASDisplayNodeCAssert(UIGraphicsGetCurrentContext(), @"Should have a context!");
-        PERFORM_WORK_WITH_TRAIT_COLLECTION(work, traitCollection)
-      }];
+      // Avoid using the imageWithActions: method because it does not support cancellation at the
+      // last moment i.e. before actually creating the resulting image.
+      __block UIImage *image;
+      NSError *error;
+      [[[UIGraphicsImageRenderer alloc] initWithSize:size format:format]
+          runDrawingActions:^(UIGraphicsImageRendererContext *rendererContext) {
+            ASDisplayNodeCAssert(UIGraphicsGetCurrentContext(), @"Should have a context!");
+            ASPerformBlockWithTraitCollection(work, traitCollection);
+          }
+          completionActions:^(UIGraphicsImageRendererContext *rendererContext) {
+            if (isCancelled == nil || !isCancelled()) {
+              image = rendererContext.currentImage;
+            }
+          }
+          error:&error];
+      if (error) {
+        NSCAssert(NO, @"Error drawing: %@", error);
+      }
+      return image;
     }
   }
 
   // Bad OS or experiment flag. Use UIGraphics* API.
   UIGraphicsBeginImageContextWithOptions(size, opaque, scale);
-  PERFORM_WORK_WITH_TRAIT_COLLECTION(work, traitCollection)
-  UIImage *image = UIGraphicsGetImageFromCurrentImageContext();
+  ASPerformBlockWithTraitCollection(work, traitCollection)
+  UIImage *image = nil;
+  if (isCancelled == nil || !isCancelled()) {
+    image = UIGraphicsGetImageFromCurrentImageContext();
+  }
   UIGraphicsEndImageContext();
   return image;
 }
+
+UIImage *ASGraphicsCreateImageWithTraitCollectionAndOptions(ASPrimitiveTraitCollection traitCollection, CGSize size, BOOL opaque, CGFloat scale, UIImage * sourceImage, void (NS_NOESCAPE ^work)()) {
+  return ASGraphicsCreateImage(traitCollection, size, opaque, scale, sourceImage, nil, work);
+}

Source/Details/_ASDisplayViewAccessiblity.mm

@@ -210,6 +210,17 @@ static void CollectAccessibilityElementsForContainer(ASDisplayNode *container, U
   [elements addObject:accessiblityElement];
 }
 
+/// Check if a view is a subviews of an UIScrollView. This is used to determine whether to enforce that
+/// accessibility elements must be on screen
+static BOOL recusivelyCheckSuperviewsForScrollView(UIView *view) {
+    if (!view) {
+        return NO;
+    } else if ([view isKindOfClass:[UIScrollView class]]) {
+        return YES;
+    }
+    return recusivelyCheckSuperviewsForScrollView(view.superview);
+}
+
 /// Collect all accessibliity elements for a given view and view node
 static void CollectAccessibilityElements(ASDisplayNode *node, NSMutableArray *elements)
 {
@@ -226,7 +237,7 @@ 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;
@@ -246,11 +257,13 @@ 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;
+      continue;
     }
-    // If a subnode is outside of the view's window, exclude it
+
+    // If a subnode is outside of the view's window, exclude it UNLESS it is a subview of an UIScrollView.
+    // In this case UIKit will return the element even if it is outside of the window or the scrollView's visible rect (contentOffset + contentSize)
     CGRect nodeInWindowCoords = [node convertRect:subnode.frame toNode:nil];
-    if (!CGRectIntersectsRect(view.window.frame, nodeInWindowCoords)) {
+    if (!CGRectIntersectsRect(view.window.frame, nodeInWindowCoords) && !recusivelyCheckSuperviewsForScrollView(view)) {
       continue;
     }
 

Source/Private/ASIGListAdapterBasedDataSource.mm

@@ -86,6 +86,21 @@ - (void)collectionNode:(ASCollectionNode *)collectionNode didSelectItemAtIndexPa
   [self.delegate collectionView:collectionNode.view didSelectItemAtIndexPath:indexPath];
 }
 
+- (void)collectionNode:(ASCollectionNode *)collectionNode didDeselectItemAtIndexPath:(NSIndexPath *)indexPath
+{
+  [self.delegate collectionView:collectionNode.view didDeselectItemAtIndexPath:indexPath];
+}
+
+- (void)collectionNode:(ASCollectionNode *)collectionNode didHighlightItemAtIndexPath:(NSIndexPath *)indexPath
+{
+  [self.delegate collectionView:collectionNode.view didHighlightItemAtIndexPath:indexPath];
+}
+
+- (void)collectionNode:(ASCollectionNode *)collectionNode didUnhighlightItemAtIndexPath:(NSIndexPath *)indexPath
+{
+  [self.delegate collectionView:collectionNode.view didUnhighlightItemAtIndexPath:indexPath];
+}
+
 - (void)scrollViewDidScroll:(UIScrollView *)scrollView
 {
   [self.delegate scrollViewDidScroll:scrollView];
@@ -147,6 +162,32 @@ - (void)collectionNode:(ASCollectionNode *)collectionNode willBeginBatchFetchWit
   [ctrl beginBatchFetchWithContext:context];
 }
 
+- (void)collectionNode:(ASCollectionNode *)collectionNode willDisplayItemWithNode:(ASCellNode *)node
+{
+  NSIndexPath *indexPath = [collectionNode.view indexPathForNode:node];
+  UIView *contentView = node.view.superview;
+  UICollectionViewCell *cell = contentView.superview;
+
+  if (cell == nil || indexPath == nil) {
+    return;
+  }
+
+  [self.delegate collectionView:collectionNode.view willDisplayCell:cell forItemAtIndexPath:indexPath];
+}
+
+- (void)collectionNode:(ASCollectionNode *)collectionNode didEndDisplayingItemWithNode:(ASCellNode *)node
+{
+  NSIndexPath *indexPath = [collectionNode.view indexPathForNode:node];
+  UIView *contentView = node.view.superview;
+  UICollectionViewCell *cell = contentView.superview;
+
+  if (cell == nil || indexPath == nil) {
+    return;
+  }
+
+  [self.delegate collectionView:collectionNode.view didEndDisplayingCell:cell forItemAtIndexPath:indexPath];
+}
+
 /**
  * Note: It is not documented that ASCollectionNode will forward these UIKit delegate calls if they are implemented.
  * It is not considered harmful to do so, and adding them to documentation will confuse most users, who should

Tests/ASCollectionViewTests.mm

@@ -182,18 +182,6 @@ - (void)setUp
   [ASConfigurationManager test_resetWithConfiguration:config];
 }
 
-- (void)tearDown
-{
-  // We can't prevent the system from retaining windows, but we can at least clear them out to avoid
-  // pollution between test cases.
-  for (UIWindow *window in [UIApplication sharedApplication].windows) {
-    for (UIView *subview in window.subviews) {
-      [subview removeFromSuperview];
-    }
-  }
-  [super tearDown];
-}
-
 - (void)testDataSourceImplementsNecessaryMethods
 {
   UICollectionViewFlowLayout *layout = [[UICollectionViewFlowLayout alloc] init];

Tests/ASDisplayViewAccessibilityTests.mm

@@ -20,6 +20,7 @@
 #import <AsyncDisplayKit/ASTextNode.h>
 #import <AsyncDisplayKit/ASConfiguration.h>
 #import <AsyncDisplayKit/ASConfigurationInternal.h>
+#import <AsyncDisplayKit/ASScrollNode.h>
 #import <AsyncDisplayKit/ASViewController.h>
 #import <OCMock/OCMock.h>
 #import "ASDisplayNodeTestsHelper.h"
@@ -382,6 +383,60 @@ - (void)testAccessibilityElementsNotInAppWindow {
   XCTAssertTrue([elements containsObject:partiallyOnScreenNodeY.view]);
 }
 
+- (void)testAccessibilityElementsNotInAppWindowButInScrollView {
+
+  UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 568)];
+  ASScrollNode *node = [[ASScrollNode alloc] init];
+  node.automaticallyManagesSubnodes = YES;
+
+  ASViewController *vc = [[ASViewController alloc] initWithNode:node];
+  window.rootViewController = vc;
+  [window makeKeyAndVisible];
+  [window layoutIfNeeded];
+
+  CGSize windowSize = window.frame.size;
+  node.view.contentSize = CGSizeMake(window.frame.size.width, window.frame.size.height * 2.0);
+  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(windowSize.width - 10, 100, 100, 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 == 6);
+  XCTAssertTrue([elements containsObject:label.view]);
+  XCTAssertTrue([elements containsObject:partiallyOnScreenNodeX.view]);
+  XCTAssertTrue([elements containsObject:partiallyOnScreenNodeY.view]);
+  XCTAssertTrue([elements containsObject:offScreenNodeY.view]);
+  XCTAssertTrue([elements containsObject:offScreenNodeX.view]);
+  XCTAssertTrue([elements containsObject:offScreenNode.view]);
+}
+
 - (void)testAccessibilitySort {
   ASDisplayNode *node1 = [[ASDisplayNode alloc] init];
   node1.accessibilityFrame = CGRectMake(0, 0, 50, 200);