-
Notifications
You must be signed in to change notification settings - Fork 66
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
Update rule to address assistive technology inconsistencies #1384
Conversation
Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
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 looks good - it addresses everything I reported in in #1367
@@ -21,7 +21,7 @@ acknowledgments: | |||
|
|||
## Applicability | |||
|
|||
The rule applies to `iframe` elements that are [included in the accessibility tree][] and that can be accessed by [sequential focus navigation][]. | |||
The rule applies to `iframe` elements that are [included in the accessibility tree][]. |
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 not convinced that "included in the accessibility tree" covers it here. In trying this out, the following iframe still shows up in the accessibility tree for Chrome and Firefox:
<iframe role="none" tabindex="-1"></iframe>
Even if I put aria-hidden="true" on it, Chrome still puts it in the accessibility tree (Firefox does not). Common ways of taking things out of the accessibility tree don't seem to work for iframes. I think that's something the rule should account for.
The other thing I noticed is that despite that iframes with role="none" are included in the accessibility tree, Voiceover treats them completely differently. They're not treated as regions to enter into, but instead treated as part of the regular content. That makes a lot of sense, but does away with this idea that whether or not it is included in the accessibility tree matters.
That example above... despite the fact that very unintuitively it is included in the accessibility tree, doesn't seem like it actually needs an accessible name. VO skips it because of role=none, NVDA skips it because of tabindex="-1". I'm not sure what JAWS does with it. Definitely something worth testing.
Either way I think we should explore doing this rule without "included in the accessibility tree". Accessibility trees seem to have unique rules for iframes, which as far as I can tell aren't spec'ed out. Instead of hoping someone does enough testing to figure it out, we should probably define what we found in this rule so that everyone can do this consistently.
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.
To add to above findings, for above example:
NVDA/Firefox - has iframe, can get to it and reads "about:blank".
JAWS/IE11 - does not see the iframe
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 argument is very valid, given the disparity in how iframe
is interpreted by various AT's.
Given this context, I have updated this rule to not use included in accessibility tree
, which is the step in the right direction.
Added a generic accessibility note, but open to expanding on that if necessary.
@@ -21,7 +21,7 @@ acknowledgments: | |||
|
|||
## Applicability | |||
|
|||
The rule applies to `iframe` elements that are [included in the accessibility tree][] and that can be accessed by [sequential focus navigation][]. | |||
The rule applies to `iframe` elements. |
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.
Why would display:none
need to be tested for this?
Co-authored-by: daniel-montalvo <49305434+daniel-montalvo@users.noreply.github.com>
@WilcoFiers as discussed are you taking over this one? |
Closing this as @WilcoFiers is taking it over. |
Addressed issues identified, detailed in below issue.
Closes issue(s):
Need for Final Call: