Bug 264410 - iOS 17 regression, VoiceOver does not announce button in text if button is in shadow root
Summary: iOS 17 regression, VoiceOver does not announce button in text if button is in...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: Safari 17
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joshua Hoffman
URL:
Keywords: InRadar
: 265989 (view as bug list)
Depends on:
Blocks:
 
Reported: 2023-11-08 07:43 PST by Liam DeBeasi
Modified: 2023-12-21 20:49 PST (History)
13 users (show)

See Also:


Attachments
Code reproduction (806 bytes, text/html)
2023-11-08 07:43 PST, Liam DeBeasi
no flags Details
Patch (4.38 KB, patch)
2023-11-09 12:32 PST, Joshua Hoffman
no flags Details | Formatted Diff | Diff
Patch (4.39 KB, patch)
2023-11-09 13:36 PST, Joshua Hoffman
no flags Details | Formatted Diff | Diff
Patch (5.28 KB, patch)
2023-11-09 17:57 PST, Joshua Hoffman
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Liam DeBeasi 2023-11-08 07:43:51 PST
Created attachment 468515 [details]
Code reproduction

VoiceOver does not announce button text if the button is in the Shadow DOM and has "display: flex" and the text is wrapped in a div.

Steps to reproduce:

1. Open attached code reproduction on iOS 17.
2. Turn VoiceOver on.
3. Focus the button that says "Button One". Observe that VoiceOver only reads "button".
4. Focus the button that says "Button Two". Observe that VoiceOver reads "button" as well as "Button Two".

Expected Behavior:

I expect VoiceOver to announce "button" and "Button One" when focusing the first button.

Actual Behavior:

VoiceOver only announces "button" when focusing the first button.

Other Info:

- I can also reproduce this on STP Release 182 (Safari 17.4, WebKit 19618.1.4.1). However, I can not reproduce this on Version 17.0 (19616.1.27.211.1) which leads me to believe this regressed somewhat recently.
Comment 1 Radar WebKit Bug Importer 2023-11-08 07:44:03 PST
<rdar://problem/118118138>
Comment 2 Joshua Hoffman 2023-11-09 12:32:49 PST
Created attachment 468539 [details]
Patch
Comment 3 Joshua Hoffman 2023-11-09 13:36:02 PST
Created attachment 468540 [details]
Patch
Comment 4 Joshua Hoffman 2023-11-09 17:57:59 PST
Created attachment 468550 [details]
Patch
Comment 5 EWS 2023-11-10 09:47:59 PST
Committed 270542@main (ecb40fdcddf8): <https://commits.webkit.org/270542@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 468550 [details].
Comment 6 Steve Repsher 2023-11-30 11:49:27 PST
Would this fix also apply when the button role is outside the shadow DOM?  As in the following cases:

```
<my-element role="button">Just Text</my-element>
<my-element role="button"><span>Nested Text</span></my-element>
<button><my-element>Just Text</my-element></button>
<button><my-element><span>Nested Text</span></my-element></button>
```

where `shadowRoot.innerHTML` is simply `<div><slot></slot></div>`.

If so, could those situations please be added to the test cases as they are all broken as well?
Comment 7 Tyler Wilcock 2023-12-18 15:29:24 PST
*** Bug 265989 has been marked as a duplicate of this bug. ***
Comment 8 Steve Repsher 2023-12-18 16:28:45 PST
Tyler, you mentioned in the duplicate that this should be fixed in iOS 17.2, but that is definitely not the case for me.

