Does aria-hidden obey DOM or AX tree ancestors?

aleventhal commented 2 years ago

Spec says: "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."

In Chrome, aria-hidden propagates down the AX tree, meaning that it follows aria-owns relationships. Therefore, something might not be a DOM ancestor of something aria-hidden, but still become aria-hidden because it's within the AX hierarchy. This differs from using CSS to hide subtree (e.g. display:none), which only follows the DOM hierarchy.

Is Chrome correct? The spec refers to "ancestors", but I did not see ancestors defined in the "Important Terms" section: https://w3c.github.io/aria/#terms

aleventhal commented 2 years ago

Note that there is a Chrome CL out for review that depends on getting an answer to this (hopefully soon): https://chromium-review.googlesource.com/c/chromium/src/+/3547516

scottaohara commented 2 years ago

been thinking about this for the last half hour or so, and i think i agree that this is correct – that aria-owns elements should also be treated as 'hidden` in such a case?

i'm just trying to think if there may be any sort of responsive layout gotcha that i'm just not thinking of.... where some component is cobbled together via an aria-owns restructuring in a large viewport view, but at a small viewport view the containing element that 'owned' these other sub-element goes away (is set to display none), and all these other elements are meant to still meant to be exposed?

aleventhal commented 2 years ago

Also, we should check to see if Firefox/Chrome/Safari all act the same.

And, "ancestor" should go into important terms. We should make sure it always means the same thing throughout the doc.

pkra commented 2 years ago

Added to https://github.com/w3c/aria/projects/16 since that seems like a good fit.

spectranaut commented 2 years ago

We discussed having a test case to see how hidden for the parent is currently handled across all the browsers/platforms, does this capture all the scenarios you wanted clarification on, @aleventhal?

test case

About the browsers:

Safari hasn't implemented aria-owns, so there is no propagation of aria-hidden down the accessibility tree through aria-owns
Firefox does support aria-owns but does not propagate aria-hidden down the accessibility tree past aria-owns
Chrome does support aria-owns and does not propagate aria-hidden down the accessibility tree past aria-owns.

aleventhal commented 2 years ago

Thanks @spectranaut, the test case is good.

Regarding Chrome, I'm testing here on Canary/Mac, and here's what I see:

CSS propagates down the DOM only, and not the a11y tree, as expected
aria-hidden also propagates down the DOM only, and not the a11y tree, but this is unexpected and does not match what I see in the code in AXObject::ComputeIsAriaHidden()

Is this what you're seeing? I wasn't sure what your comment was saying, because "children" is slightly ambiguous.

Also, I was surprised to learn Safari does not implement aria-owns, To be fair, getting it right has been an absolute nightmare. Very long tail of edge cases and bugs.

aleventhal commented 2 years ago

I think it should obey the accessibility hierarchy, since it's an accessibility property.

spectranaut commented 2 years ago

Following up from the discussion in the meeting today

It seems that the consensus is that aria-hidden should obey AX tree ancestors, though the status of the browsers is very inconsistent.

@cookiecrook, what is the priority of aria-owns in Safari? @jcsteh, do you want to weigh in on this, as Firefox does not currently propagate aria-hidden down the accessibility tree?

jcsteh commented 2 years ago

I'm really confused. In Chrome Canary on Windows, just as was noted in https://github.com/w3c/aria/issues/1714#issuecomment-1117373616, I'm seeing all the owned nodes in the test case; none of them are hidden. The behaviour seems identical to Firefox.

I guess I follow the argument that aria-hidden is an accessibility property, so it should impact other accessibility properties. On the other hand, aria-hidden effectively removes the subtree from the a11y tree altogether. It could be interpreted that aria-hidden causes a subtree to be ignored, so we should also ignore any properties set anywhere in that subtree.

From an implementation standpoint, in Firefox, we actually remove aria-hidden subtrees altogether internally. If the subtree doesn't exist, we can't respect properties set on nodes inside that subtree. Fixing that would probably require that we internally render the a11y tree for aria-hidden but not expose it to clients, which seems to be what Chrome does. That's a pretty massive change for us. If that's where we need to go, I can look into the feasibility of that... but at this point, Chrome doesn't seem to be handling this anyway.

