cashapp / AccessibilitySnapshot

Easy regression testing for iOS accessibility
Apache License 2.0
541 stars 69 forks source link

Remove the use of UIApplication.shared #19

Closed jiayi-zhou closed 3 years ago

jiayi-zhou commented 4 years ago

When importing into a project with Require Only App-Extension-Safe API = YES, the use of UIApplication.shared is not allowed. This is used in AccessibilityHierarchyParser.swift to determine the user interface layout direction.

This information can be determined by using let userInterfaceLayoutDirection = root. effectiveUserInterfaceLayoutDirection (available as of iOS 10.0), and this is a better way of determining layout direction that is specific to each UIView that is analyzed.

NickEntin commented 4 years ago

Hi @jiayi-zhou, thanks for your interest in AccessibilitySnapshot!

The parser intentionally uses the application's user interface direction instead of the view's user interface direction, since that matches the observed behavior of assistive technologies. You can see this in the project's demo app by following these steps:

  1. Select an interface direction to test. Since the demo app is only localized to English currently, it will default to LTR, but you can set the scheme's application language to "Right-to-Left Pseudolanguage" to test the RTL case.
  2. Run the app on your device.
  3. Open the "Element Order with Semantic Content" screen.
  4. You can breakpoint in the layout code of UserInterfaceDirectionViewController.View to check the effectiveUserInterfaceLayoutDirection of the various views. You should see that this is dependent on the semanticContentAttribute of each view, as expected.
  5. Turn on an assistive technology. Either VoiceOver or Switch Control works well to test this.
  6. Iterate forward through the elements (with Switch Control, perform the "Move To Next Item" action; with VoiceOver in LTR, perform a left-to-right flick gesture; with VoiceOver in RTL, perform a right-to-left flick gesture). You should see that the ordering is consistent in each row, matching the application-level setting, rather than changing based on each row's semantic content attribute.

Relying on the view's user interface direction when parsing the hierarchy would therefore not match the interactions we're representing.

What's your use case for setting Require Only App-Extension-Safe API = YES? It's possible we may be able to rearrange things, for example to determine the user interface direction somewhere else.

jiayi-zhou commented 4 years ago

I see, thanks for the explanation! I am currently working on an extension to https://github.com/playbook-ui/playbook-ios in order to capture accessibility information, and that framework has the setting Require Only App-Extension-Safe API set to YES. Thus, its dependencies must avoid using UIApplication.shared. Is it possible to pass in user interface direction from outside of this framework instead?

NickEntin commented 4 years ago

Oh cool! That looks like a really interesting project.

It sounds like you only need to depend on the Core subspec (see the wiki for more background on this). One option is to move the responsibility for determining the interface direction to the integration layer. Specifically, that would involve:

  1. Remove the default value for userInterfaceLayoutDirectionProvider from AccessibilityHierarchyParser.parseAccessibilityElements(in:userInterfaceLayoutDirectionProvider:).
  2. Add a matching userInterfaceLayoutDirectionProvider parameter (also with no default value) to AccessibilitySnapshotView.parseAccessibility(useMonochromeSnapshot:) and pass the value through to the parser.
  3. Pass UIApplication.shared as the provider in the relevant calls from the FBSnapshotTestCase extensions.

This would remove all references to UIApplication.shared from the Core subspec, which would allow it to be used in application extensions. The downside here is that consumers would need to understand what to pass for this value. We can document how to handle it in the main application, but I'm not sure what the right answer to that is in extensions.

NickEntin commented 3 years ago

Since it looks like you were able to work around this in PlaybookAccessibilitySnapshot and it's unclear how we could cleanly address the issue here, I'm going to close this issue as wontfix. If you have any ideas for ways we could handle app extensions better, please feel free to reopen the issue. Thanks!