-
Notifications
You must be signed in to change notification settings - Fork 125
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
Clarify relationship between aria-hidden
and aria-owns
#1839
base: main
Are you sure you want to change the base?
Conversation
Two points to mention: I'm encountering a problem similar to #1818 but the other way around, and I'm not sure this PR fully handles it 🤔 The case in #1818 was: <ul aria-owns="li"></ul>
<div aria-hidden="true">
<li id="li">Child node</li>
</div> That is, a not-hidden element What happens in the reverse case: <div aria-hidden="true">
<div id="owner" aria-owns="li" />
</div>
<ul> <li id="li">Item</li> </ul> In that case, a hidden element Second point is that the language in §7.1 Excluding Elements from the Accessibility Tree uses a lot of "descendant" and other tree-related wording, notably for the case concerning this PR:
Should there be a precision that "descendants" here mean "either descendant in the DOM tree, or descendant in the accessibility tree where |
For the second point: #1150 |
Mmm, thanks for poking at this, @Jym77. I hadn’t considered that reverse case. “Owning element” is currently defined as “any DOM ancestor of the element, or any element with an In this reverse scenario, |
@adampage That actually makes sense 😄 <div aria-hidden="true" aria-owns="li"></div>
<ul> <li id="li">Item</li> </ul> In that case, the current behaviour is to expose I guess it's a bit of a question which of Can get weirder with a case like: <div aria-hidden="true" aria-owns="foo"></div>
<div id="foo" aria-owns="bar"></div>
<div id="bar"></div> Now, |
This discussion has been super helpful, @Jym77, thank you. 😁 I think you hit the nail on the head with the question of which attribute “acts faster”. Let’s say that this PR now includes some additional text (somewhere appropriate in the spec) to the effect of: “Changes to ownership are resolved in DOM order. If both Now let’s apply that to the four scenarios that have come up. In the initial #1818 example: <ul aria-owns="li"></ul>
<div aria-hidden="true">
<li id="li">Child node</li>
</div> ...the In the reverse case: <div aria-hidden="true">
<div id="owner" aria-owns="li" />
</div>
<ul> <li id="li">Item</li> </ul> ... In the follow-up simplified case: <div aria-hidden="true" aria-owns="li"></div>
<ul> <li id="li">Item</li> </ul> ... In the follow-up weirder case: <div aria-hidden="true" aria-owns="foo"></div>
<div id="foo" aria-owns="bar"></div>
<div id="bar"></div> ...the previous story is essentially true again. The original intention of this spec clarification was to align with the current Chrome & Firefox behavior, so I built out all four of these scenarios in Codepen and tested them with macOS + VoiceOver + Chrome. Each outcome I described above is correctly demonstrated by that AT combo. Whew. Having said all that, would you agree that the gist of my additional spec proposal is helpful & accurate? And that it would reasonably lead to the interpretations I described for each of those four scenarios? Are there yet other ambiguous scenarios lurking out there? 😅 Thanks again! |
Yes, I think that clarification works. Maybe we need to add that change of ownership is only initiated by non-hidden node (that's more or less the second sentence but making it clear that one should stop looking for This also makes it clear that Great clarification 👍 |
was following along with this until:
this statement has made me wonder if this actually makes sense or not. since display none does modify the a11y tree... and then i'm not sure what case 2 refers to? the second of Adam's examples in his last comment, or the second odd case you brought up, @Jym77 ? |
Yep, poor writing on my side 🙈 and messing up things badly… <ul aria-owns="li"></ul>
<div style="display: none">
<li id="li">Child node</li>
</div> case 1 not 2 in Adam's list with This does not expose That is of course linked to non-interference with the host language ( |
Thanks for the correction/clarification. Yah that makes sense again now. |
3ddc773
to
4d5c3a6
Compare
This reverts commit a3dc2a9.
So, I reacquainted myself with the spec’s definition of “hidden”:
Based on that, I took another pass at the Beyond that, I wordsmithed the clarifications we discussed earlier in this thread and added them to the |
I feel there is still a small clarification needed to say that Owning element states:
Because of the "or", and no mention of unicity, in case 1 above <ul aria-owns="li"></ul>
<div aria-hidden="true">
<li id="li">Child node</li>
</div> Both the So, without disambiguating that, the Should we change the definition of "owning element" to state something like
Otherwise, I think this is correct and expresses what is needed 👍 |
I just want to note that @Jym77's recommended definition of owned is nice, and, that there is also a 1.3-blocking issue related to inconsistent use the term "owned by", just like how hidden was just revisited: The "owned by inconsistencies" can be addressed before of after this PR, but it might also lead to a term change or term update. |
Thanks @spectranaut and @Jym77, I’ve just passed along your suggested clarification for “owning element” in PR #1454. I think it will be helpful to address it in that context. If that goes through, then I’d plan to update this PR by leveraging @spectranaut’s recently clarified definition of “hidden”, the improved definition of “owning element”, and the new “accessibility parent” term. My first instinct would be to update the <p>
...An element is considered [=element/hidden=] if:
</p>
<ul>
<li>
It or any of its <a>accessibility parents</a> are not rendered
</li>
<li>
It or any of its <a>owning elements</a> are not rendered or have their
<code>aria-hidden</code> attribute value set to <code>true</code>
</li>
</ul> Could I trouble you to first take a look at my comment in #1454, and then consider my proposal here? |
I’ve created a new Codepen to describe & test assorted scenarios involving We originally decided to align the spec with the current behavior of Chrome & Firefox, but my testing showed some interesting inconsistencies for Chrome 110 on macOS vs Windows. Firefox 110 seemed to behave consistently with our expectations everywhere. Once #1454 is finalized, I’ll take another whack at the spec language for this PR, orienting it around Firefox’s demonstration of our intent. |
omg this code pen is AMAZING. |
index.html
Outdated
@@ -11756,7 +11756,7 @@ <h2>Definitions of States and Properties (all aria-* attributes)</h2> | |||
<sdef>aria-hidden</sdef> | |||
<div class="state-description"> | |||
<p><a>Indicates</a> whether the <a>element</a> is exposed to an accessibility API. See related <sref>aria-disabled</sref>.</p> | |||
<p>User agents determine an element's [=element/hidden=] status based on whether it is rendered, and the rendering is usually controlled by CSS. For example, an element whose <code>display</code> property is set to <code>none</code> is not rendered. An element is considered [=element/hidden=] if it, or any of its ancestors are not rendered or have their <code>aria-hidden</code> attribute value set to <code>true</code>.</p> | |||
<p>User agents determine an element's [=element/hidden=] status based on whether it is rendered, and the rendering is usually controlled by CSS. For example, an element whose <code>display</code> property is set to <code>none</code> is not rendered. An element will be <a href="#tree_exclusion">excluded from the accessibility tree</a> if it or any of its <a>owning elements</a> are [=element/hidden=] or have their <code>aria-hidden</code> attribute value set to <code>true</code>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think my only issue with this update is that the clarification about when to take aria-owns into account is so far removed from this initial mention of owning elements. As prior to reading the new paragraph added on line 12333 of this diff, i would have interpreted the use of "owning elements" to mean that "boo" should also be "hidden" because the hidden div "owns" boo... but for the fact on 12333 it is stated that's not true because the owning element is itself not rendered, so thus it can't own the "boo" element.
<div hidden aria-owns=boo>foo</div>
<div id=boo>boo</div>
i could see this clarified by either mentioning that if an element is an owning element due to usage of aria-owns, how this works is clarified later. Or that these bits of text are closer together in the spec, or that this paragraph remains just about ancestors being hidden, and owning elements / aria-owns headaches are talked about in one new paragraph/sets of paragraphs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heya @scottaohara, now that the accessibility parent/child language has been merged in, I updated this statement to reference accessibility parents
instead of owning elements
. I think this is now more accurate and also sidesteps the ambiguity you described here, at least until an author digs deeper into the aria-owns
section? Here’s the full new sentence:
An element will be excluded from the accessibility tree if it or any of its accessibility parents are hidden or have their
aria-hidden
attribute value set totrue
.
@spectranaut, I’m seeing that this would be the spec’s first mention of accessibility parents plural. I re-read that term’s definition and it feels okay to pluralize it here to essentially mean ancestors. Do you agree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh actually no, there is only one "accessibility parent" -- I think the appropriate phrase here is "Accessibility ancestor". We don't have it defined, but I think it is a clear enough term? You could link to the accessibility tree section of the document: https://w3c.github.io/aria/#accessibility_tree
Agreed. Most useful CodePen I think I've ever seen. @adampage, you are planning to turn some of this into a series of WPT tests for web-platform-tests/interop-accessibility#32 correct? Are you planning to file the implementation bugs you've discovered? Trying to determine how best to cross-reference the trackers from the issue, etc. |
index.html
Outdated
@@ -11756,7 +11756,7 @@ <h2>Definitions of States and Properties (all aria-* attributes)</h2> | |||
<sdef>aria-hidden</sdef> | |||
<div class="state-description"> | |||
<p><a>Indicates</a> whether the <a>element</a> is exposed to an accessibility API. See related <sref>aria-disabled</sref>.</p> | |||
<p>User agents determine an element's [=element/hidden=] status based on whether it is rendered, and the rendering is usually controlled by CSS. For example, an element whose <code>display</code> property is set to <code>none</code> is not rendered. An element is considered [=element/hidden=] if it, or any of its ancestors are not rendered or have their <code>aria-hidden</code> attribute value set to <code>true</code>.</p> | |||
<p>User agents determine an element's [=element/hidden=] status based on whether it is rendered, and the rendering is usually controlled by CSS. For example, an element whose <code>display</code> property is set to <code>none</code> is not rendered. An element will be <a href="#tree_exclusion">excluded from the accessibility tree</a> if it or any of its <a>owning elements</a> are [=element/hidden=] or have their <code>aria-hidden</code> attribute value set to <code>true</code>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or have their <code>aria-hidden</code> attribute value set to <code>true</code>.
I know you didn't add this part, but I recall @aleventhal wanted a way to invert aria-hidden
on an ancestor by setting aria-hidden=false
on a rendered subtree. e.g. aria-hidden=true
on the <body>
and aria-hidden=false
on a modal view (a dialog perhaps) ... This current wording excludes that I think.
It also might exclude inverting visibility: hidden;
with visibility: visible;
unless there is already an exception carved out in the ARIA definition of "hidden."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This current wording excludes that I think.
The below text is terrible but something like:
- if the current element is hidden/unrendered (def tbd), OR
- (if the element does not have aria-hidden=true set AND its nearest ancestor with aria-hidden defined is (both aria-hidden=false and NOT hidden aka rendered))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I acknowledge that distinction is difficult to parse. Perhaps a Deep Dive (w./ Aaron) or Agenda+ for a weekly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In anticipation of #2090 getting merged (in which we made aria-hidden="false"
synonymous with undefined
), I’m now thinking that this part can be left as is?
Heya @cookiecrook, I just turned all of these tests into comparable (I hope) WPT tests: PR #44822. Using your latest tip, I’ve marked that file as I’d be grateful for your thoughts on my approach for testing this spec change within WPT’s capabilities. My original Codepen tests weren’t accname-focused; I just rendered a bunch of With WPT, I chose to use the test driver’s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good now, except for the switch from "accessibility parents" to "accessibility ancestors"! I'll mark as approving once that is done :)
Thanks, @spectranaut. I’ve swapped in “accessibility ancestors” for “accessibility parents” and resolved the merge conflict. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Closes #1818.
The definition of Owning Element is already inclusive of both DOM ancestry and ownership via
aria-owns
:Because of this, I’m thinking it’ll be sufficient to swap out “ancestors” for “owning elements” in
aria-hidden
’s description.I also editorially removed what I believe is an unhelpful comma.
Preview | Diff