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

Follow-up: tree inclusion of focusable elements from #1100 #1381

Closed
cookiecrook opened this issue Jan 12, 2021 · 19 comments · Fixed by #1387
Closed

Follow-up: tree inclusion of focusable elements from #1100 #1381

cookiecrook opened this issue Jan 12, 2021 · 19 comments · Fixed by #1387
Assignees
Labels
Milestone

Comments

@cookiecrook
Copy link
Contributor

cookiecrook commented Jan 12, 2021

Regarding Issues #841 and #1234, as well as PR #1100.

Currently the accessibility tree inclusion section includes this line:

[include in the accessibility tree…] "Elements that are focusable even if the element or one of its ancestor elements has its aria-hidden attribute set to true."

I had some prior concerns about including all "focusable" elements (even if hidden) versus including hidden elements at the time they became "focused." In #841, I originally suggested:

…include something like both "visible elements that are focusable" and ~"hidden elements if/when they become focused."

@carmacleod waited on @aleventhal and me to resolve this comment in PR #1100:

@aleventhal Are you good with this? Does it need to be more specific about whether or not it is exposed to the tree at the time that the element is focused?

And then I deferred and approved the PR:

We can address that in a follow-up issue. I don't think it needs to delay this one... Marking PR as Reviewed.

But I can't see that I filed the "follow-up issue" and don't see it linked... So filing this new one for more discussion.

This is coming up now because the implementation of this change in WebKit will cause a number of layout tests to fail... I think these are probably expected and can be updated, but I wanted to verify with @aleventhal, @joanmarie, and @carmacleod before it ships. IIRC the discussion from April 2020 correctly, the WG came to the consensus that the language "focusable" was intended, not the "only when focused" language originally proposed. I think that's probably correct, but wanted to point out that this makes it more difficult for authors to use aria-hidden to hide larger containers.

<div aria-hidden="true">
   <p>This paragraph text is hidden, but the <a href="#">link</a> is exposed to AT out of context.</p>
</div>

