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.

Sign up for GitHub

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

Jump to bottom

Do not compute accessible role and name for non-Element nodes in the accessibility locator #702

Merged
merged 3 commits into from
May 7, 2024
Merged

Do not compute accessible role and name for non-Element nodes in the accessibility locator #702

merged 3 commits into from
May 7, 2024

Conversation

OrKoN
Copy link
Contributor

@OrKoN OrKoN commented Apr 24, 2024 •
edited by pr-preview bot

This PR updates the accessibility locator algorithm to avoid getting accessible name and role on non-element nodes while allowing to descend into non-element nodes' subtrees.

Preview | Diff

@juliandescottes juliandescottes mentioned this pull request Apr 24, 2024
Closed
OrKoN
OrKoN commented Apr 24, 2024
View reviewed changes
index.bs Show resolved Hide resolved
OrKoN
OrKoN commented Apr 24, 2024
View reviewed changes
index.bs Outdated Show resolved Hide resolved
jgraham
jgraham previously requested changes Apr 26, 2024
View reviewed changes
index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
sadym-chromium
sadym-chromium reviewed Apr 29, 2024
View reviewed changes
index.bs Outdated Show resolved Hide resolved
sadym-chromium
sadym-chromium reviewed Apr 29, 2024
View reviewed changes
index.bs Outdated Show resolved Hide resolved
@OrKoN
Copy link
Contributor Author

OrKoN commented Apr 30, 2024

I am gonna split the changes required for a11y locator from changes to innerText (which requires more changes)

@OrKoN OrKoN changed the title fix: check for Elements in accessibility and innerText locators fix: check for Elements in the accessibility locator Apr 30, 2024
@OrKoN OrKoN changed the title fix: check for Elements in the accessibility locator Do not computed role and name for non-Element nodes in the accessibility locator Apr 30, 2024
@OrKoN OrKoN changed the title Do not computed role and name for non-Element nodes in the accessibility locator Do not compute accessible role and name for non-Element nodes in the accessibility locator Apr 30, 2024
@OrKoN OrKoN requested a review from jgraham April 30, 2024 06:14
@whimboo whimboo requested a review from lutien May 6, 2024 13:55
whimboo
whimboo approved these changes May 6, 2024
View reviewed changes
Copy link
Contributor

@whimboo whimboo left a comment

Choose a reason for hiding this comment

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

Looks fine to me but maybe @lutien can have a look as well by comparing it to our implementation in Firefox if it works as expected.

lutien
lutien approved these changes May 6, 2024
View reviewed changes
Copy link
Contributor

@lutien lutien left a comment

Choose a reason for hiding this comment

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

Looks good to me as well

@OrKoN OrKoN dismissed jgraham’s stale review May 7, 2024 07:17

changes has been applied

@OrKoN OrKoN merged commit e74d46a into main May 7, 2024
5 checks passed
@OrKoN OrKoN deleted the orkon/a11y-element branch May 7, 2024 07:41
github-actions bot added a commit that referenced this pull request May 7, 2024
@OrKoN @github-actions
Do not compute accessible role and name for non-Element nodes in the …
948c48f 
…accessibility locator (#702)

SHA: e74d46a
Reason: push, by OrKoN

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@OrKoN OrKoN mentioned this pull request May 7, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimevans jimevans jimevans left review comments

@sadym-chromium sadym-chromium sadym-chromium left review comments

@whimboo whimboo whimboo approved these changes

@jgraham jgraham jgraham approved these changes

@lutien lutien lutien approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@OrKoN @whimboo @jgraham @jimevans @lutien @sadym-chromium