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

Add steps for ariaLabelledByElements and ariaDescribedByElements #170

Closed
wants to merge 2 commits into from

Conversation

nolanlawson
Copy link
Member

@nolanlawson nolanlawson commented Sep 29, 2022

Related to #167, and attempts to address #51.

With whatwg/html#7934 we have the addition of the ariaLabelledByElements and ariaDescribedByElements properties, which reference attr-associated elements (a FrozenArray).

whatwg/html#7934 already explains how these arrays interact with shadow DOM, so (AIUI) the accname spec does not need to explicitly recapitulate these steps. But for the record:

Let elements be an empty list.

If element's explicitly set attr-elements is not null, then:

For each attrElement in the element's explicitly set attr-elements:

If attrElement is not a descendant of any of element's shadow-including ancestors, then continue.

Append attrElement to elements.

So I believe accname needs only to make mention of the ariaLabelledByElements/ ariaDescribedByElements arrays in the same places where it makes mention of aria-labelledby and aria-describeddby. If two elements are in shadow roots that cannot be associated (i.e. one is not a descendant of the other's shadow-including ancestors), then the array would not contain that element in the first place.

/cc @mrego


Preview | Diff

index.html Outdated
<ol>
<li id="step2B.ii.a">Set the <code>current node</code> to the node referenced by the IDREF.</li>
<li id="step2B.ii.a">Set the <code>current node</code> to the element referenced by the IDREF or element in the array.</li>
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed this from node to element since technically an IDREF can only point to an element, since only elements can have IDs.

@nolanlawson
Copy link
Member Author

nolanlawson commented Sep 30, 2022

After re-reading this discussion (whatwg/html#8306), I think this PR may need to be amended.

It seems aria-labelledby and aria-describedby may get "out of sync" with their FrozenArray counterparts. For instance, the ariaLabelledByElements may have no IDs (in which case aria-labelledby is the empty string), or the ariaLabelledByElements may be moved into an invalid shadow root (in which case aria-labelledby will still have the "stale" ID, or possibly be the empty string if whatwg/html#8306 is fixed).

So perhaps we should change the accname spec to only make reference to the FrozenArrays, since they are guaranteed to be correct. This would be quite a big change to the spec, though.

@nolanlawson
Copy link
Member Author

This ^ (524cec6) is my attempt to prefer the *Elements array properties as the source of truth. I removed references to aria-labelledby and aria-describedby from the normative sections of the document, and added a comment explaining the distinction between the two.

I kept aria-labelledby and aria-describedby in the non-normative sections and declarative examples (e.g. HTML code snippets), as well as in phrases like "aria-labelledby traversal", where it doesn't seem important whether the attribute or property flavor is used.

@annevk
Copy link
Member

annevk commented Oct 27, 2022

You cannot refer to the public API here. Instead you need to use and reference the internal concepts that @mrego defined for reflect:

  • attr-associated element (note that attr is a variable here which you need to replace with the attribute name)
  • attr-associated elements (same)

@jnurthen
Copy link
Member

jnurthen commented Nov 4, 2022

I think this is going to need to change once #69 lands

@nolanlawson
Copy link
Member Author

You cannot refer to the public API here. Instead you need to use and reference the internal concepts […]

I'm wondering if, at this point, it makes more sense to rewrite this spec from scratch to use the a11y tree rather than the DOM tree?

I've already gotten feedback from @jsteh that Firefox walks the a11y tree rather than the DOM tree. If WebKit and Chromium do the same, then it would be a bit odd to have this spec describe something that browsers don't actually implement.

That said, I'm happy to proceed as-is to avoid making the perfect the enemy of the good.

@cookiecrook
Copy link
Contributor

I'm wondering if, at this point, it makes more sense to rewrite this spec from scratch to use the a11y tree rather than the DOM tree?

I've already gotten #167 (review) from @jsteh that Firefox walks the a11y tree rather than the DOM tree. If WebKit and Chromium do the same, then it would be a bit odd to have this spec describe something that browsers don't actually implement.

There isn’t a standard accessibility tree between platforms or engines, so that would be even more difficult. Each engine implements its own internal tree that is then vended to the platform accessibility API, to create the platform specific tree.

WebKit based its implementation off the render tree, Gecko off the DOM tree. Chromium moved a little closer to Firefox since the fork from WebKit, but each of the engine-specific accessibility trees are still unique to some degree.

@annevk
Copy link
Member

annevk commented Nov 9, 2022

I think most browsers use some variant of the render/layout tree. E.g., they all hide display:none subtrees and such as far as I know.

Anyway, defining that is somewhat orthogonal to this particular issue as even if we did that, in order to get from an item in the render tree to its labelled elements, we'd have to poke at its corresponding DOM node and look at the internal concepts I mentioned above.

@MelSumner
Copy link
Contributor

Reading through this PR and all of the comments, I wonder if we have a path forward with this particular PR or maybe a different PR is needed? @nolanlawson WDYT?

@nolanlawson
Copy link
Member Author

@MelSumner Yes, I think that this PR probably needs to be rewritten from scratch at this point. In any place where we are referring to aria-labelledby or aria-describedby, we need to instead refer to the attr-associated elements.

@nolanlawson nolanlawson closed this Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants