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

feat: Add isAccessible field into element page source representation #493

Conversation

TheDarkestDay
Copy link

This is the follow-up PR for discussion raised here - appium/appium#15232.

In our use case we would like to run several accessibility checks, relying on the information provided by page source. However, it is difficult to distinguish which elements should be accessible or not (and therefore, should be checked against accessibility checks) without actually having this information inside the tree.

WebDriver allows to grab isAccessible field for individual element, on the other hand requesting this data for each element takes immense amount of time.

That's why we think that it would be great to expose this field via page source sent by /source endpoint. This is the whole purpose of this PR.

Feel free to express any thoughts or concerns.

@mykola-mokhnach
Copy link

The PR only adds this attribute to the JSON representation of the page tree. XML tree will remain unchanged. Is such behaviour expected?

@TheDarkestDay
Copy link
Author

The PR only adds this attribute to the JSON representation of the page tree. XML tree will remain unchanged. Is such behaviour expected?

No, I think this is my bad. I guess it should be available in general, without any dependency on data representation type.

@TheDarkestDay TheDarkestDay changed the title Added isAccessible field into element page source representation feat: Added isAccessible field into element page source representation Apr 19, 2021
@TheDarkestDay TheDarkestDay changed the title feat: Added isAccessible field into element page source representation feat: Add isAccessible field into element page source representation Apr 19, 2021
@mykola-mokhnach
Copy link

Some tests are still failing. Please update them. Regarding the changes in the code: LGTM

@TheDarkestDay
Copy link
Author

I am afraid I am a little bit stuck at this point. There is a job which failed with some compilation error https://dev.azure.com/AppiumCI/Appium%20CI/_build/results?buildId=14928&view=logs&j=e09f1c88-ad07-58cd-1353-eec10b5bcfad&t=f942f577-1cf3-5a58-01a8-f7ff19fbb293&l=59. Neither can I really understand what causes it nor I can get the same error locally.

Have you ever experienced something like this? Does it make any sense to try re-running this job? Or am I missing something in my changes? Sorry for this.

@mykola-mokhnach
Copy link

To me it looks like a CI glitch, which is not related to the original PR changes.

@mykola-mokhnach mykola-mokhnach merged commit 035af06 into appium:master Apr 21, 2021
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.

None yet

2 participants