The new buttons that can be added to the toolbar don't announce themselves to screen readers as buttons.

VERIFIED FIXED in Firefox 57

Status

()

Firefox
Disability Access
P1
normal
VERIFIED FIXED
11 months ago
9 months ago

People

(Reporter: MarcoZ, Assigned: adw)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 verified)

Details

(Whiteboard: [reserve-photon-structure])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

11 months ago
The new buttons that can be added to the toolbar don't appear to screen readers as buttons, but as grphics. The name and accessible description are OK, but the role isn't. Screen readers think these are simple graphics. But they should be buttons so screen readers know that they can click on them. The click action works, at least with NVDA, but others might not be as forgiving.
Blocks: 1352697
No longer blocks: 1387042
Whiteboard: [photon-structure][triage]

Updated

11 months ago
Flags: qe-verify+
Priority: -- → P3
QA Contact: gwimberly
Whiteboard: [photon-structure][triage] → [reserve-photon-structure]
(Assignee)

Updated

11 months ago
Assignee: nobody → adw
Status: NEW → ASSIGNED
(Assignee)

Comment 1

11 months ago
These icons are <image> nodes.  We've had images in the urlbar for a long time now, like the bookmark star button, and as far as I can tell, we haven't used any accessibility-related markup on them.

So I have a couple of questions, Marco:

First, I'm wondering how image nodes were presented in the past.  For example, did the bookmark star button previously announce itself as a button?  Or the reader mode button or the page compatibility button?

Second, I tried adding role=button to these images, but that doesn't seem to change how NVDA's speech reader presents them.  NVDA still just speaks their tooltips.  What should I be looking for exactly?
Flags: needinfo?(mzehe)
(Assignee)

Comment 2

11 months ago
Oh, looks like there's an NVDA option to report roles.  Indeed, I see that these new icons are read as "graphic".  When I add role=button, they're read as "button".  Interestingly the reader mode button is also read as "button", but I don't understand why.

I also see that the Pocket button isn't being read correctly.
Flags: needinfo?(mzehe)
Comment hidden (mozreview-request)
(Assignee)

Comment 4

11 months ago
Summary:

* Add role=button for all <image> buttons in markup.  NVDA already speaks the reader mode button as "button", and I don't know why, but adding a role for it seems like a good idea anyway.

* When creating urlbar buttons for page actions, set role=button

* Update Pocket's button: add role=button, and also set a tooltip (actually on both images, which seems to be necessary, same as the bookmark star)

Updated

11 months ago
Iteration: --- → 57.3 - Sep 19
Priority: P3 → P1

Comment 5

11 months ago
mozreview-review
Comment on attachment 8902898 [details]
Bug 1395178 - The new buttons that can be added to the toolbar don't announce themselves to screen readers as buttons.

https://reviewboard.mozilla.org/r/174614/#review179694
Attachment #8902898 - Flags: review?(jaws) → review+

Comment 6

11 months ago
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/91cd19239d21
The new buttons that can be added to the toolbar don't announce themselves to screen readers as buttons. r=jaws
https://hg.mozilla.org/mozilla-central/rev/91cd19239d21
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57

Comment 8

11 months ago
Did you mean to set role=button both on the <hbox> and on the <image> for the items that have both (bookmarks star, pocket) ? That doesn't seem quite right...
Flags: needinfo?(adw)
(Reporter)

Comment 9

11 months ago
Right, that set bookmarks thing is funny, it has two buttons that sound the same, and the parent, supposedly a grouping, is announced as a button, too. The other buttons are fine in that toolbar.
(Assignee)

Comment 10

11 months ago
Thanks, no I don't think I meant to set role on both the hbox and its buttons, only the buttons.  I'll open a new bug once I'm clear on how to fix the bookmarks button.

Marco, this bookmarks parent box has two images in it, but only so that we can implement an animation when you click it.  Conceptually it's just one button with one image.  How should the roles be set here?  The Pocket button is similar by the way, except I didn't goof up and add role=button to its outer hbox.

Here is the bookmark button's (incorrect) markup as it's currently written, in case it helps:

<hbox id="star-button-box"
      hidden="true"
      class="urlbar-icon-wrapper urlbar-page-action"
      role="button"
      context="pageActionPanelContextMenu"
      oncontextmenu="BrowserPageActions.onContextMenu(event);"
      onclick="BrowserPageActions.bookmark.onUrlbarNodeClicked(event);">
  <image id="star-button"
         class="urlbar-icon"
         role="button"
         observes="bookmarkThisPageBroadcaster"/>
  <hbox id="star-button-animatable-box">
    <image id="star-button-animatable-image"
           role="button"
           observes="bookmarkThisPageBroadcaster"/>
  </hbox>
</hbox>
Flags: needinfo?(adw) → needinfo?(mzehe)
(Reporter)

Comment 11

11 months ago
(In reply to Drew Willcoxon :adw from comment #10)
> Marco, this bookmarks parent box has two images in it, but only so that we
> can implement an animation when you click it.  Conceptually it's just one
> button with one image.  How should the roles be set here?  The Pocket button
> is similar by the way, except I didn't goof up and add role=button to its
> outer hbox.

And you didn't add a button role to the animatable image I think, since I don't see that, only one Pocket button.

> Here is the bookmark button's (incorrect) markup as it's currently written,
> in case it helps:
> 
> <hbox id="star-button-box"
>       hidden="true"
>       class="urlbar-icon-wrapper urlbar-page-action"
>       role="button"

Remove the role.

>       context="pageActionPanelContextMenu"
>       oncontextmenu="BrowserPageActions.onContextMenu(event);"
>       onclick="BrowserPageActions.bookmark.onUrlbarNodeClicked(event);">
>   <image id="star-button"
>          class="urlbar-icon"
>          role="button"
>          observes="bookmarkThisPageBroadcaster"/>
>   <hbox id="star-button-animatable-box">
>     <image id="star-button-animatable-image"
>            role="button"

Make this role="presentation".

>            observes="bookmarkThisPageBroadcaster"/>
>   </hbox>
> </hbox>

And to be on the safe side, do the same for the Pocket button, also for consistency.
Flags: needinfo?(mzehe)
(Assignee)

Updated

11 months ago
Depends on: 1397609
(Assignee)

Comment 12

11 months ago
Thanks Marco.  I've moved this to the new bug 1397609.  I'll post a patch there and a try build.
Hi Drew!

I managed to verify this issue using NVDA on Windows 10 64bit.

I didn't managed to verify this using Orca Screen reader on linux and VoiceOver on mac (didn't managed to find the options for reporting the button roles).

Could you help me out with this one or is it safe to verify this issue only with NVDA?
Flags: needinfo?(adw)
(Assignee)

Comment 14

9 months ago
IIRC Marco has said before that Firefox doesn't work well VoiceOver in general, so I'm not sure whether it's expected to work here.  I don't know about Orca.  Marco, do you know the answer to Emil's questions in comment 13?
Flags: needinfo?(adw) → needinfo?(mzehe)
(Assignee)

Comment 15

9 months ago
Emil, also, this bug didn't really fix the problem right.  It might be better for you to verify only bug 1397609 instead.
(Reporter)

Comment 16

9 months ago
It's safe to verify with NVDA. I don't know the Orca commands to get to buttons that aren't tabable off-hand, but if it works with NVDA on Windows, it usually works with Orca, too. It theoretically also works with VoiceOver on the Mac, but Mac a11y is currently in a stale state.
Flags: needinfo?(mzehe)
Marking this as verified fixed as per comment 13 and comment 16.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.