w3c / accname

Accessible Name and Description Computation
https://w3c.github.io/accname/
61 stars 23 forks source link

Add steps for `ariaLabelledByElements` and `ariaDescribedByElements` #170

Closed nolanlawson closed 6 months ago

nolanlawson commented 1 year ago

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

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

https://github.com/whatwg/html/pull/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

nolanlawson commented 1 year ago

After re-reading this discussion (https://github.com/whatwg/html/issues/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 https://github.com/whatwg/html/issues/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 commented 1 year ago

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 commented 1 year ago

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

jnurthen commented 1 year ago

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

nolanlawson commented 1 year ago

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 commented 1 year ago

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 https://github.com/w3c/accname/pull/167#pullrequestreview-1148595405 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 commented 1 year ago

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 commented 6 months ago

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 commented 6 months ago

@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.