aleventhal commented 2 years ago

I can buy that. Makes things simpler to keep the same as that argument works. We should clarify in the spec.

On Thu, May 26, 2022, 9:01 PM James Teh @.***> wrote:

I'm really confused. In Chrome Canary on Windows, just as was noted in #1714 (comment) https://github.com/w3c/aria/issues/1714#issuecomment-1117373616, I'm seeing all the owned nodes in the test case; none of them are hidden. The behaviour seems identical to Firefox.

I guess I follow the argument that aria-hidden is an accessibility property, so it should impact other accessibility properties. On the other hand, aria-hidden effectively removes the subtree from the a11y tree altogether. It could be interpreted that aria-hidden causes a subtree to be ignored, so we should also ignore any properties set anywhere in that subtree.

From an implementation standpoint, in Firefox, we actually remove aria-hidden subtrees altogether internally. If the subtree doesn't exist, we can't respect properties set on nodes inside that subtree. Fixing that would probably require that we internally render the a11y tree for aria-hidden but not expose it to clients, which seems to be what Chrome does. That's a pretty massive change for us. If that's where we need to go, I can look into the feasibility of that... but at this point, Chrome doesn't seem to be handling this anyway.

— Reply to this email directly, view it on GitHub https://github.com/w3c/aria/issues/1714#issuecomment-1139184844, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQAZS6SDLWOVWPNZDE27TVMANGJANCNFSM5RRLM6KA . You are receiving this because you were mentioned.Message ID: @.***>

spectranaut commented 2 years ago

@jcsteh Did you look at the accessibility tree? In the Chrome accessibility tree "child c" is missing, and in the Firefox accessibility tree "child c" exists.

Though I do see your reasoning, and we will revisit in the ARIA working group meeting today!

cyns commented 2 years ago

What is the use case for putting aria-owns and aria-hidden on the same element? Is this primarily an author error, and we're talking about a repair strategy, or is it an author intent that needs to be expressed?

Are there performance trade-offs for not pruning aria-hidden elements before doing other accessibility tree calculations? If so, does the use-case outweigh them? Does having a spec inconsistency outweigh them?

jnurthen commented 2 years ago

What is the use case for putting aria-owns and aria-hidden on the same element?

Its not just the same element we need to consider - aria-owns could end up being on a child of the hidden element.

jcsteh commented 2 years ago

@jcsteh Did you look at the accessibility tree?

Yes.

In the Chrome accessibility tree "child c" is missing

For me, in latest Chrome Canary, it is definitely present:

++++++++genericContainer ignored invisible location=(85, 1130) size=(2425, 98) fontFamily='Times New Roman' htmlTag='div' language='en' backgroundColor=ffffff color=000000 isLineBreakingObject=true lineStarts= wordStarts= wordEnds= lineEnds= sentenceStarts= sentenceEnds=
++++++++++staticText ignored invisible location=(110, 1156) size=(188, 46) fontFamily='Times New Roman' language='en' name='PARENT C' nameFrom=contents backgroundColor=ffffff color=000000 lineStarts=0 wordStarts=0,7 wordEnds=6,8 lineEnds=8 sentenceStarts=0 sentenceEnds=8
++++++++genericContainer location=(85, 1247) size=(2425, 98) pageLocation=(85, 1247) pageSize=(2425, 98) unclippedLocation=(85, 1247) unclippedSize=(2425, 98) display='block' fontFamily='Times New Roman' htmlTag='div' language='en' backgroundColor=ffffff color=000000 text-align=left textDirection=ltr domNodeId=12 isLineBreakingObject=true fontSize=16.00 fontWeight=400.00 lineStarts=0 wordStarts=0,6 wordEnds=5,7 lineEnds=7 sentenceStarts=0 sentenceEnds=7
++++++++++staticText location=(110, 1273) size=(159, 46) pageLocation=(110, 1273) pageSize=(159, 46) unclippedLocation=(110, 1273) unclippedSize=(159, 46) display='block' fontFamily='Times New Roman' language='en' name='CHILD C' nameFrom=contents backgroundColor=ffffff color=000000 textDirection=ltr domNodeId=13 fontSize=16.00 fontWeight=400.00 lineStarts=0 wordStarts=0,6 wordEnds=5,7 lineEnds=7 sentenceStarts=0 sentenceEnds=7

