-
Notifications
You must be signed in to change notification settings - Fork 23
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
Specify behavior when following a hidden subtree via aria-labelledby … #150
Specify behavior when following a hidden subtree via aria-labelledby … #150
Conversation
I've setup my account as requested, feel free to "revalidate". Thanks! |
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.
Personally I like it and approve. Pending review by @MelSumner and @cookiecrook of course. Hopefully we can discuss this at the next meeting to finally get this pulled in.
I'm looking at these use cases in a CodePen, apologies for the review delay. |
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 looks good to me. I always find this spec hurts my brain a lot and it takes me forever to figure out what behaviour exactly is changing. 😳 For the sake of absolute clarity, I assume the first example you've added demonstrates the difference between the previous and proposed new behaviours? With the previous behaviour (as specified), the label for element1 would be empty because element3 is hidden and not directly referenced.
@jcsteh: Exactly, that's the difference between the previous wording and the new one. |
@MelSumner: thanks for the CodePen, it's very useful! |
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.
seems like we are agreed on these changes, thank you for the submission.
Not sure why it was marking some changes as conflicted, but I think they are resolved now. |
…review SHA: 3419bd8 Reason: push, by @MelSumner Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@MelSumner I thought we were waiting for @cookiecrook and @cyns reviews |
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.
Mostly editorial with a few clarity questions. Overall I think it makes sense.
Since this is already merged, I could file a separate Issue/PR after we settle on a replacement or clarification for "root of the traversal"…
<li id="step2A">If the <code>current node</code> is <a class="termref">hidden</a> <strong>and is not</strong> directly referenced by <code>aria-labelledby</code> or <code>aria-describedby</code>, nor directly referenced by a native host language text alternative [=element=] (e.g. <code>label</code> in HTML) or [=attribute=], return the empty string. | ||
<li id="step2A">If the <code>current node</code> is <a class="termref">hidden</a> <strong>and is</strong>: | ||
<ul> | ||
<li><strong>Not</strong> part of an <code>aria-labelledby</code> or <code>aria-describedby</code> traversal, where the node directy referenced by that relation was hidden.</li> |
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.
Last two words ("was hidden") seem to imply either transiency (e.g. element is no longer hidden) or that the element was the direct target of the hiding (e.g. this specific element is the actually hidden node rather than being hidden as a side effect of a hidden ancestor).
Perhaps "is hidden" would be better here, since I understand the intent to be "currently hidden" and we don't care why/how it was hidden? If I'm misunderstanding, more clarification of the intent would be helpful.
I can't add a diff suggestion. Perhaps because it's already merged?
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.
Perhaps "is hidden" would be better here, since I understand the intent to be "currently hidden" and we don't care why/how it was hidden?
Exactly this, I was not trying to express anything different. I should have used the present tense as in the original.
<li id="step2A">If the <code>current node</code> is <a class="termref">hidden</a> <strong>and is</strong>: | ||
<ul> | ||
<li><strong>Not</strong> part of an <code>aria-labelledby</code> or <code>aria-describedby</code> traversal, where the node directy referenced by that relation was hidden.</li> | ||
<li><strong>Nor</strong> part of a native host language text alternative <a class="termref">element</a> (e.g. <code>label</code> in HTML) or <a class="termref">attribute</a> traversal, where the root of that traversal was hidden.</li> |
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.
ditto
- was hidden
+ is 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.
"where the root of that traversal" may also be ambiguous.
In this case, is "root" referring to the element labeled by this host language text alternative (e.g. an <input>
labeled by <label>
that is somewhere inside the overall computation)? Or is this referring to the root of the entire name computation traversal? I think the former because there may not be a time when a hidden node can be the root of the overall computation.
I labored over a diff for a few minutes but this may be similarly confusing. Sharing anyway as a straw man example.
- where the root of that traversal
+ where the element referenced by the host language text alternative
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.
is "root" referring to the element labeled by this host language text alternative ...?
Yes, that was my intention with that wording. I find the proposed diff fitting, it reuses the terminology used by the original wording which is more appropriate, although it's confusing in the same sense as the original was.
There is already an open issue about this: #148
<ul> | ||
<li>Nodes with CSS properties <code>display:none</code>, <code>visibility:hidden</code>, <code>visibility:collapse</code> or <code>content-visibility:hidden</code>: They are considered hidden, as they match the guidelines "not perceivable" and "explicitly hidden".</li> | ||
<li>Nodes with CSS properties <code>opacity:0</code> or <code>filter:opacity(0%)</code>, or similar SVG mechanisms: They are not considered hidden. Text hidden with these methods can still be selected or copied, and user agents still expose it in their accessibility trees.</li> | ||
<li>Nodes with the <code>aria-hidden="true"</code> property: it is considered hidden, matching the "explicitly hidden" guideline.</li> |
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.
Editorial nits to match the other lines:
- it is
+ They are
There is also a mix of casings in They/they on lines 282–285. I don't have a case preference other than consistency.
<li>Nodes with CSS properties <code>display:none</code>, <code>visibility:hidden</code>, <code>visibility:collapse</code> or <code>content-visibility:hidden</code>: They are considered hidden, as they match the guidelines "not perceivable" and "explicitly hidden".</li> | ||
<li>Nodes with CSS properties <code>opacity:0</code> or <code>filter:opacity(0%)</code>, or similar SVG mechanisms: They are not considered hidden. Text hidden with these methods can still be selected or copied, and user agents still expose it in their accessibility trees.</li> | ||
<li>Nodes with the <code>aria-hidden="true"</code> property: it is considered hidden, matching the "explicitly hidden" guideline.</li> | ||
<li>Nodes hidden off screen or behind another object: they are not considered hidden. They are exposed in the accessibility tree and they can even name on-screen objects.</li> |
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.
- Nodes hidden off screen
+ Nodes positioned off screen
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.
- and they can even name on-screen objects.
+ and can label visible elements.
<div><details> | ||
<summary>Comment:</summary> | ||
<p>By default, <a class="termref">assistive technologies</a> do not relay hidden information, but an author can explicitly override that and include hidden text as part of the <a class="termref">accessible name</a> or <a class="termref">accessible description</a> by using <code>aria-labelledby</code> or <code>aria-describedby</code>. </p> | ||
</details></div> | ||
<div><details> | ||
<summary>Example:</summary> | ||
<p>The following examples show the meaning of the clause "Not part of an <code>aria-labelledby</code> or <code>aria-describedby</code> traversal, where the node directy referenced by that relation was hidden.".</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.
- was hidden
+ is hidden
Merging provides additional motivation. 🤣 |
Thanks for the review, I definitely agree with all your suggestions, some are even mistakes on my side caused by writing over and over. To be more clear, I cannot think of a better suggestion for "root of the traversal". I would agree to revert it back to the term originally used, and keep track of what it means and how to enhance it in #148. |
This is an update in the spec: w3c/accname#150 When an aria-labelledby or aria-describedby relation points to a node which is hidden all its nodes will be exposed, including aria-hidden nodes. Chrome previously did exclude aria-hidden nodes in the situation described above. This change aligns the different user agent implementations. Finally, we keep some corrections in place to assign names to aria- -hidden content that can be focused. These corrections are now explicit in the code, when they used to be implicit. Bug: 1330931 Change-Id: Ib5186772a989838010d1bbafc9600827acd29c3b AX-relnotes: name computation: match spec for aria-hidden behavior Cq-Include-Trybots: luci.chromium.try:linux-blink-web-tests-force-accessibility-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3689418 Reviewed-by: Aaron Leventhal <aleventhal@chromium.org> Auto-Submit: Jacobo Aragunde Pérez <jaragunde@igalia.com> Commit-Queue: Aaron Leventhal <aleventhal@chromium.org> Cr-Commit-Position: refs/heads/main@{#1031522}
This is an update in the spec: w3c/accname#150 When an aria-labelledby or aria-describedby relation points to a node which is hidden all its nodes will be exposed, including aria-hidden nodes. Chrome previously did exclude aria-hidden nodes in the situation described above. This change aligns the different user agent implementations. Finally, we keep some corrections in place to assign names to aria- -hidden content that can be focused. These corrections are now explicit in the code, when they used to be implicit. Bug: 1330931 Change-Id: Ib5186772a989838010d1bbafc9600827acd29c3b AX-relnotes: name computation: match spec for aria-hidden behavior Cq-Include-Trybots: luci.chromium.try:linux-blink-web-tests-force-accessibility-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3689418 Reviewed-by: Aaron Leventhal <aleventhal@chromium.org> Auto-Submit: Jacobo Aragunde Pérez <jaragunde@igalia.com> Commit-Queue: Aaron Leventhal <aleventhal@chromium.org> Cr-Commit-Position: refs/heads/main@{#1031522} NOKEYCHECK=True GitOrigin-RevId: d29e6ca63942ee8d96c63c856ad61904191ae0cc
…or -describedby.
This proposal tries to bring the spec closer to the actual behavior of the different user agents. In general, user agents expose hidden content when following an aria-labelledby or -describedby relation into a hidden subtree. There are differences between engines, though.
This is a summary of what user agents will need to change to comply with this text update:
display:none
andvisibility:hidden
. Example:display:none
was used instead.Thanks in advance for your time and feedback.
Closes #57.
Preview | Diff