Skip to content

Conversation

@gkassabli
Copy link
Contributor

Added print accessibility tree command that is based on Apple internal methods. It could print whole tree or subtree of specified root element.
Find accessibility element command was reworked to use internal API and make use of only relevant and visible elements

@KingOfBrian
Copy link

This is fantastic. Great finds on the AX stuff!

Choose a reason for hiding this comment

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

It would be helpful to add the ability to print the accessibility value and traits if possible. If you had the values for them, adding an optional second argument to pa11y would be pretty easy.

@gkassabli
Copy link
Contributor Author

Hey, @KingOfBrian.
That's a great idea. I haven't looked into that, but I will try to add this in next iteration of command

@kastiglione
Copy link
Contributor

@KingOfBrian How does this pull request compare / contrast to #55?

@KingOfBrian
Copy link

Much better approach, with fewer round trips by a large margin. The different types of AX values can be added pretty easily. forceStartAccessibilityServer is awesome, I'm gonna add that to FRY to enable AX.

I've never determined traversal choices via a nil accessiblityLabel, and I think it may cut the tree off a bit early. A UITableView accessibilityLabel for instance is used to store the empty table view AX value, and without actually testing, I'm not sure how this would work. There are other instances, like a navigation bar item where the accessibilityLabel is duplicated in an internal view. It's not important to pick up that distinction, but for a debug tool, I think it'd be better to be more liberal. And, some magic inside of _accessibilityElementsInContainer may behave differently than I think.

Anyway, this approach is definitely the correct one. I don't have time to test it, as my AX work has passed for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems this will always be true, since foundLocally will always be false in this elif branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. Did it correct way initially, but refactored wrong way

Copy link
Contributor

Choose a reason for hiding this comment

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

Were you intending to rectify this in 55cca66?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this should work properly now

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

a2 added a commit that referenced this pull request Feb 4, 2015
Added print accessibility tree command. Find accessibility element command reworked
@a2 a2 merged commit 167dc38 into facebook:master Feb 4, 2015
@kastiglione
Copy link
Contributor

Thanks @a2, and apologies @gkassabli for the dropped ball. I had asked a question and was waiting to hear back, but perhaps it wasn't seen.

@gkassabli
Copy link
Contributor Author

@kastiglione Yeah, I fixed it there. Apologies for not responding in thread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants