1477354 - Win 7, Ubuntu and Mac accessibility tools do not read door-hanger menu items

Status: VERIFIED FIXED

(DevTools :: General, defect, P3)

Description

[Abe - QA (:Abe_LV)]

Steps to reproduce:

1. Enable voice over (system preferences > accessibility > VoiceOver) 
2. Open door-hanger menus
3. Highlight a menu item using the mouse
4. Check if it reads the menu items

Expected Result:
Voice over should read the menu items.

Actual Result
Voice over does not read menu items.

Comment 1

[Abe - QA (:Abe_LV)]

This is reproducible in both Chevron and meatballs door-hanger menu items.

Comment 2

[Abe - QA (:Abe_LV)]

In addition to Mac, this issue also reproducible in Windows 7 Narrator and Ubuntu screen reader. These accessibility tools do not read the door-hanger menu items text.

But, it works fine with Windows 10 narrator.

Comment 3

[Brian Birtles (:birtles)]

James or Yura, are you able to give me advice about how to make these menus readable by screen readers?

For context, this is about the "meatball menu" in Devtools (the menu with the ... prompt) which we have recently replaced with a doorhanger-style menu. It is mostly done in HTML with one XUL <panel> element so that it can extend beyond the window bounds.

I tried to be careful to use semantic tags and aria-expanded, aria-haspopup="menu", and aria-controls=<id> etc. to define the menu popup. The menu items referred to in this bug are just <span> elements, inside a <button>, inside an <li>.

Comment 4

[James Teh [:Jamie]]

(In reply to Abe - QA (:Abe_LV) from comment #0)
> 3. Highlight a menu item using the mouse
> Expected Result:
> Voice over should read the menu items.

I'm not sure what VoiceOver's default mouse reading behaviour is; I'd need to check and don't have a Mac to hand. However, it's possible that reading items under the mouse is not default behaviour. Also, Firefox accessibility is somewhat broken on mac at best, so I would probably disregard Mac at this point for the purposes of this bug.

(In reply to Abe - QA (:Abe_LV) from comment #2)
> In addition to Mac, this issue also reproducible in Windows 7 Narrator and
> Ubuntu screen reader. These accessibility tools do not read the door-hanger
> menu items text.

Steps to reproduce? Are you using the mouse or the keyboard? Again, reading items under the mouse may not be default behaviour for these screen readers. I'm pretty certain this is true for Narrator at least. Please test with the keyboard and report your findings. If you do want to test with the mouse, that's valid, but please check the screen reader's mouse related settings.

For me, this works as expected if I use the keyboard with NVDA. As I press the arrow keys, the focused menu item is reported. If I route the mouse to any other item (with mouse tracking enabled), that is also reported as expected.

One thing I would suggest is that the <li> elements are marked with role="presentation". Right now, all the menu items report "1 of 1" for their position information because the menuitem is the only child of its parent. role="presentaition" eliminates the <li> parent for accessibility, which will allow the menu items to be counted correctly.

Comment 5

[Brian Birtles (:birtles)]

That's great. Thanks James!

Comment 6

[Brian Birtles (:birtles)]

> One thing I would suggest is that the <li> elements are marked with role="presentation". Right now, all the menu items report "1 of 1" for their position information because the menuitem is the only child of its parent. role="presentaition" eliminates the <li> parent for accessibility, which will allow the menu items to be counted correctly.

For my own reference I misunderstood this at first because we have a MenuItem component that sets class="menuitem" but Jamie is referring to the role="menuitem" elements here.

Specifically, we currently have a structure something like simplified example below:

  <div role="menu" id="toolbox-meatball-menu">
    <li class="menuitem">
      <button
        role="menuitemcheckbox"
        aria-checked="true">
        <span class="label">Dock to Bottom</span>
      </button>
    </li>
    <li class="menuitem">
      <button
        role="menuitemcheckbox">
        <span class="label">Dock to Right</span>
      </button>
    </li>
    <hr></hr>
    <li class="menuitem">
      <button
        class="command"
        role="menuitem">
        <span class="label">Documentation…</span>
      </button>
    </li>
  </div>

In particular, for that last <button role="menuitem"> element there is a <li> wrapper between it and the <div role="menu"> element. Marking the <li> element as role="presentation" should hopefully give a more sensible accessibility tree.

Comment 7

[Brian Birtles (:birtles)]

The accessibility inspector too reveals a more sensible accessibility tree if I do that. (Curiously the aria-checked value doesn't seem to be reflected for the menuitemcheckbox items, however.)

Comment 8

[Brian Birtles (:birtles)]

Attachment: Bug 1477354 - Add role=presentation to MenuItem wrapper element; r?Jamie

Currently we have a structure like the following:

  <div role="menu" id="toolbox-meatball-menu">
    <li class="menuitem">
      <button
        role="menuitemcheckbox"
        aria-checked="true">
        <span class="label">Dock to Bottom</span>
      </button>
    </li>
    <li class="menuitem">
      <button
        role="menuitemcheckbox">
        <span class="label">Dock to Right</span>
      </button>
    </li>
    <hr></hr>
    <li class="menuitem">
      <button
        class="command"
        role="menuitem">
        <span class="label">Documentation…</span>
      </button>
    </li>
  </div>

As a result there is an <li> wrapper element between the role="menuitem" element
at the role="menu" element. By setting role="presentation" on the <li> wrapper
we can produce a more sensible accessibility tree.

Comment 9

[Phabricator Automation]

Comment on attachment 8996918 [details]
Bug 1477354 - Add role=presentation to MenuItem wrapper element; r?Jamie

James Teh [:Jamie] has approved the revision.

https://phabricator.services.mozilla.com/D2642

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b45de18ed240
Add role=presentation to MenuItem wrapper element; r=Jamie

Comment 11

[James Teh [:Jamie]]

(In reply to Brian Birtles (:birtles) from comment #7)
> (Curiously the aria-checked value doesn't seem to be reflected
> for the menuitemcheckbox items, however.)

I do see the correct checked/not checked state in Nightly. It's hard for me to test this with the Inspector, as I can't interact with the Inspector without the menu closing (as a screen reader user).

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/b45de18ed240

Comment 13

[Abe - QA (:Abe_LV)]

I retested this on OSX 10.13, Windows 7 and Ubuntu 16.04 with the latest Nightly. Voiceover, narrator, and screen-reader are now reading the door-hanger menu items. The issue is verified as fixed. 


One of the test env'ts
---------------------
Version 	63.0a1
Build ID 	20180814100100
Update Channel 	nightly
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:63.0) Gecko/20100101 Firefox/63.0