Bug 254934 - AX: Slot elements referenced with aria-labelledby have no label
Summary: AX: Slot elements referenced with aria-labelledby have no label
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tyler Wilcock
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-04-03 12:09 PDT by Carlos Lopez
Modified: 2023-08-26 10:45 PDT (History)
11 users (show)

See Also:


Attachments
Patch (7.61 KB, patch)
2023-04-30 23:47 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (7.59 KB, patch)
2023-05-01 00:39 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (7.52 KB, patch)
2023-05-01 00:42 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (7.50 KB, patch)
2023-05-01 11:26 PDT, Tyler Wilcock
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Lopez 2023-04-03 12:09:42 PDT
When using a ShadowDOM, an HTMLInputElement that references an HTMLSlotElement will not provide a label, unless that HTMLSlotElement has a display property other than "contents" (eg: inline-block).

See ShadowDOM tests 13 & 14 in this codepen: https://codepen.io/shortfuse/pen/PodMYBo

There are other bugs noted on the CodePen, but I'd like to specifically target this one related to HTMLSlotElement.

Sample tree:

<x-button>
  #shadow-root
    <input type=button aria-labelledby=slot>
    <slot id=slot></slot>
</x-button>
Comment 1 Radar WebKit Bug Importer 2023-04-03 12:10:20 PDT
<rdar://problem/107570512>
Comment 2 Tyler Wilcock 2023-04-29 21:11:55 PDT
Here's a runnable example of #14 stripped down from your complete Codepen:

https://codepen.io/twilco/pen/yLRovwR

We probably need to update AccessibilityNodeObject::accessibleNameForNode (or some other function) to collect accessible text from HTMLSlotElement::assignedNodes when we find a slot. But the fact that it works fine if the slot has something other than display:contents is interesting and maybe suggests that something else is wrong, so we'll need to investigate.
Comment 3 Tyler Wilcock 2023-04-29 22:10:40 PDT
Although I just tried adding:

slot {
  display: inline-block;
}

And Safari still doesn't provide a label for the button. Do you have a minimal example where giving the slot a non-display-contents display value results in a labelled button?
Comment 4 Tyler Wilcock 2023-04-30 23:47:19 PDT
Created attachment 466154 [details]
Patch
Comment 5 Tyler Wilcock 2023-05-01 00:39:54 PDT
Created attachment 466155 [details]
Patch
Comment 6 Tyler Wilcock 2023-05-01 00:42:40 PDT
Created attachment 466156 [details]
Patch
Comment 7 chris fleizach 2023-05-01 11:19:39 PDT
So it's labelled by a hidden node, so we instead fallback to text contents?

<input id='button' aria-labelledby='slot' type=button><slot id='slot' aria-hidden='true'></slot>
Comment 8 Tyler Wilcock 2023-05-01 11:26:18 PDT
Created attachment 466159 [details]
Patch
Comment 9 Tyler Wilcock 2023-05-01 11:30:47 PDT
(In reply to chris fleizach from comment #7)
> So it's labelled by a hidden node, so we instead fallback to text contents?
> 
> <input id='button' aria-labelledby='slot' type=button><slot id='slot'
> aria-hidden='true'></slot>
Hmm, not exactly. The goal with this patch is that when a slot is the target of aria-labelledby, the accessible name of that slot is that of its assigned contents. So perhaps someone is making a button web component, and they have a slot that represents the label for that button, and thus the button claims it is aria-labelledby whatever ends up in that slot.

I uploaded a new patch with aria-hidden removed from the testcase to prevent ambiguity  about the intentions here.
Comment 10 Carlos Lopez 2023-05-01 13:11:24 PDT
It's going to be a a couple days until I build the other reproducible. The codepen is my attempt at building a solution, so apologies for not including a smaller reproducible.

There are other bugs at play, like `<label>` reading it's own children, and how Safari concatenates nodes instead of using rendered style (as if using `textContent` instead of `innerText`), and `display: contents` can mess up the parsing, even on light DOM.

The point of `aria-hidden` is that the text node is just there to serve as a label for the input. The screen reader shouldn't announce the input with it's label (via the slot) and then continue to read the slot as if it were text.  See: https://www.tpgi.com/short-note-on-aria-labelledby-and-aria-describedby/

There might be more bugs we can file, though I'd like for slot to be able used as an `aria-labelledby` target with it's `display:contents` intact. Fixing `<label>` and the text concatenation isn't as important, though I'd understand if that needs to be fixed first.
Comment 11 Tyler Wilcock 2023-05-01 16:52:22 PDT
(In reply to Carlos Lopez from comment #10)
> It's going to be a a couple days until I build the other reproducible. The
> codepen is my attempt at building a solution, so apologies for not including
> a smaller reproducible.
> 
> There are other bugs at play, like `<label>` reading it's own children, and
> how Safari concatenates nodes instead of using rendered style (as if using
> `textContent` instead of `innerText`), and `display: contents` can mess up
> the parsing, even on light DOM.
> 
> The point of `aria-hidden` is that the text node is just there to serve as a
> label for the input. The screen reader shouldn't announce the input with
> it's label (via the slot) and then continue to read the slot as if it were
> text.  See:
> https://www.tpgi.com/short-note-on-aria-labelledby-and-aria-describedby/
> 
> There might be more bugs we can file, though I'd like for slot to be able
> used as an `aria-labelledby` target with it's `display:contents` intact.
> Fixing `<label>` and the text concatenation isn't as important, though I'd
> understand if that needs to be fixed first.
I filed:

https://bugs.webkit.org/show_bug.cgi?id=256186 (AX: Setting aria-hidden on a slot does not hide the slots assigned nodes)

Please file the other bugs you mentioned when you get some time. Thank you!
Comment 12 EWS 2023-05-03 12:48:52 PDT
Committed 263644@main (daa59c39a53b): <https://commits.webkit.org/263644@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 466159 [details].
Comment 13 Carlos Lopez 2023-08-26 10:45:21 PDT
I'm testing on V17 now and found a new bug. Essentially we tested:

#shadowroot
  <input type=button aria-labelledby=slot><slot></slot>

<x-button>Slotted</x-button>  => AXLabel: Slotted (Good)

----

But when using fallback content for unfilled slots, Safari will always read the innerText (not textContent) of the slot.

LightDOM: 
<x-button>Slotted <span style="display:none">Hidden</span></x-button>  => AXLabel: Slotted Hidden (Bad)

#shadowroot
  <input type=button aria-labelledby=slot><slot><span>Fallback<span></slot>

#shadowroot
  <input type=button aria-labelledby=slot><slot><span style="display:none">Hidden<span></slot>


To note, *visually* this works as expected. But the AXLabel is wrong when using fallback slotted nodes or hidden nodes in light DOM.

Filed over at https://bugs.webkit.org/show_bug.cgi?id=260772