Add steps for shadow roots and slots #167

nolanlawson · 2022-09-15T23:58:16Z

Fixes #20 and #93 (related: #51).

Clarifies how accessible names are calculated for shadow roots and slots.

I believe this is accurate in terms of what UAs actually do to calculate the accessible name inside of shadow roots and slots, although WPT tests are probably needed.

Implementation

WPT tests: PR, landed - wpt.fyi
Implementations:
- WebKit: n/a
- Gecko: bug
- Blink: bug, landed
Related HTML-AAM change: issue

Preview | Diff

Fixes w3c#51 and w3c#20. Clarifies how accessible names are calculated for shadow roots and slots.

nolanlawson · 2022-09-16T20:30:33Z

Just chatted with @spectranaut – the suggestion was that "displayed child nodes" is probably the wrong phrase here, and maybe "rendered child nodes" is more accurate. I'm happy with either one; naming things is hard. 🙂

jnurthen · 2022-09-22T17:28:27Z

@jcsteh can you please look at this

cookiecrook

Added some diffs so that we can re-review with @spectranaut's suggestion.

I'm not sure this addresses all of #51 since the submitter asked about aria-labelledby, which would presumably use a static element array when pointing into the shadow root. This is a great start, but an explicit mention of that new technique needed near the labelledby stage of the Name Computation?

index.html

Co-authored-by: James Craig <cookiecrook@users.noreply.github.com>

nolanlawson · 2022-09-29T18:41:31Z

I'm not sure this addresses all of #51 since the submitter asked about aria-labelledby, which would presumably use a static element array when pointing into the shadow root.

The current accname spec says:

if computing a name, and the current node has an aria-labelledby attribute that contains at least one valid IDREF […]

I think potentially there is nothing accname needs to do here. If an IDREF crosses a shadow boundary, then by definition it is not valid.

Perhaps this should go into the definition of an IDREF? I see that in the aria spec, it says:

ID reference
Reference to the ID of another element in the same document

It seems to me that this should read "in the same document or shadow root."

nolanlawson · 2022-09-29T18:55:43Z

OTOH there is the related question of what happens if a <slot> element itself has an aria-labelledby or aria-describeddby (or aria-label, for that matter).

(Previous, erroneous comment)

Whipping up a quick CodePen, and looking at the Chrome/Firefox Accessibility DevTools and VoiceOver+Safari, it looks like:

Firefox ignores the aria-labelledby
Chrome and Safari consider it a valid label

