-
Notifications
You must be signed in to change notification settings - Fork 22
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 shadow roots and slots #167
Merged
Merged
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
cf152ed
Add steps for shadow roots and slots
nolanlawson 3007834
Update index.html
nolanlawson 852dd64
Update index.html
nolanlawson 8020dce
Update index.html
nolanlawson 37d4048
Update index.html
nolanlawson fd225a3
Update index.html
nolanlawson afd6ea5
Update index.html
nolanlawson 95782ce
add language about slot representing
nolanlawson 442df9a
fix: fix roman numerals
nolanlawson 8d17a28
fix: remove comment
nolanlawson 51c6b09
chore: fix whitespace
nolanlawson f33d45d
Merge branch 'main' into shadow-dom
jnurthen ecf6cb4
Merge remote-tracking branch 'origin/main' into shadow-dom
nolanlawson 9a625c8
Merge remote-tracking branch 'nolanlawson/shadow-dom' into shadow-dom
nolanlawson a9cfa2e
fix: revert whitespace changes
nolanlawson 84a03a5
fix: update index.html
nolanlawson 1d37077
fix: update index.html
nolanlawson 493b13b
Merge branch 'main' into shadow-dom
jnurthen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 might well be missing something - this spec still really boggles my brain - but what excludes the
<slot>
itself from being processed? The Chromium bug you filed is the precise example of this. We'll process the label as an idref, start walking its descendants as a result of 2h, recurse into the div, recurse to the slot as one of the child nodes of the div starting at step 2, and end up using the aria-label on the slot in 2d.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.
My intention is to tweak the children-traversal part of the algorithm to traverse into slot assigned nodes rather than child nodes. The slot itself is not excluded from processing; you're correct.
Based on w3c/html-aam#440 it looks like this should be handled in the
html-aam
spec instead?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.
If I understand correctly, that means the Chromium bug isn't dealt with by this addition to the spec, correct? I'm not saying this isn't necessary, just clarifying for sure that it doesn't address the Chromium bug.
This gets back to my comment about tree traversal. As I see it, it's less to do with the fact that a slot can't be named and more to do with the fact that the slot itself (but not its rendered children) should be skipped in the traversal in the first place. I realise this might sound like nitpicking, but I think it's important in understanding how browsers actually do this.
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.
Correct, it's unrelated.
That makes sense to me! I would ask @scottaohara if this is similar to other "HTML elements which do not support naming," or if
<slot>
is a special case?To add another wrinkle: the default UA stylesheet for
<slot>
hasdisplay: contents
, but this can be overridden by the web author. So I'm not sure of the "namelessness" of slots is due to their being slots, or due todisplay: contents
.Edit: apparently
display: contents
should not affect the accessibility tree, but today it does? So it may be irrelevant to the<slot>
discussion.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.
Oh blah. You're right: it's probably the fact that it has display: contents that excludes it from the Firefox a11y tree. As your test case shows, it does get exposed with display: block (because block elements have some semantic value by virtue of being block). That means that
<slot aria-label="foo">
probably would get exposed in Firefox except for this bug.So it does look like we'll need some explicit provision that slots shouldn't be exposed. I'm not sure if them not having a name is enough. That said, this is a bit tricky because
<slot style="display: block;" tabindex="0">
does become focusable.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.
Thanks, that makes sense! To identify this bug, we could try adding WPT tests that set
display: block
on the<slot>
.Well, maybe
<slot>
should be an element that can support naming? Unless #173 (comment) is the last word on this.Note that I don't have a strong opinion; I'm just trying to determine the correct behavior.
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 the case someone does
<slot style="display: block;" tabindex="0">
, we've gotten quite into the territory of author error correction.If properly used, the slot element, along with elements like title, meta, template, etc... none of those should ever actually be exposed to a user and they should not be nameable. But, if you force these elements to be rendered by changing their CSS display property, then they essentially become 'generic' elements.
I think it makes sense to call out that if an author forces elements like these to be exposed to the a11y tree, then browsers should handle them like generics. But, again, I would stress this is correcting for author misuse and otherwise - if used properly - they cannot be named.
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.
@scottaohara Thanks for the feedback! Do you think this should be done in
accname
orhtml-aam
? I'm wondering if it makes more sense to put it inhtml-aam
(per w3c/html-aam#440), since inaccname
we're merely defining the tree traversal algorithm w.r.t<slot>
s and their assigned nodes, not whether<slot>
s can be named.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.
yes, i think this should be handled in html aam per the linked issue. i don't think accName should need to call this particular element out in any specific way. that's is what html aam can do.