Skip to content

Conversation

carlosapaduarte
Copy link
Member

<< Describe the changes >>
#643 identifies a situation where an element cannot be focused (because of scripts) but would still fail the rule
#1386 identifies a situation where an element is not in the sequential focus navigation but can still be focused with a click or touch event

This PR updates the expectation so that now test targets cannot be focused through any mechanism, fixing the 2 issues.
Examples were updated in accordance. Example descriptions were also updated.

Closes issue(s):

Need for Final Call:
This will require a 2 weeks Final Call


Pull Request Etiquette

When creating PR:

  • Make sure you're requesting to pull a branch (right side) to the develop branch (left side).

After creating PR:

  • Add yourself (and co-authors) as "Assignees" for PR.
  • Add label to indicate if it's a Rule, Definition or Chore.
  • Link the PR to any issue it solves. This will be done automatically by referencing the issue at the top of this comment in the indicated place.
  • Optionally request feedback from anyone in particular by assigning them as "Reviewers".

When merging a PR:

  • Close any issue that the PR resolves. This will happen automatically upon merging if the PR was correctly linked to the issue, e.g. by referencing the issue at the top of this comment.

How to Review And Approve

  • Go to the “Files changed” tab
  • Here you will have the option to leave comments on different lines.
  • Once the review is completed, find the “Review changes” button in the top right, select “Approve” (if you are really confident in the rule) or "Request changes" and click “Submit review”.
  • Make sure to also review the proposed Final Call period. In case of disagreement, the longer period wins.

@carlosapaduarte carlosapaduarte added the Rule Update Use this label for an existing rule that is being updated label Sep 9, 2020
@carlosapaduarte carlosapaduarte self-assigned this Sep 9, 2020
Copy link
Contributor

@kasperisager kasperisager left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Jym77
Jym77 previously requested changes Sep 10, 2020
Copy link
Collaborator

@Jym77 Jym77 left a comment

Choose a reason for hiding this comment

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

I'm having big concerns around the testability of "can be focused" and the actual meaning of it…

## Expectation