~~This feels like a bug to me (in which case, Firefox's behavior is correct and the language in my PR is correct). But I admit I'm not sure.~~

On second thought, I think my current PR language matches Chrome's and Safari's behavior – <slot>s are not treated as special, except that their assigned nodes supersede their child nodes for the purposes of traversing the DOM. Maybe this behavior is fine; it seems best not to treat <slot>s as special w.r.t. aria-labelledby/aria-label/etc.

I've opened a separate issue to track this: #173

nolanlawson · 2022-09-29T20:23:33Z

which would presumably use a static element array when pointing into the shadow root

Just grokked this point. For IDREF reflection (whatwg/html#7934) – i.e. ariaLabelledByElements, ariaDescribedByElements, and friends – it looks like yes, we need to add explicit steps for that.

I think potentially this could be handled in a separate PR, since it's not strictly about shadow DOM. E.g. with el1.ariaLabelledByElements = [el2], both el1 and el2 could be in light DOM.

spectranaut

Hi Nolan, your right this change should have WPT tests, as that is how we currently keep a list of tests for accname. Here is an example test: https://github.com/web-platform-tests/wpt/blob/master/accname/name_text-title-manual.html

Let me know if anything is not straightforward, but you might be able to get the gist of the test by looking at it and others. These test what the browser exposes in the native API, which you can see in the ATTAcomm object. You would just want to make a new file, new title, some example html (give id="test" to the element you are testing the accname of), update the expected accname string for all APIs in the ATTAcomm json.

If you write them I'll review them! :)

index.html

spectranaut · 2022-10-19T18:51:00Z

Also can you confirm, this algorithm represents what Safari and Firefox already do, but Chrome does not?

And here is the chrome bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1374358

jcsteh

This looks correct to me overall, aside from my inline question.

One of the challenges here is that this spec defines traversal in terms of the DOM. I'm not sure about other browsers, but Firefox walks the a11y tree when doing name computation, not the DOM tree, with the only exception being the case where a node isn't rendered into the a11y tree (display: none, etc.). I suspect Chrome does the same based on the slot bug, but I'm not sure about Safari. That makes the spec a little difficult to reason about from an implementation perspective. We don't have specific code to deal with slots in our name computation code because slots aren't rendered into the a11y tree at all. I suspect Chromium will need a similar fix. This also raises questions about how aria-owns is handled. Oof.

I guess we'd need a separate a11y tree traversal spec to deal with that, at which point we'd reference that from the AccName spec in terms of how we traverse. We don't have that yet, so I guess we'll need to talk in terms of the DOM for now.

index.html

jcsteh · 2022-10-20T06:21:06Z

index.html

-                    <li id="step2F.iii.b">Compute the text alternative of the  <code>current node</code> beginning with step 2.  Set the <code>result</code> to that text alternative.</li>
-                    <li id="step2F.iii.c">Append the <code>result</code> to the <code>accumulated text</code>. </li>
+                    <li id="step2F.iii.a">If the <code>current node</code> has an attached [=shadow root=], set the <code>rendered child nodes</code> to be the child nodes of the [=shadow root=].</li>
+                    <li id="step2F.iii.b">Otherwise, if the <code>current node</code> is a [=slot=] with [=slot/assigned nodes=], set the <code>rendered child nodes</code> to be the [=slot/assigned nodes=] of the <code>current node</code>.</li>


I might well be missing something - this spec still really boggles my brain - but what excludes the <slot> itself from being processed? The Chromium bug you filed is the precise example of this. We'll process the label as an idref, start walking its descendants as a result of 2h, recurse into the div, recurse to the slot as one of the child nodes of the div starting at step 2, and end up using the aria-label on the slot in 2d.

My intention is to tweak the children-traversal part of the algorithm to traverse into slot assigned nodes rather than child nodes. The slot itself is not excluded from processing; you're correct.

Based on w3c/html-aam#440 it looks like this should be handled in the html-aam spec instead?

The slot itself is not excluded from processing; you're correct.

If I understand correctly, that means the Chromium bug isn't dealt with by this addition to the spec, correct? I'm not saying this isn't necessary, just clarifying for sure that it doesn't address the Chromium bug.

it looks like this should be handled in the html-aam spec instead?

This gets back to my comment about tree traversal. As I see it, it's less to do with the fact that a slot can't be named and more to do with the fact that the slot itself (but not its rendered children) should be skipped in the traversal in the first place. I realise this might sound like nitpicking, but I think it's important in understanding how browsers actually do this.

If I understand correctly, that means the Chromium bug isn't dealt with by this addition to the spec, correct?

Correct, it's unrelated.

As I see it, it's less to do with the fact that a slot can't be named and more to do with the fact that the slot itself (but not its rendered children) should be skipped in the traversal in the first place.

That makes sense to me! I would ask @scottaohara if this is similar to other "HTML elements which do not support naming," or if <slot> is a special case?

To add another wrinkle: the default UA stylesheet for <slot> has display: contents, but this can be overridden by the web author. So I'm not sure of the "namelessness" of slots is due to their being slots, or due to display: contents.

Edit: apparently display: contents should not affect the accessibility tree, but today it does? So it may be irrelevant to the <slot> discussion.

Oh blah. You're right: it's probably the fact that it has display: contents that excludes it from the Firefox a11y tree. As your test case shows, it does get exposed with display: block (because block elements have some semantic value by virtue of being block). That means that <slot aria-label="foo"> probably would get exposed in Firefox except for this bug.

So it does look like we'll need some explicit provision that slots shouldn't be exposed. I'm not sure if them not having a name is enough. That said, this is a bit tricky because <slot style="display: block;" tabindex="0"> does become focusable.

Thanks, that makes sense! To identify this bug, we could try adding WPT tests that set display: block on the <slot>.

That said, this is a bit tricky because <slot style="display: block;" tabindex="0"> does become focusable.

Well, maybe <slot> should be an element that can support naming? Unless #173 (comment) is the last word on this.

Note that I don't have a strong opinion; I'm just trying to determine the correct behavior.

In the case someone does <slot style="display: block;" tabindex="0">, we've gotten quite into the territory of author error correction.

If properly used, the slot element, along with elements like title, meta, template, etc... none of those should ever actually be exposed to a user and they should not be nameable. But, if you force these elements to be rendered by changing their CSS display property, then they essentially become 'generic' elements.

I think it makes sense to call out that if an author forces elements like these to be exposed to the a11y tree, then browsers should handle them like generics. But, again, I would stress this is correcting for author misuse and otherwise - if used properly - they cannot be named.

I think it makes sense to call out that if an author forces elements like these to be exposed to the a11y tree, then browsers should handle them like generics.

@scottaohara Thanks for the feedback! Do you think this should be done in accname or html-aam? I'm wondering if it makes more sense to put it in html-aam (per w3c/html-aam#440), since in accname we're merely defining the tree traversal algorithm w.r.t <slot>s and their assigned nodes, not whether <slot>s can be named.

yes, i think this should be handled in html aam per the linked issue. i don't think accName should need to call this particular element out in any specific way. that's is what html aam can do.

nolanlawson · 2022-10-20T15:18:22Z

@spectranaut

Also can you confirm, this algorithm represents what Safari and Firefox already do, but Chrome does not?
And here is the chrome bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1374358

Correct, based on my testing (#173), Safari and Firefox ignore the aria-label on the <slot>, whereas Chrome does not.

Based on w3c/html-aam#440 though, it seems like we want to cover "elements that cannot be named" (e.g. <slot>) outside of the accname spec, in the html-aam spec instead. So I didn't handle it in this PR.

FWIW I did also open a PR to add some basic accname tests for shadow DOM and slots: web-platform-tests/wpt#36541 . It broadly covers what this PR covers (although not aria-description).

rniwa · 2022-10-20T20:20:07Z

As discussed during AOM meeting, it's important that there is no JS API that exposes the computed accessibility names that emanate out of a shadow DOM since that would violate shadow DOM's encapsulation. And so far we've concluded there is none so we're good here.

cookiecrook

This looks okay to me now. Sorry I didn't get back to it after my initial review, but I agree my feedback has been addressed.

Of note, @nolanlawson, this will be testable in wpt/accname/name/comp_name_from_content.html once WPT #39604 lands. Please update the ReadMe in that same dir if you're the one making the change.

nolanlawson · 2023-04-21T19:12:20Z

@cookiecrook No worries, thank you for the review!

jnurthen · 2023-04-21T19:14:01Z

@spectranaut are your review comments addressed?

spectranaut

looks good to me!

cookiecrook

Need to move the name values back to their original steps. New numbered name values should not be added; only named id values like id="comp_something_logical_and_unique_to_this_step_even_if_it_is_moved".

cookiecrook · 2023-05-08T23:12:05Z

index.html

-                    <li id="comp_name_from_content_for_each_child_set_current" name="step2F.iii.a">Set the <code>current node</code> to the child node.</li>
-                    <li id="comp_name_from_content_for_each_child_recursion" name="step2F.iii.b">Compute the text alternative of the  <code>current node</code> beginning with the overall <a href="#comp_computation">Computation</a> step. Set the <code>result</code> to that text alternative.</li>
-                    <li id="comp_for_each_child_append" name="step2F.iii.c">Append the <code>result</code> to the <code>accumulated text</code>. </li>
+                    <li id="comp_name_from_content_find_child_has_shadow_root" name="step2F.iii.a">If the <code>current node</code> has an attached [=shadow root=], set the <code>rendered child nodes</code> to be the child nodes of the [=shadow root=].</li>


Repurposing the "step2F.iii.a" here breaks the old permalinks. We kept those so that legacy deep links in would not break, but the legacy name value should stay with the original reference (comp_name_from_content_for_each_child_set_current) and no new numbered name values should be added.

New permalinks should used uniquely names id values only.

No problem. I changed it so that:

New content has its own convention for the name

Pre-existing content (comp_name_from_content_for_each_child) has the same name as before

GitHub doesn't do a great job with the diff, but permalinks should not be broken anymore.

cookiecrook · 2023-05-08T23:14:41Z

index.html

-                <li id="comp_name_from_content_return" name="step2F.iv">Return the <code>accumulated text</code> if it is not the empty string ("").</li>
+                <li id="comp_name_from_content_for_each_child" name="step2F.iv"><em>Name From Each Child:</em> For each <code>rendered child node</code> of the <code>current node</code>:
+                  <ol>
+                    <li id="comp_name_from_content_for_each_child_set_current" name="step2F.iv.a">Set the <code>current node</code> to the <code>rendered child node</code>.</li>


Suggested change

<li id="comp_name_from_content_for_each_child_set_current" name="step2F.iv.a">Set the <code>current node</code> to the <code>rendered child node</code>.</li>

<li id="comp_name_from_content_for_each_child_set_current" name="step2F.iii.a">Set the <code>current node</code> to the <code>rendered child node</code>.</li>

Perhaps we should even add comments like above so this is less likely to happen in the future

Sounds good. I did add a comment just to be safe:

accname/index.html

Line 442 in a9cfa2e

cookiecrook

Substantive change looks good, but editorial nits included.

index.html

Co-authored-by: James Craig <cookiecrook@users.noreply.github.com>

Jym77 · 2023-09-07T12:31:41Z

(semi-randomly finding that now)
Isn't this trying to redefine the flat tree of CSS?
Shouldn't we just change step 2.F.iii from

For each child node of the current node

to

For each child node of the current node in the flat tree

that should already pretty much take care of all the shadow DOM magic 🤔

nolanlawson · 2023-09-18T16:57:29Z

@Jym77 I wasn't even aware that there was a concept of a "flattened tree." The link you provide is from the CSS spec – if there's something in the HTML spec, maybe we could reference it here to simplify the logic?

nolanlawson · 2023-09-18T16:58:41Z

BTW the WPT tests for this PR have already been merged: web-platform-tests/wpt#36541

Maybe we can merge and then refactor later as necessary?

Jym77 · 2023-09-20T13:46:01Z

@Jym77 I wasn't even aware that there was a concept of a "flattened tree." The link you provide is from the CSS spec – if there's something in the HTML spec, maybe we could reference it here to simplify the logic?

I'm not aware of anything like that in HTML (or DOM) specs. I agree it would be better to have this definition there rather than in CSS... From what I gather, CSS needs the concept for inheritance (second paragraph), but DOM or HTML never directly need that concept 😞
In ACT rules, we've been using "flat tree" with link to CSS for some times, e.g. in the Applicability of Text has minimum contrast (and many other rules).

jcsteh · 2023-09-20T23:08:54Z

Realistically, accessibility implementations depend on layout rendering to some extent; e.g. display: none, visibility: hidden. Given that, it's probably not unreasonable to refer to a CSS concept here. FWIW, Gecko's accessibility engine internally uses the flat tree to build the accessibility tree and I assume other browsers might do similar.

alice · 2023-09-20T23:35:03Z

Gecko's accessibility engine internally uses the flat tree to build the accessibility tree and I assume other browsers might do similar.

Blink does the same (technically the "layout tree builder traversal" which includes pseudo-elements, but otherwise just uses the flat tree traversal), since the whole display: contents incident. (Previously it walked the layout tree directly, which mostly amounts to the same thing anyway.)

MelSumner · 2024-02-08T18:35:52Z

I am not sure what we want to do about this so I'm going to mark it for the agenda, and read through it all in the meantime.

spectranaut · 2024-02-15T18:54:12Z

This looks read to land, @jnurthen will resolve conflicts

SHA: 67ca225 Reason: push, by spectranaut Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Add steps for shadow roots and slots

cf152ed

Fixes w3c#51 and w3c#20. Clarifies how accessible names are calculated for shadow roots and slots.

jnurthen requested review from aleventhal and cyns September 22, 2022 17:27

jnurthen requested a review from cookiecrook September 22, 2022 17:28

cookiecrook requested changes Sep 29, 2022

View reviewed changes

nolanlawson and others added 7 commits September 29, 2022 11:03

Update index.html

3007834

Co-authored-by: James Craig <cookiecrook@users.noreply.github.com>

Update index.html

852dd64

Co-authored-by: James Craig <cookiecrook@users.noreply.github.com>

Update index.html

8020dce

Co-authored-by: James Craig <cookiecrook@users.noreply.github.com>

Update index.html

37d4048

Co-authored-by: James Craig <cookiecrook@users.noreply.github.com>

Update index.html

fd225a3

Co-authored-by: James Craig <cookiecrook@users.noreply.github.com>

Update index.html

afd6ea5

Co-authored-by: James Craig <cookiecrook@users.noreply.github.com>

add language about slot representing

95782ce

nolanlawson mentioned this pull request Sep 29, 2022

Add steps for ariaLabelledByElements and ariaDescribedByElements #170

Closed

nolanlawson mentioned this pull request Oct 7, 2022

How should <slot aria-label="foo"> be treated? #173

Closed

nolanlawson mentioned this pull request Oct 18, 2022

Add basic accname tests for shadow DOM web-platform-tests/wpt#36541

Merged

spectranaut requested changes Oct 19, 2022

View reviewed changes

index.html Outdated Show resolved Hide resolved

jcsteh reviewed Oct 20, 2022

View reviewed changes

fix: fix roman numerals

442df9a

fix: remove comment

8d17a28

MelSumner requested review from MelSumner and accdc October 31, 2022 18:23

chore: fix whitespace

51c6b09

cookiecrook approved these changes Apr 20, 2023

View reviewed changes

spectranaut approved these changes Apr 21, 2023

View reviewed changes

cookiecrook mentioned this pull request May 2, 2023

TAC and #presentation should cover shadow-DOM descendants (e.g. <video controls> in some user agents) #20

Closed

spectranaut mentioned this pull request May 5, 2023

Accessibility Parent/Child follow ups w3c/aria#1930

Closed

cookiecrook requested changes May 8, 2023

View reviewed changes

nolanlawson added 3 commits May 26, 2023 15:50

Merge remote-tracking branch 'origin/main' into shadow-dom

ecf6cb4

Merge remote-tracking branch 'nolanlawson/shadow-dom' into shadow-dom

9a625c8

fix: revert whitespace changes

a9cfa2e

nolanlawson requested a review from cookiecrook May 26, 2023 23:03

cookiecrook approved these changes May 30, 2023

View reviewed changes

index.html Outdated Show resolved Hide resolved

index.html Outdated Show resolved Hide resolved

nolanlawson and others added 2 commits June 1, 2023 09:34

fix: update index.html

84a03a5

Co-authored-by: James Craig <cookiecrook@users.noreply.github.com>

fix: update index.html

1d37077

Co-authored-by: James Craig <cookiecrook@users.noreply.github.com>

Jym77 mentioned this pull request Sep 7, 2023

Name computation: Include shadow DOM in name from content Siteimprove/alfa#1470

Merged

MelSumner added the Agenda label Feb 8, 2024

MelSumner requested review from accdc and MelSumner February 8, 2024 18:35

Merge branch 'main' into shadow-dom

493b13b

spectranaut merged commit 67ca225 into w3c:main Feb 27, 2024
1 check passed

github-actions bot added a commit that referenced this pull request Feb 27, 2024

Add steps for shadow roots and slots (#167)

b27ec3e

SHA: 67ca225 Reason: push, by spectranaut Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Jym77 mentioned this pull request Apr 30, 2024

Is the accessibility tree built on the DOM tree, or the flat tree (incl. shadow DOMs)? w3c/aria#2173

Open

	<li id="comp_name_from_content_for_each_child_set_current" name="step2F.iv.a">Set the <code>current node</code> to the <code>rendered child node</code>.</li>
	<li id="comp_name_from_content_for_each_child_set_current" name="step2F.iii.a"><!-- Leave legacy "step2F.iii.a" name value with this step, and don't add new name values. -->Set the <code>current node</code> to the <code>rendered child node</code>.</li>

Add steps for shadow roots and slots #167

Add steps for shadow roots and slots #167

Conversation

nolanlawson commented Sep 15, 2022 • edited by pr-preview bot Loading

Implementation

nolanlawson commented Sep 16, 2022

jnurthen commented Sep 22, 2022

cookiecrook left a comment

Choose a reason for hiding this comment

nolanlawson commented Sep 29, 2022

nolanlawson commented Sep 29, 2022 • edited Loading

nolanlawson commented Sep 29, 2022

spectranaut left a comment

Choose a reason for hiding this comment

spectranaut commented Oct 19, 2022

jcsteh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nolanlawson Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nolanlawson commented Oct 20, 2022

rniwa commented Oct 20, 2022

cookiecrook left a comment

Choose a reason for hiding this comment

nolanlawson commented Apr 21, 2023

jnurthen commented Apr 21, 2023

spectranaut left a comment

Choose a reason for hiding this comment

cookiecrook left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cookiecrook left a comment

Choose a reason for hiding this comment

Jym77 commented Sep 7, 2023 • edited Loading

nolanlawson commented Sep 18, 2023

nolanlawson commented Sep 18, 2023

Jym77 commented Sep 20, 2023

jcsteh commented Sep 20, 2023

alice commented Sep 20, 2023

MelSumner commented Feb 8, 2024

spectranaut commented Feb 15, 2024

nolanlawson commented Sep 15, 2022 •

edited by pr-preview bot

Loading

nolanlawson commented Sep 29, 2022 •

edited

Loading

nolanlawson Oct 21, 2022 •

edited

Loading

Jym77 commented Sep 7, 2023 •

edited

Loading