cookiecrook commented 2 years ago

I would have thought aria-hidden propagated down the DOM tree (not the AX tree like Chrome) but I don't have a strong opinion on the cases where aria-owns blurs that boundary.

@spectranaut wrote:

Safari hasn't implemented aria-owns

@aleventhal wrote:

I was surprised to learn Safari does not implement aria-owns

These comments (and more from the IRC thread) seem demonstrably incorrect.

There is implementation of aria-owns in WebKit on Mac and iOS, but it creates a reference from the owning element to the owned element (AXOwns: AXElementRef, IIRC on Mac), rather than reordering the tree. IIRC, all the original browser implementations of aria-owns all did this, because of implementation concerns. I don't know enough about Chromium's changes to know where Chrome versus WebKit is no longer in alignment on aria-owns, or if reordering the tree is now possible in all implementations, if that's what is being suggested here.

cookiecrook commented 2 years ago

Much of the WebKit implementation of aria-owns was from 2010. https://github.com/WebKit/WebKit/blob/main/Source/WebCore/accessibility/AccessibilityObject.h#L304

I do see a source update from as recently as January of this year. https://bugs.webkit.org/show_bug.cgi?id=233383

cookiecrook commented 2 years ago

AXOwns on Mac: https://github.com/WebKit/WebKit/blob/main/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm#L151

scottaohara commented 2 years ago

It's the reordering / 'expected' behavior that doesn't appear to be implemented in Safari.

an example: https://codepen.io/scottohara/pen/VwQdyRN

with latest macOS/iOS Safari, VoiceOver only treats the tablist as if it has 1 tab within, though the second tab is referenced by aria-owns.

VoiceOver with Chrome and Firefox however both treat that second tab as if it was within the tablist, and if using VO navigation, the second tab is in the reading order after the first tab and before the paragraph of content with these browsers.

spectranaut commented 2 years ago

Wow @jcsteh thanks you are right, chrome doesn't implement propagating aria-hidden through aria-owns, I corrected my summary and sorry for the confusion. Not sure what I was looking at.

Also thanks for the history/context on aria-owns in webkit, @cookiecrook !

aleventhal commented 2 years ago

@cookiecrook I'd still like to get your thoughts on Scott's comment. This comes up a lot.

cookiecrook commented 2 years ago

@aleventhal wrote:

@cookiecrook I'd still like to get your thoughts on @scottohara's comment.

Discussed in the call this morning and raised https://webkit.org/b/241694

aleventhal commented 2 years ago

I wrote up a doc describing how Blink handles aria-owns, and while looking at Chrome's code, I realized that we handle the aria-hidden issue described here pretty well. Chrome simply ignores aria-owns if either the parent or child are aria-hidden. It's simply not worth the extra pain to implement that and other aria-owns corner cases. Our goal with unusual aria-owns cases is just to avoid crashing.

spectranaut commented 2 years ago

We discussed this in a meeting a few weeks back and ultimately agreed with @jcsteh: https://www.w3.org/2022/06/09-aria-minutes#t05

In conclusion, aria-hidden obeys DOM ancestor only. In other words, aria-owns is ignored if the element has been hidden with aria-hidden.

There is no change to implementations for this. However, the work to do now is add the ability within the spec to differential between "DOM hierarchy" and "AX hierarchy", and make sure that hidden definition uses the correct language to indicate "DOM hierarchy": https://w3c.github.io/aria/#dfn-hidden

@smhigley or @pkra, do you think the work in this issue will clarify this difference? https://github.com/w3c/aria/issues/1150

If ancestor and hierarchy ultimately always refer to the accessibility tree, then we might want to consider Aaron's suggestion:

<aaronlev> We could say ancestor/descendant always means AX tree
<aaronlev> except in case of aria-hidden
<aaronlev> Because the aria-hidden property could say "Use of aria-hidden combined with aria-owns is undefined"
<aaronlev> to scare ppl away
<aaronlev> undefined or invalid

w3c / aria

Does aria-hidden obey DOM or AX tree ancestors? #1714