Also, if you could please address my last comment, I would appreciate it so I know if I need to file an additional bug.
Comment 9 Tyler Wilcock 2023-12-18 17:20:03 PST
(In reply to Steve Repsher from comment #8)
> Tyler, you mentioned in the duplicate that this should be fixed in iOS 17.2,
> but that is definitely not the case for me.
Can you tell me more about this? I just tested Liam's attachment on iOS 17.2.1 (21C66), and VoiceOver reads the label of both buttons. In Settings > General > About > iOS Version, what version string is present?

> Would this fix also apply when the button role is outside the shadow DOM?  As in the following cases:
Yes, I would expect this fix to apply in those scenarios too. If you're still experiencing the wrong behavior, a separate bug with a Codepen would be greatly appreciated.

> If so, could those situations please be added to the test cases as they are all broken as well?
More test cases would be great. These might also be good candidates for web platform tests, e.g. new accname tests: https://wpt.fyi/results/accname?label=experimental&label=master&aligned
Comment 10 Steve Repsher 2023-12-19 05:33:12 PST
(In reply to Tyler Wilcock from comment #9)
> Can you tell me more about this? I just tested Liam's attachment on iOS
> 17.2.1 (21C66), and VoiceOver reads the label of both buttons. In Settings >
> General > About > iOS Version, what version string is present?

I'm on 17.2 (21C62), which is the latest for my phone at the moment.  Is the fix in the patch release?  I don't have a dev environment setup so I can test again when I get the patch.
Comment 11 Tyler Wilcock 2023-12-19 09:36:13 PST
(In reply to Steve Repsher from comment #10)
> (In reply to Tyler Wilcock from comment #9)
> > Can you tell me more about this? I just tested Liam's attachment on iOS
> > 17.2.1 (21C66), and VoiceOver reads the label of both buttons. In Settings >
> > General > About > iOS Version, what version string is present?
> 
> I'm on 17.2 (21C62), which is the latest for my phone at the moment.  Is the
> fix in the patch release?  I don't have a dev environment setup so I can
> test again when I get the patch.
Ah no, that version should have the change in this patch. When you say this isn't fixed, do you mean VoiceOver doesn't read both button labels in Liam's attachment? Or is it some other testcase not working right? Are you running a simulator, or a real device?
Comment 12 Steve Repsher 2023-12-19 10:36:28 PST
(In reply to Tyler Wilcock from comment #11)
> Ah no, that version should have the change in this patch. When you say this
> isn't fixed, do you mean VoiceOver doesn't read both button labels in Liam's
> attachment? Or is it some other testcase not working right? Are you running
> a simulator, or a real device?

So it does look like the attachment case here works okay, but I'm also testing an app with Material web components, and they still do not work.  They are all cases where the role lives outside the shadow root as I stated above.

Not minimal, but you can see a test case by going to https://demo.home-assistant.io/#/lovelace/0.  Click on the overflow button menu near the top, which displays a single menu item "Edit dashboard".  That text is not read.  It's slotted into the `<mwc-list role="menuitem">`.

I'm on a real device (iPhone XR).
Comment 13 Tyler Wilcock 2023-12-19 11:19:39 PST
(In reply to Steve Repsher from comment #12)
> (In reply to Tyler Wilcock from comment #11)
> > Ah no, that version should have the change in this patch. When you say this
> > isn't fixed, do you mean VoiceOver doesn't read both button labels in Liam's
> > attachment? Or is it some other testcase not working right? Are you running
> > a simulator, or a real device?
> 
> So it does look like the attachment case here works okay, but I'm also
> testing an app with Material web components, and they still do not work. 
> They are all cases where the role lives outside the shadow root as I stated
> above.
> 
> Not minimal, but you can see a test case by going to
> https://demo.home-assistant.io/#/lovelace/0.  Click on the overflow button
> menu near the top, which displays a single menu item "Edit dashboard".  That
> text is not read.  It's slotted into the `<mwc-list role="menuitem">`.
> 
> I'm on a real device (iPhone XR).
Got it, OK a separate bug would be greatly appreciated, thanks!
Comment 14 Steve Repsher 2023-12-20 11:25:07 PST
(In reply to Tyler Wilcock from comment #13)
> Got it, OK a separate bug would be greatly appreciated, thanks!

Here's one that I could boil down to a minimal repro that seems very related to this one (it only occurs with child slots):

https://bugs.webkit.org/show_bug.cgi?id=266717

I don't think that would fix the issue from my last comment though.  I'm still trying to make a minimal repro for that.  If I cannot, I'll just have to use the Home Assistant demo page.
Comment 15 Tyler Wilcock 2023-12-20 11:27:53 PST
(In reply to Steve Repsher from comment #14)
> (In reply to Tyler Wilcock from comment #13)
> > Got it, OK a separate bug would be greatly appreciated, thanks!
> 
> Here's one that I could boil down to a minimal repro that seems very related
> to this one (it only occurs with child slots):
> 
> https://bugs.webkit.org/show_bug.cgi?id=266717
> 
> I don't think that would fix the issue from my last comment though.  I'm
> still trying to make a minimal repro for that.  If I cannot, I'll just have
> to use the Home Assistant demo page.
Thanks Steve!
Comment 16 Steve Repsher 2023-12-21 14:06:52 PST
(In reply to Tyler Wilcock from comment #15)
> Thanks Steve!

Okay here's another with minimal repro:

https://bugs.webkit.org/show_bug.cgi?id=266789

That one does cover all the `<mwc-list-item>` components that are now broken, and also seems very related to this one.

And I'm not done...  v17 broke a lot of labels.
Comment 17 Joshua Hoffman 2023-12-21 14:25:41 PST
(In reply to Steve Repsher from comment #16)
> (In reply to Tyler Wilcock from comment #15)
> > Thanks Steve!
> 
> Okay here's another with minimal repro:
> 
> https://bugs.webkit.org/show_bug.cgi?id=266789
> 
> That one does cover all the `<mwc-list-item>` components that are now
> broken, and also seems very related to this one.
> 
> And I'm not done...  v17 broke a lot of labels.

Thanks for filing that, Steve! To help us investigate, did these just break in 17.2, or did you notice this behavior in 17 & 17.1 as well?
Comment 18 Steve Repsher 2023-12-21 14:33:25 PST
(In reply to Joshua Hoffman from comment #17)

> Thanks for filing that, Steve! To help us investigate, did these just break
> in 17.2, or did you notice this behavior in 17 & 17.1 as well?

Definitely in 17.1 as well.  I'm not certain about 17.0 though.  Here's the bug report on our end:

https://github.com/home-assistant/frontend/issues/18759
Comment 19 Joshua Hoffman 2023-12-21 14:37:26 PST
(In reply to Steve Repsher from comment #18)
> (In reply to Joshua Hoffman from comment #17)
> 
> > Thanks for filing that, Steve! To help us investigate, did these just break
> > in 17.2, or did you notice this behavior in 17 & 17.1 as well?
> 
> Definitely in 17.1 as well.  I'm not certain about 17.0 though.  Here's the
> bug report on our end:
> 
> https://github.com/home-assistant/frontend/issues/18759

Gotcha, thank you!
Comment 20 Steve Repsher 2023-12-21 20:49:17 PST
(In reply to Joshua Hoffman from comment #19)
> Gotcha, thank you!

And here's one more regression that's even more closely related to this one:

https://bugs.webkit.org/show_bug.cgi?id=266804

I can only reproduce it with `display: flex` as was mentioned in the original description here, even if it wasn't the actual cause.