In order to hide that link (or any other focusable) from AT, the web author would need to do one of the following:

  • explicitly add `tabindex="-1" on every focusable element in the "hidden" section (maybe this will work?)
  • make the container unrendered (e.g. hidden="" a.k.a. display:none) instead of aria-hidden
  • use inert="" on the container instead of aria-hidden

Another way to look at this, is that the ARIA change encourages incentivizes authors to use inert="" instead of aria-hidden="true"... This is probably the right call in the long term, but inert is not yet in WebKit.

Long story short... This may not be an issue. If everyone responds, "We're good", this can probably be closed.

@jnurthen jnurthen added this to the ARIA 1.3 milestone Jan 14, 2021
@cookiecrook
Copy link
Contributor Author

Assigning to Joanie for visibility... (Unassign once you respond)

@cookiecrook
Copy link
Contributor Author

cc @accdc since he mentioned he may object to shipping this spec change. Note: PR #1100 in already merged into ARIA 1.2, so I'm changing the milestone of this one back to 1.2.

@cookiecrook cookiecrook modified the milestones: ARIA 1.3, ARIA 1.2 Jan 14, 2021
@aleventhal
Copy link
Contributor

Here's what Chrome does currently for focusable aria-hidden nodes:

  • Expose it with an attribute that marks it hidden, except on Mac. We don't do this on Mac because there is no "hidden" property -- you have to either include a node or don't include it.

As I understand it, you're asking me whether I'd be willing to change the spec + Chrome to only expose these focusable aria-hidden nodes when they actually get focused. That would probably be ok. At the time that I implemented the current solution, I thought that extra thrashing of the tree during focus might be bad. But I never proved that.

In short I'm ok with changing the spec to only expose the aria-hidden node when it actually receives focus. I don't see an urgency in updating Chrome's implementation. At some point we'll circle around to all of the "repair" approaches that have been discussed for bad aria-hidden content, and I'll probably address it then.

Is anyone checking with James Teh?

@sinabahram
Copy link

Question, does this imply that when exploring with a virtual cursor, an element won't appear, but if you focus it, it will?

@aleventhal
Copy link
Contributor

@sinabahram The virtual cursor will ignore things that are marked with STATE_SYSTEM_INVISIBLE. But exposing content as invisible does enable the screen reader to work around things for pages that are known to be broken. There can be a setting or toggle that shows invisible content, etc.

It would be great if someone would test and see that my memory is correct, and that aria-hidden focusable content is in fact ignored by the JAWS/NVDA virtual buffers.

@sinabahram
Copy link

Right, so that's my expectation as well, which makes this a bit confusing because now these elements will disappear unless if focused. As a screen reader user, I'd find that pretty disconcerting.

@aleventhal
Copy link
Contributor

@sinabahram Agreed. On the other hand, is it also potentially confusing if someone uses aria-hidden with some content that's visible behind a dialog. It can't be focused with the mouse because of pointer-events:none, or the keyboard because the tab cycle is trapped. However, it will now show up in the virtual buffer. And the author can't make things like links unfocusable by tabindex=-1 either.

@sinabahram
Copy link

Sorry, but why can't the author make it unfocusable with tabindex of -1?

What's the problem this is trying to solve? It seems like this will make things more confusing for users. I know both worlds are imperfect because this is a pure authoring mistake you are having to deal with e.g. aria-hidden being applied to a focusable thing, but if they had applied aria-hidden and tabindex of -1 to it, then it is no longer an authoring error.

@aleventhal
Copy link
Contributor

@sinabahram because tabindex=-1 doesn't make things onfocusable, it only makes them untabbable.
We could decide the rule is to only expose the tabbable things, not everything focusable.

@aleventhal
Copy link
Contributor

@sinabahram But also, what a pain it is if you have a modal dialog and you need to both use aria-hidden, and then iterate through everything in the page, and apply tabindex=-1, then remove that later.

@sinabahram
Copy link

Why is any of that necessary? If it is modal, the other stuff on the page is not in there for the AT user due to aria-modal, no?

@aleventhal
Copy link
Contributor

@sinabahram Makes sense. Let's hope people remember to use aria-modal! Note that, I believe NVDA allows you to escape the dialog whether it's modal or not. There's a command for it, I'd have to look it up.

@cookiecrook
Copy link
Contributor Author

@sinabahram wrote:

Sorry, but why can't the author make it unfocusable with tabindex of -1?

@aleventhal wrote:

@sinabahram because tabindex=-1 doesn't make things onfocusable, it only makes them untabbable.

Also, even if that worked, it's a tedious and brittle developer experience to have to have a UI framework iterate through everything focusable outside its own view (potential hundreds of links, form elements, compound custom controls, elements in encapsulated shadow DOMs, etc), and then store each focusable's current tabindex (or disabled state), and then restore those elements' interactive states later.

@eps1lon
Copy link
Contributor

eps1lon commented Jan 15, 2021

I hope this is not off-topic but I'm a bit confused by the new wording.

Speaking as a component library author it seems to me we're no longer supposed to hide "inert" trees with aria-hidden? This seems ok to me in a world where browsers/AT implement a version of aria fully but as far as I know aria-modal and inert are still not widely supported.

So how would a dialog be implemented where

  • aria-modal and inert are not implemented
  • focusable elements are included in the a11y tree even if they're inside an aria-hidden tree

Or were we never supposed to mark "inert" trees as aria-hidden?

Changing it from "focusable" to "focused" would at least for us not result in requiring a new implementation since we already have a mechanism in place that prevents focus of "inert" subtrees. This sort of focus trapping is (at least in the react ecosystem) fairly popular.

@JAWS-test
Copy link
Contributor

JAWS-test commented Jan 15, 2021

It would be great if someone would test and see that my memory is correct, and that aria-hidden focusable content is in fact ignored by the JAWS/NVDA virtual buffers.

Currently, focusable content with aria-hidden=true is not output by NVDA and JAWS (when using Chrome).

With Tab the following output occurs:

  • NVDA: no output or output of "empty".
  • JAWS: the previously focused element is output (name, role, state, value).
  • NVDA and JAWS: The screenreader focus also remains on the previously focused element, i.e. instead of the actually focused element, the previously focused element can also be operated with JAWS and NVDA.

@cookiecrook
Copy link
Contributor Author

Update from today's ARIA call as I understood it.

There is wide agreement that the change in #1100 resolves a potentially serious authoring mistake, effectively forgetting to un-hide critical focusable content marked with ARIA hidden. However, there is also wide agreement that forcing all background focusables to be included in the tree is going to cause broad problems in most accessible implementations of dialogs and other modal web UI. @accdc mentioned several examples.

The two paths forward discussed were:

  1. include some additional exception, such as continuing to ignore hidden focusables if a modal state ( like aria-modal) was used... It was acknowledged that this causes further complexity in the implementation without necessarily solving the problem: e.g. if the author forgot to remove aria-hidden, they could just as easily forget to remove aria-modal... Workarounds for that become even more complex.
  2. Roll back a portion of the diff from Clarify wording in "Including Elements in the Accessibility Tree" section #1100. I think there is a mild preference for this option, and no objections were voiced in the call, but we ran out of time before fully discussing the change.

Here's the diff in question... I think the remaining diff in #1100 is fine to leave as-is.

- <li>Elements that are focusable, or have an ID <a>attribute</a> and an ancestor with the <pref>aria-activedescendant</pref> attribute that matches the implicit or explicit <a>semantics</a> of the required context role. In either case, the element may receive focus and may fire an <a>accessibility <abbr title="Application Program Interface">API</abbr></a> focus event.</li>
+ <li>Elements that are not <a>hidden</a> and may fire an <a>accessibility <abbr title="Application Program Interface">API</abbr></a> <a>event</a>:
+   <ul>
+     <li>Elements that are <a>focusable</a> even if the element or one of its ancestor elements has its <sref>aria-hidden</sref> attribute set to <code>true</code>.</li>
+     <li>Elements that are a valid target of an <pref>aria-activedescendant</pref> attribute.</li>
+   </ul>
+ </li>

@cookiecrook
Copy link
Contributor Author

cookiecrook commented Jan 22, 2021

Another possibility.

@aleventhal mentioned that that Chromium (and Firefox IIRC) both take advantage of the Windows platform Accessibility API to mark features as "in the tree but hidden" which offloads the decision to AT re: when to convey "hidden" UI. Currently that's ~only when it gets focused.

@joanmarie replied in the ARIA call today "That's possible in ATK/AT-SPI2, but Orca treats things which claim to be 'hidden' as not there."

AFAICT, AT like Windows screenreaders only ever convey these elements to users when they become focused due to to standard keyboard navigation. So the element does not appear in standard AT navigation, but the user could still interact with it at the time it became focused... This may be achievable in Mac and iOS without an API change, if the spec updated to include this diff.

-     <li>Elements that are <a>focusable</a> even if the element or one of its ancestor elements has its <sref>aria-hidden</sref> attribute set to <code>true</code>.</li>
+     <li>Elements that become <a>focused</a>, even if the element or one of its ancestor elements has its <sref>aria-hidden</sref> attribute set to <code>true</code>.</li>

Web rendering engines could update the tree when focus changes (to include the focused element as a non-hidden element), and trigger the subsequent platform focus event(s).

I think (but the WG should consider further) that this would achieve three of the four desired goals.

  1. It would prevent the UI roadblock Clarify wording in "Including Elements in the Accessibility Tree" section #1100 was intended to solve, when web authors forgot to unhide some critical piece of focusable UI.
  2. It would prevent much of (or at least some of) the user confusion regarding out-of-context focusables in aria-hidden views. e.g. There may still be confusion when using Tab navigation (see item 3), but not when using swipe navigation or other non-Tab AT navigation like VoiceOver's "VO+Right." This matches current behavior in Windows SRs.
  3. It would NOT solve user confusion related to out-of-context focusables when using Tab navigation, but that may be okay because that point of confusion exists today in Windows. This change would not make it worse.
  4. Browsers could implement this without requiring an API change (e.g. Accessibility APIs that do not support in-tree-but-hidden), and I believe it would not require AT changes either (e.g. Orca would not need to update to convey hidden content) because when the element became focused, it would no longer be marked as hidden. Obviously there is work to be done to confirm this can be implemented in a performant way.

@cookiecrook cookiecrook self-assigned this Jan 28, 2021
@cookiecrook
Copy link
Contributor Author

Matt mentioned we should specify is this "while focused" (hidden again after losing focus) or "until focused" (remains in tree if ever focused)...

My pref would be that this is hidden again after losing focus.

cookiecrook added a commit that referenced this issue Jan 28, 2021
@cookiecrook
Copy link
Contributor Author

PR #1387

@carmacleod carmacleod linked a pull request Jan 29, 2021 that will close this issue
jnurthen pushed a commit that referenced this issue Feb 4, 2021
* focusable->focused to resolve #1381
jnurthen pushed a commit that referenced this issue Feb 10, 2021
* focusable->focused to resolve #1381
jnurthen pushed a commit that referenced this issue Feb 10, 2021
* focusable->focused to resolve #1381
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants