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

aria-hidden error case: remove attribute aria-hidden=true on elements that become focused, and any ancestor of the element that is currently focused. #1255

Open
cookiecrook opened this issue May 1, 2020 · 22 comments
Assignees
Labels
feature may add new concept(s) to ARIA which will require implementations or APG changes
Milestone

Comments

@cookiecrook
Copy link
Contributor

cookiecrook commented May 1, 2020

The WG discussed a variety of author error cases we should formalize to provide a better end-user experience for assistive technology users.

An anti-pattern we see frequently is an element hidden (via aria-hidden) getting focused (via Tab navigation or script). Browser accessibility and AT response to this varies, but it’s never ideal.

Suggestion was that, at the time any element acquires focus, if the element is hidden from the accessibility tree via aria-hidden="true" the user agent should remove any instances of aria-hidden="true" on the element and any other ancestor elements. The current thinking is that these attributes should be removed from the DOM and a warning should posted to the console (console.warn()) so that it is apparent to web developers what is happening and why.

@cookiecrook
Copy link
Contributor Author

See also #1254

@aleventhal
Copy link
Contributor

@asurkov, do you agree?

@jcsteh
Copy link

jcsteh commented May 6, 2020

Question 1:

When you say ancestor elements, do you mean we would only expose the ancestor chain (not siblings of any ancestors along that chain) or the entire tree up to the top-most ancestor? For example:

<div id="a" aria-hidden="true">
  <p id="b">b</p>
  <button id="c">c</button>
</div>
<script>
c.focus();
</script>

Does "b" get exposed? Based on the suggestion of removing aria-hidden from the DOM, I'm guessing the answer is yes, which is good... but that needs to be made super clear. If the answer is no, I have big problems with this; see #1185 (comment).

Question 2:

If the element loses focus, do we hide it again? Again, based on the DOM mutation suggestion, I'm guessing no, which is good... but again, if the answer is yes, I have a problem with this, since we'd be constantly rebuilding and throwing away parts of the tree.

Question 3:

What about the Google Groups case (maybe this is gone now?) where we have something like:

<div role="button" aria-label="blah"><input type="text" aria-hidden="true"></div>

(where the input is visually hidden)

My understanding is that this was one of the reasons Google was originally pushing for hidden/presentational focusable content to be unconditionally pruned from the tree. Screen readers even had to make changes to handle that case correctly.

@aleventhal
Copy link
Contributor

@jcsteh
Question 1:
"b" gets exposed because aria-hidden is removed from the DOM. I didn't see the need for clarification because once aria-hidden is removed, that will naturally happen.
Question 2:
No, element is not hidden again once it loses focus
Question 3:
If the user manages to focus the input, e.g. by tabbing, then aria-hidden will be removed from it. Depending on how the input is visually hidden, it may be impossible to tab to it or otherwise focus it.

@vicfei-ms
Copy link

Chime in from Microsoft here, I'm not supportive of this runtime behavior overriding aria-hidden. It will be a nightmare to debug. I'm more in favor of simplifying aria-hidden more and to make it more predictable, even at the expense of some existing content visibility :)

@aleventhal
Copy link
Contributor

@vicfei-ms please suggest an alternative solution to the "401k problem". This is the problem where users are not able to access extremely important pages that misuse aria-hidden, and there are no workarounds available.

@charmarkk charmarkk added this to the ARIA 1.3 milestone May 7, 2020
@vicfei-ms
Copy link

vicfei-ms commented May 7, 2020

@aleventhal
Using bad authoring practices as a rationale to change behavior of a property can be a double edged sword – it would make it harder to rely on this property, but also hurts authors that legitimately use the property and would be confused trying to debug why something is not working.
I personally think there are already too many knobs for hiding elements, it would be better if aria-hidden can simplified :)
As for the "401k problem" I agree with you it can be an issue, but I don't think we should bend the standards for it.

@aleventhal
Copy link
Contributor

@vicfei-ms No one has a perfect solution ... Would you be willing to write up your proposal so we can compare the pros and cons? :)

@sinabahram
Copy link

@vicfei-ms Can you please enumerate a valid use case that this proposal would then render invalid since you indicated:

"... hurts authors that legitimately use the property and would be confused trying to debug why something is not working"

That pasted clause implies a valid use case being now rendered "not working". Please present one or an alternative solution to help us understand what valid use case we would make more difficult to debug?

It is my assertion that it is never appropriate for a focussed item to have aria-hidden=true on it, full stop.

@sinabahram
Copy link

@jcsteh do @aleventhal's answers to your questions address your remaining concerns on this issue?

@jcsteh
Copy link

jcsteh commented Jun 10, 2020

@jcsteh do @aleventhal's answers to your questions address your remaining concerns on this issue?

Yes, except for one thing:

Question 3:
If the user manages to focus the input, e.g. by tabbing, then aria-hidden will be removed from it. Depending on how the input is visually hidden, it may be impossible to tab to it or otherwise focus it.

My understanding of the Google Groups case was that it was focusing the (hacky) input, but the intention was that because the input was aria-hidden, focus would land on the role="button" ancestor. That would break with this proposal. Personally, I always thought this was a terrible implementation anyway, but I know it was something Google was doing in the wild, so I thought it worth raising. I'm not sure if this is still being done in the wild.

@sinabahram
Copy link

That is a pretty clear authoring violation, though, right? We can't improve anything, full stop, ever in the future of the universe, if we are binding ourselves both by good previous code and bad previous code. It's actually mathematically impossible to proceed at that point, as it covers the entire set of possible code, so am I missing something, or are we basically good to go on this issue? Also @aleventhal perhaps you can poke the appropriate internal teams to resolve that use case; thereby completely addressing it?

@jcsteh
Copy link