None of the target elements are part of [sequential focus navigation](https://html.spec.whatwg.org/multipage/interaction.html#sequential-focus-navigation), nor do they have [descendants](https://dom.spec.whatwg.org/#concept-tree-descendant) in the [flat tree](https://drafts.csswg.org/css-scoping/#flat-tree) that are part of [sequential focus navigation](https://html.spec.whatwg.org/#sequential-focus-navigation).
None of the target elements can be [focused][], nor do they have [descendants](https://dom.spec.whatwg.org/#concept-tree-descendant) in the [flat tree](https://drafts.csswg.org/css-scoping/#flat-tree) that can be [focused][].
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this is the definition of :focus-within: https://drafts.csswg.org/selectors-4/#the-focus-within-pseudo

## Expectation

None of the target elements are part of [sequential focus navigation](https://html.spec.whatwg.org/multipage/interaction.html#sequential-focus-navigation), nor do they have [descendants](https://dom.spec.whatwg.org/#concept-tree-descendant) in the [flat tree](https://drafts.csswg.org/css-scoping/#flat-tree) that are part of [sequential focus navigation](https://html.spec.whatwg.org/#sequential-focus-navigation).
None of the target elements can be [focused][], nor do they have [descendants](https://dom.spec.whatwg.org/#concept-tree-descendant) in the [flat tree](https://drafts.csswg.org/css-scoping/#flat-tree) that can be [focused][].
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am a bit unsure on the formulation "can be focused".
Inlining the definition, this yields "can match the :focus pseudo-class" (or the :focus-within). Which sounds weird. And then again, the definition of :focus is "The :focus pseudo-class applies while an element has the focus (accepts keyboard or mouse events, or other forms of input)." (no further links). So "can be focused" actually means "can have the focus" (we'd better shortcut the two indirections and put that directly) and focus is not defined further…

:focus has special case for HTML (we may decide to restrict he rule to HTML) liking to focusable area… This has nice perks on inert and disable, but then again, this definition ultimately requires "[tabindex or] the element is determined by the user agent to be focusable" without further link.

So, essentially, an HTML element that "can be focused" is an element that "the UA determined as being focusable". Period. No further explanation… And I do not know if there is a way to list these elements¹.

Which does make some sense from the rule point of view as in "let the UA decide", the same way we take DOM or accessibility tree as input aspects and let the UA build them.

Note that this is fairly different from saying that an element "is [focused][]", aka "matches the :focus pseudo-class" as this is something we can easily query from the UA's API. It is perfectly determined which element matches :focus, not which elements can match it.


Also, if I take the code example from #643 (https://stackblitz.com/edit/angular-n7f6qz?file=src%2Fapp%2Fdialog-overview-example.ts) I am not sure what it means that the elements in the back "cannot match the :focus pseudo-class".
Does it mean that when I send .focus() on them, they do not match :focus (after a bit of time needed or processing)? That looks fairly hard to test.
Does it means that I cannot interact with the page in a way that they end up matching :focus? That seems not true as my interaction could close the modal and then focus the background element.

So, how do you test (auto or manual) that an element "cannot match the :focus pseudo-class"?
Especially because the interaction may change the rest of the page. For example, focusing (whether click or programmatic) the background elements may actually closes the modal and removes the aria-hidden on the background. Then, these elements are aria-hidden and can be focused, but the focus handler removes the aria-hidden and saves the day. The rule would fail because it doesn't take into account the possibility of altering the aria-hidden on focus…
(and I can fairly easily imagine cases where clicking out of the modal closes it)


Also, I'd argue that elements in the background of this example "can be focused". I'd say that the UA does "determine them as being focusable". It's not the UA that makes them non-focusable, it's the script on top of it. I would say the UA sees an input, consider that it is focusable and when it tries to .focus() it, gets hijacked by the focusin event handler on an ancestor. But the UA has no way² to actually figure out that these elements will never get focus and would consider them as focusable.


¹ Keeping this nice list here: https://allyjs.io/data-tables/focusable.html as it might help. But as far as I understand, it is generated by sending .focus() to elements and looking at the result…
² Apart from figuring out the semantic of the script, which is undecidable.

Copy link
Contributor

Choose a reason for hiding this comment

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

I personally find the use of "can match the :focus pseudo-class" to be an improvement. Yes, it's document language specific. Yes, it might be different between UAs. However, it's meaning is perfectly clear; an element is focusable if you can somehow make it match the :focus pseudo-class, of course without modifying the structure of the document. A first approach to testing this may be the Element#focus() method. Or keyboard emulation through browser automation. Or heuristics, which is the approach we would need to take in Alfa.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But Element#focus(), for example, won't be enough…

<button aria-hidden="true" onfocus="this.setAttribute("aria-hidden", "false")>Hello</button>

is aria-hidden (hence Applicable), and can be focused (thus failing the Expectation and the rule), but is not an actual problem (since the focus does clear the aria-hidden).

This is actually already a problem with the current version of the rule…

Copy link
Contributor

@kasperisager kasperisager Sep 10, 2020

Choose a reason for hiding this comment

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

That code is very much a problem though because AT will ignore the button and never move focus to it 🤔 I couldn't interact with it at all using just VoiceOver; I had to rely on tab navigation to clear the aria-hidden attribute before VoiceOver could see the button.

Copy link
Member

Choose a reason for hiding this comment

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

Counterproposal: How about instead of making this change, we add a second expectation, that the subject can not be focused through a pointer event, either because it can not be focused, because it is obscured, off screen, or pointer-events:none on it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Digging into the docs and everything is getting hairier and hairier 🙈


Our definition of focusable, or similarly, the synonyms focusable and focusable area in the specs, is essentially "let the UA decide".

Notably, specs has case of focusable areas with

the element is determined by the user agent to be focusable;

followed by example:

sometimes <a href=""> (depending on platform conventions).

And later, again a case

Any other element or part of an element determined by the user agent to be a focusable area, especially to aid with accessibility or to better match platform conventions.

with examples

A user agent could make all list item bullets sequentially focusable, so that a user can more easily navigate lists.

Similarly, a user agent could make all elements with title attributes sequentially focusable, so that their advisory information can be accessed.

See also this part of a later note confirming that UAs can act very differently…

For example, macOS users can set the user agent to skip non-form control elements, or can skip links when doing sequential focus navigation with just the Tab key (as opposed to using both the Option and Tab keys).

So, "focusable" is just something left at the choice of the UA (and https://allyjs.io/data-tables/focusable.html further confirm that it was a mess 4 years ago). This is not a problem per se (DOM tree, Accessibility tree, … are also at the choice at the UA). This is a bit more of a problem because I do not find any way to query the UA and get a list of focusable elements¹. Comparatively, it is possible to query an UA for the DOM, and (more or less) for the currently focused element (Document.activeElement: https://html.spec.whatwg.org/multipage/interaction.html#dom-documentorshadowroot-activeelement). But I do not see a way to get all the focusable elements which makes the full testability of the rule, and reproducibility of the tests, difficult.

Note that "sequential focus order" (or maybe rather "sequentially focusable" which appear to be different 🙄) is imho better as it is (at least manually) query-able simply by tabbing through the page.

At the very least, using "focusable" should only be done together with a big accessibility support warning…
(note that "focusable" is currently used in many rules, but that seems to be only a consequence of being used in notes/examples in "included in the accessibility tree" and "semantic role", so not an actual problem, but should be cleaned nonetheless imho)

¹ actually, there is something around tabindex ordered focus navigation scope and focus navigation scope which seems to be such a list but is not clear that it's query-able. Both jQueryUI and ally.js seem to solve this by having their own list of elements, possibly different from those of UAs…


I do not think that "focusable" solves #643.

Consider a case like <span tabindex="0" onfocus="this.blur(); focusNext()" aria-hidden="true"> which is essentially a stripped down version of the example there.

It has a non-null tab index, it is not a shadow host, it is not disabled, it is not inert, it is rendered. Therefore, it does match all the conditions for the first case in the table that "describes what objects can be focusable areas" (https://html.spec.whatwg.org/multipage/interaction.html#focusable-area). Being a focusable area, I do not see how it is not focusable as per our definition. So, it would be focusable and aria-hidden, and fail the rule despite not being an actual problem (and being the case we try to solve with this PR…)


What about:
Applicability: element in sequential focus order.
Expectation: when the element is :focused, it is not aria-hidden.
?

That would let go of the "progamatically prevented focus" elements like #643 because even though they are in sequential focus order, they cannot be :focused and thus the expectation would always pass (may need to be a bit clear that this is the intended behaviour of the rule, of course).

Copy link
Member

Choose a reason for hiding this comment

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

I don't know if this is a problem we need to solve Jean-Yves. Adding an assumption is always an option. Something I encountered just yesterday is that Firefox makes scrollable element focusable, and unless they have tabindex="-1" it adds them to the sequential focus navigation too.

What does need to be solved is that just because an element is focusable, does not mean it can be focused. Mark's comment is about touch. It's not possible to fire a touch / click event on an element that's off screen, or that's completely obscured by an element that takes pointer-events. I think that needs to be solved as part of this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What does need to be solved is that just because an element is focusable, does not mean it can be focused.

Which is indeed a fairly big problem given that our definition of focusable is quite literarily "can be focused"…
And the specs don't help because the definition of "focusable" there is essentially "let the UA decide". The FF example also shows that. FF decides that some more stuff should be focusable and it's perfectly allowed to do so according to specs…


And again, the example in #643, essentially <span tabindex="0" onfocus="this.blur(); focusNext()" aria-hidden="true"> is matching the HTML definition of "focusable area" (non-null tabindex, not a shadow host, not disabled, not inert, rendered), and thus is very much "focusable" as per our definition. Which in turn make "can be :focus" likely matching due to this part of CSS specs:

There may be document language or implementation specific limits on which elements can acquire :focus. For example, [HTML] defines a list of focusable areas.

which I read as "in HTML, elements that can acquire :focus are the so-called focusable areas". This span is a focusable area, so I read it as "can acquire :focus" according to CSS def…


The problem we face here (and in #643) is scripting that overrides the HTML semantics. We can't really solve it (I think) by relying on pure HTML semantics/behaviour (such as "focusable").

Copy link
Collaborator

@Jym77 Jym77 Nov 20, 2020

Choose a reason for hiding this comment

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

I think my problem with this PR is that it tries to use a static mean (HTML & CSS) to solve a dynamic problem (JS).

We essentially can't (automatically) look into the scripting (because anything becomes undecidable), so any attempt at getting a definition of "focusable" that only depends on HTML and CSS is, in my opinion, doomed to consider "focusable" elements that get their focus highjacked by scripting (barebone example being <button onfocus="this.blur()"> can be more elaborated and real life like the case in #643).

What does that leaves us? I see three OK-ish solutions:

  1. Add an assumption that scripting is not mishandling focus. This essentially means that the case in aria-hidden-focus (6cfa84): as it applied to modal content (keyboard trap and aria-modal) #643 is violating this assumption so the rule produce weird results. This is not super satisfying as real life cases like that one do exist…

  2. Have a definition of "focusable" which is more explicit about considering scripting. That essentially makes the check manual. Something like "it is possible to interact with the page in a way that results in the element to match :focus" Maybe with a time limit (e.g. need to match :focus for more than 5 seconds in a row to make sense). Maybe with a precision that the DOM tree hasn't changed (so that something behind a modal is not "focusable" because closing the modal to focus it would change the DOM).

  3. The somewhat lampshading solution I proposed above: have an expectation in the line of "when the element is :focus, it is not aria-hidden". That does hide a bit the fact that the process is slightly manual (by hiding the how to we reach a point where the element is :focus).

…not sure that's helping… 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

I went with inspiration from option 2.

  • updated the focused definition with the "interaction with the page" and the time limit
  • added an assumption to the rule to clarify that the interactions with the page should not change elements that were previously hidden to not being hidden anymore

My main question to everyone: do you find "user interacted with the page" to be objective?

@carlosapaduarte carlosapaduarte dismissed Jym77’s stale review October 8, 2020 10:13

discuss new proposal for expectation

@carlosapaduarte carlosapaduarte requested a review from Jym77 October 8, 2020 10:14
## Expectation

None of the target elements are part of [sequential focus navigation](https://html.spec.whatwg.org/multipage/interaction.html#sequential-focus-navigation), nor do they have [descendants](https://dom.spec.whatwg.org/#concept-tree-descendant) in the [flat tree](https://drafts.csswg.org/css-scoping/#flat-tree) that are part of [sequential focus navigation](https://html.spec.whatwg.org/#sequential-focus-navigation).
None of the target elements can be [focused][], nor do they have [descendants](https://dom.spec.whatwg.org/#concept-tree-descendant) in the [flat tree](https://drafts.csswg.org/css-scoping/#flat-tree) that can be [focused][].
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if this is a problem we need to solve Jean-Yves. Adding an assumption is always an option. Something I encountered just yesterday is that Firefox makes scrollable element focusable, and unless they have tabindex="-1" it adds them to the sequential focus navigation too.

What does need to be solved is that just because an element is focusable, does not mean it can be focused. Mark's comment is about touch. It's not possible to fire a touch / click event on an element that's off screen, or that's completely obscured by an element that takes pointer-events. I think that needs to be solved as part of this.

@carlosapaduarte
Copy link
Member Author

Finally removed the incorrectly placed asset

Copy link
Collaborator

@Jym77 Jym77 left a comment

Choose a reason for hiding this comment

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

Minor editorial. It's good to go for me.


## Background

Using `aria-hidden="false"` on a descendant of an element with `aria-hidden="true"` **does not** expose that element. `aria-hidden="true"` hides itself and all its content from assistive technologies.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe link to https://www.w3.org/TR/wai-aria-1.1/#aria-hidden

An element is considered hidden if it, or any of its ancestors are not rendered or have their aria-hidden attribute value set to true.

Or the better https://www.w3.org/TR/wai-aria-1.2/#tree_exclusion

The following elements are not exposed via the accessibility API and user agents MUST NOT include them in the accessibility tree:
(…)

  • Elements, including their descendants, that have aria-hidden set to true. In other words, aria-hidden="true" on a parent overrides aria-hidden="false" on descendants.

But that's ARIA 1.2 😕

@carlosapaduarte carlosapaduarte removed the request for review from jeeyyy February 4, 2022 18:20
@carlosapaduarte carlosapaduarte added the Review call 2 weeks Call for review for new rules and big changes label Feb 11, 2022
@carlosapaduarte
Copy link
Member Author

Call for reviews ends on February 25

@carlosapaduarte
Copy link
Member Author

Call for review ended. Merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review call 2 weeks Call for review for new rules and big changes reviewers wanted Rule Update Use this label for an existing rule that is being updated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants