Skip to content

Commit

Permalink
Fix WKWebView Accessibility (TextureGroup#1955)
Browse files Browse the repository at this point in the history
* Return nil instead of empty array when no accessibility elements are found. Fixes TextureGroup#1954.

* Use nullability annotations to fix static analyzer warnings.

* Add UI test target.

* Add UI test to make sure web view stays accessible.

* Revert "Add UI test to make sure web view stays accessible."

This reverts commit 00253f4.

* Revert "Add UI test target."

This reverts commit 288b5e0.

* Add unit test to make sure accessibility elements are correct when a WKWebView is wrapped in an ASDisplayNode.
  • Loading branch information
ZevEisenberg committed Feb 6, 2021
1 parent 8395c1a commit 45aba97
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 6 deletions.
4 changes: 4 additions & 0 deletions AsyncDisplayKit.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@
CCEDDDD9200C518800FFCD0A /* ASConfigurationTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CCEDDDD8200C518800FFCD0A /* ASConfigurationTests.mm */; };
CCF18FF41D2575E300DF5895 /* NSIndexSet+ASHelpers.h in Headers */ = {isa = PBXBuildFile; fileRef = CC4981BA1D1C7F65004E13CC /* NSIndexSet+ASHelpers.h */; settings = {ATTRIBUTES = (Private, ); }; };
CCF1FF5E20C4785000AAD8FC /* ASLocking.h in Headers */ = {isa = PBXBuildFile; fileRef = CCF1FF5D20C4785000AAD8FC /* ASLocking.h */; settings = {ATTRIBUTES = (Public, ); }; };
CD0F261C25CA03BB00735A79 /* WebKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = CD0F261B25CA03BB00735A79 /* WebKit.framework */; };
D933F041224AD17F00FF495E /* ASTransactionTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = D933F040224AD17F00FF495E /* ASTransactionTests.mm */; };
D99F9158232990F30083CC8E /* ASImageNodeTests.m in Sources */ = {isa = PBXBuildFile; fileRef = D99F9157232990F30083CC8E /* ASImageNodeTests.m */; };
DB55C2671C641AE4004EDCF5 /* ASContextTransitioning.h in Headers */ = {isa = PBXBuildFile; fileRef = DB55C2651C641AE4004EDCF5 /* ASContextTransitioning.h */; settings = {ATTRIBUTES = (Public, ); }; };
Expand Down Expand Up @@ -994,6 +995,7 @@
CCEDDDD0200C488000FFCD0A /* ASConfiguration.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = ASConfiguration.mm; sourceTree = "<group>"; };
CCEDDDD8200C518800FFCD0A /* ASConfigurationTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = ASConfigurationTests.mm; sourceTree = "<group>"; };
CCF1FF5D20C4785000AAD8FC /* ASLocking.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ASLocking.h; sourceTree = "<group>"; };
CD0F261B25CA03BB00735A79 /* WebKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = WebKit.framework; path = Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/System/Library/Frameworks/WebKit.framework; sourceTree = DEVELOPER_DIR; };
D3779BCFF841AD3EB56537ED /* Pods-AsyncDisplayKitTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-AsyncDisplayKitTests.release.xcconfig"; path = "Pods/Target Support Files/Pods-AsyncDisplayKitTests/Pods-AsyncDisplayKitTests.release.xcconfig"; sourceTree = "<group>"; };
D785F6601A74327E00291744 /* ASScrollNode.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASScrollNode.h; sourceTree = "<group>"; };
D785F6611A74327E00291744 /* ASScrollNode.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASScrollNode.mm; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1075,6 +1077,7 @@
isa = PBXFrameworksBuildPhase;
buildActionMask = 2147483647;
files = (
CD0F261C25CA03BB00735A79 /* WebKit.framework in Frameworks */,
CC36C19F218B894800232F23 /* CoreMedia.framework in Frameworks */,
CC36C19E218B894400232F23 /* AVFoundation.framework in Frameworks */,
CC90E1F41E383C0400FED591 /* AsyncDisplayKit.framework in Frameworks */,
Expand Down Expand Up @@ -1155,6 +1158,7 @@
058D09AE195D04C000B7D73C /* Frameworks */ = {
isa = PBXGroup;
children = (
CD0F261B25CA03BB00735A79 /* WebKit.framework */,
CC36C19B218B847400232F23 /* CoreMedia.framework */,
CC36C199218B846F00232F23 /* CoreLocation.framework */,
CC36C197218B846300232F23 /* QuartzCore.framework */,
Expand Down
12 changes: 7 additions & 5 deletions Source/Details/_ASDisplayViewAccessiblity.mm
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ @implementation _ASDisplayView (UIAccessibilityContainer)

#pragma mark - UIAccessibility

- (void)setAccessibilityElements:(NSArray *)accessibilityElements
- (void)setAccessibilityElements:(nullable NSArray *)accessibilityElements
{
ASDisplayNodeAssertMainThread();
// While it looks very strange to ignore the accessibilyElements param and set _accessibilityElements to nil, it is actually on purpose.
Expand All @@ -331,7 +331,7 @@ - (void)setAccessibilityElements:(NSArray *)accessibilityElements
_accessibilityElements = nil;
}

- (NSArray *)accessibilityElements
- (nullable NSArray *)accessibilityElements
{
ASDisplayNodeAssertMainThread();

Expand All @@ -352,7 +352,7 @@ - (NSArray *)accessibilityElements

@implementation ASDisplayNode (AccessibilityInternal)

- (NSArray *)accessibilityElements
- (nullable NSArray *)accessibilityElements
{
// NSObject implements the informal accessibility protocol. This means that all ASDisplayNodes already have an accessibilityElements
// property. If an ASDisplayNode subclass has explicitly set the property, let's use that instead of traversing the node tree to try
Expand All @@ -364,12 +364,14 @@ - (NSArray *)accessibilityElements

if (!self.isNodeLoaded) {
ASDisplayNodeFailAssert(@"Cannot access accessibilityElements since node is not loaded");
return @[];
return nil;
}
NSMutableArray *accessibilityElements = [[NSMutableArray alloc] init];
CollectAccessibilityElements(self, accessibilityElements);
SortAccessibilityElements(accessibilityElements);
return accessibilityElements;
// If we did not find any accessibility elements, return nil instead of empty array. This allows a WKWebView within the node
// to participate in accessibility.
return accessibilityElements.count == 0 ? nil : accessibilityElements;
}

@end
Expand Down
2 changes: 1 addition & 1 deletion Source/Private/ASDisplayNode+FrameworkPrivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ NS_INLINE UIAccessibilityTraits ASInteractiveAccessibilityTraitsMask() {
}

@interface ASDisplayNode (AccessibilityInternal)
- (NSArray *)accessibilityElements;
- (nullable NSArray *)accessibilityElements;
@end;

@interface UIView (ASDisplayNodeInternal)
Expand Down
29 changes: 29 additions & 0 deletions Tests/ASDisplayViewAccessibilityTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#import <AsyncDisplayKit/ASDKViewController.h>
#import <OCMock/OCMock.h>
#import "ASDisplayNodeTestsHelper.h"
#import <WebKit/WebKit.h>

extern void SortAccessibilityElements(NSMutableArray *elements);

Expand Down Expand Up @@ -187,6 +188,34 @@ - (void)testAccessibilityNonLayerbackedNodesOperationInNonContainer
XCTAssertTrue([[updatedElements2.lastObject accessibilityLabel] isEqualToString:@"world"]);
}

- (void)testAccessibilityElementsAreNilForWrappedWKWebView {
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);
WKWebView *webView = [[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 200, 400)];
[container.view addSubview:webView];

NSString *htmlString =
@"<html>"
@" <head>"
@" <meta name='viewport' content='width=device-width' />"
@" </head>"
@" <body>"
@" <h1>Texture is Awesome!</h1>"
@" <p>Especially when web views inside nodes are accessible.</p>"
@" </body>"
@" </html>";
[webView loadHTMLString:htmlString baseURL:nil];

// Accessibility elements should be nil in this case, because
// WKWebView handles accessibility out of process.
NSArray *accessibilityElements = container.accessibilityElements;
XCTAssertNil(accessibilityElements);
}

#pragma mark -
#pragma mark UIAccessibilityAction Forwarding

Expand Down

0 comments on commit 45aba97

Please sign in to comment.