jcsteh commented Jun 10, 2020

That is a pretty clear authoring violation, though, right?

According to the current spec, I'm not sure it is. I don't see anything in the current spec which explicitly prohibits aria-hidden on a focusable element. And therein lies a backwards compatibility problem: we're breaking things that were acceptable - they weren't violations- according to the existing spec.

@sinabahram
Copy link

Ok, fine, let me ask it differently. We have tons of bad things that happen when aria-hidden="true" is used on a focusable element. Can you list a single bad thing that occurs if it is disallowed now? I hear you, but specs are written by humans. Humans make mistakes. All we can do is fix it going forward. We must fix this, right?

@jcsteh
Copy link

jcsteh commented Jun 10, 2020 via email

@aleventhal
Copy link
Contributor

Where is this Google Groups aria-hidden input? 🤮

@jcsteh
Copy link

jcsteh commented Jun 10, 2020

I vaguely recall this was Google Groups, but it seems it might have actually been the Google Apps Control Panel and role="presentation", not aria-hidden. Similar issue, but separate to this. See nvaccess/nvda#3781.

However, I went back through my email archives and I did find a conversation with Google engineers in 2014 about Google Sheets. Back then (when braille mode didn't exist), in Google Sheets, when focused in the spreadsheet (but not
editing), a textarea with aria-hidden="true" had focus. This was causing NVDA to try to treat the spreadsheet as an editable text control in Firefox and IE, since Firefox and IE didn't prune aria-hidden trees back then. That has since been changed, so that particular case is theoretical now. However, quoting my email:

Dominic has raised this issue before in discussions about
aria-hidden/role presentation in Firefox. He argued that where this
occurs, the parent element should get focus instead with respect to
accessibility.

So, I think it might be worth bringing Dominic (@minorninth) in on this one.

@vicfei-ms
Copy link

vicfei-ms commented Jun 12, 2020

@sinabahram

@vicfei-ms Can you please enumerate a valid use case that this proposal would then render invalid since you indicated:

"... hurts authors that legitimately use the property and would be confused trying to debug why something is not working"

That pasted clause implies a valid use case being now rendered "not working". Please present one or an alternative solution to help us understand what valid use case we would make more difficult to debug?

It is my assertion that it is never appropriate for a focussed item to have aria-hidden=true on it, full stop.

By the above I mean consider a simple case like the following:
<button aria-hidden="true">hide me</button>
The above case is a fairly common practice if an author wants to hide the button from ATs. But if we were to go with the spec change proposal (when an element gains focus and aria-hidden gets removed), then to hide the button from AT, the author would also have to keep in mind to make sure the button is not focusable.
<button aria-hidden="true" tabindex="-1">hide me</button>. So for something simple such as hiding an element from ATs, the author will also have to find out if an element could gain focus as well... this is simply too many knobs to turn..
An author could naturally be assuming that if visibility:hidden hides the button from AT regardless if focus is set, why should aria-hidden=true not work?
<button style="visibility:hidden">hide me 2</button>
Yes I do agree that there are cases where when an element gains focus we want to override aria-hidden, but there are just as many existing sites where authors just expect aria-hidden=true to work regardless focus.

@vicfei-ms
Copy link

vicfei-ms commented Jun 12, 2020

@aleventhal

@vicfei-ms please suggest an alternative solution to the "401k problem". This is the problem where users are not able to access extremely important pages that misuse aria-hidden, and there are no workarounds available.

Regarding the 401k problem and the separate example you gave me, where ATs expect to announce content in role="dialog". Here are my thoughts, but may be far fetched:
What if we allow aria-hidden="false" in the spec and advocate authors to use it correctly, do you think that will reduce the scenarios where authors expect focus to override aria-hidden="true"? It seems to me in the case below and perhaps other cases where if the aria-hidden="false" is supported and used explicitly by the author, there wouldn't be a need for "focus overrides aria-hidden=true behavior".

<body aria-hidden="true">
<div role="dialog">   <!-- Some examples put aria-hidden="false" here, which does nothing -->
<fieldset>
  <legend>Cat info</legend>
  <label for="in">Name of cat:</label> <input id="in">
  <input type="checkbox" id="cbox"><label for="cbox">I like my cat</label>
</fieldset>
<button>Submit</button>
</div>
</body>

@sinabahram
Copy link

@vicfei-ms you said:
"hide me"

That should never happen, full stop. That button does not also have tabindex=-1, so it will result in exactly the problem we are trying to prevent. That is not a valid use case. That is an invalid use case that I have absolutely no problem breaking. At least you'll know, as a screen reader user, what you've landed on, instead of landing on "blank" and having no recourse what-so-ever.

@JamesCatt
Copy link

I think this is a great idea, provided that this is done:

...a warning should posted to the console (console.warn()) so that it is apparent to web developers what is happening and why.

In fact, I think it would be great if browsers did more of this. A lot of common mistakes could be eliminated simply by surfacing them to developers. I don't mean to sidetrack or self-promote, but I wrote a blog post on exactly that.

Would there be a possible issue here for NVDA, given that it moves keyboard focus along with the cursor? Specifically I'm thinking if you had aria-hidden set on an ancestor while a dialog is open—javascript can be used to prevent the user focusing elements outside the dialog via TAB, but cannot prevent NVDA from moving the cursor.

@alia11y alia11y pinned this issue Apr 15, 2021
@jnurthen
Copy link
Member

jnurthen commented May 6, 2021

unless anyone wants to be assigned moving to 1.4

@jnurthen jnurthen modified the milestones: ARIA 1.3, ARIA 1.4 May 6, 2021
@cyns cyns unpinned this issue Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature may add new concept(s) to ARIA which will require implementations or APG changes
Projects
None yet
Development

No branches or pull requests

9 participants