-
Notifications
You must be signed in to change notification settings - Fork 791
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
fix(bypass): find headings in iframes #2310
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the aforementioned accessibility errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the aforementioned accessibility errors
if (windowIsTopMatches(node)) { | ||
return !!node.querySelector('a[href]'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm kind of confused about what you are trying to accomplish here. I don't see how pages having native links is relevant to whether or not they need a mechanism for bypassing repeating content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original matches function required a page link before testing the entire page. Without one, the iframes (which don't have anchors) would not be tested for headings or landmarks. As to why we require the link in the first place, I have no idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the aforementioned accessibility errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the aforementioned accessibility errors
.github/axe-linter.yml
Outdated
- CHANGELOG.md | ||
- test/**/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a separate PR.
This reverts commit be62002.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the aforementioned accessibility errors
I needed to modify the bypass matches function to only look for anchor elements on the top level. Inside iframes we should still run and find any bypassable blocks.
The iframe tests were mostly copied from
page-has-heading-one
integration tests.Closes issue: #2105
Reviewer checks
Required fields, to be filled out by PR reviewer(s)