Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing pa11y and fa11y commands for iOS8.1+ #128

Merged
merged 5 commits into from
Nov 30, 2015

Conversation

gkassabli
Copy link
Contributor

Pa11y and fa11y weren't working since iOS8.1 because of Apple API changes. This fix should work both on newer and older versions

@@ -77,7 +77,7 @@ def forceStartAccessibilityServer():
#We try to start accessibility server only if we don't have needed method active
if not fb.evaluateBooleanExpression('[UIView instancesRespondToSelector:@selector(_accessibilityElementsInContainer:)]'):
#Starting accessibility server is different for simulator and device
if fb.evaluateExpressionValue('(id)[[UIDevice currentDevice] model]').GetObjectDescription().lower().find('simulator') >= 0:
if fb.evaluateExpressionValue('(id)[[UIDevice currentDevice] name]').GetObjectDescription().lower().find('simulator') >= 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a backwards compatible change, will it still work < iOS 8.1?

@kastiglione
Copy link
Contributor

Thanks @gkassabli! I have a couple questions.

@gkassabli
Copy link
Contributor Author

Thanks for good point, @kastiglione. Addressed your comments!

@@ -77,7 +77,7 @@ def forceStartAccessibilityServer():
#We try to start accessibility server only if we don't have needed method active
if not fb.evaluateBooleanExpression('[UIView instancesRespondToSelector:@selector(_accessibilityElementsInContainer:)]'):
#Starting accessibility server is different for simulator and device
if fb.evaluateExpressionValue('(id)[[UIDevice currentDevice] model]').GetObjectDescription().lower().find('simulator') >= 0:
if (fb.evaluateExpressionValue('(id)[[UIDevice currentDevice] model]').GetObjectDescription().lower().find('simulator') >= 0) | (fb.evaluateExpressionValue('(id)[[UIDevice currentDevice] name]').GetObjectDescription().lower().find('simulator') >= 0):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This should use or instead of binary |.
  2. Maybe extract into a helper method, isSimulator?
  3. Is there a reason this isn't using evaluateObjectExpression?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will address first 2 comments
3. EvaluateObjectExpression will give us string address, while we need string python representation. expressionValue().GetObjectDescription() seems best way for that

@kastiglione
Copy link
Contributor

Thanks @gkassabli. One more comment, but otherwise looks good!

@kastiglione
Copy link
Contributor

Thank you 💎

kastiglione added a commit that referenced this pull request Nov 30, 2015
Fixing pa11y and fa11y commands for iOS8.1+
@kastiglione kastiglione merged commit 0eed26e into facebook:master Nov 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants