Closed
Bug 1395178
Opened 7 years ago
Closed 7 years ago
The new buttons that can be added to the toolbar don't announce themselves to screen readers as buttons.
Categories
(Firefox :: Disability Access, defect, P1)
Firefox
Disability Access
Tracking
()
Tracking | Status | |
---|---|---|
firefox57 | --- | verified |
People
(Reporter: MarcoZ, Assigned: adw)
References
(Blocks 1 open bug)
Details
(Whiteboard: [reserve-photon-structure])
Attachments
(1 file)
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.
Updated•7 years ago
|
Updated•7 years ago
|
Flags: qe-verify+
Priority: -- → P3
QA Contact: gwimberly
Whiteboard: [photon-structure][triage] → [reserve-photon-structure]
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → adw
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•7 years 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•7 years 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•7 years 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•7 years ago
|
Iteration: --- → 57.3 - Sep 19
Priority: P3 → P1
Comment 5•7 years 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+
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
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/91cd19239d21
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Comment 8•7 years 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•7 years 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•7 years 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•7 years 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 | ||
Comment 12•7 years ago
|
||
Thanks Marco. I've moved this to the new bug 1397609. I'll post a patch there and a try build.
Comment 13•7 years ago
|
||
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•7 years 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•7 years 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•7 years 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)
Comment 17•7 years ago
|
||
Marking this as verified fixed as per comment 13 and comment 16.
You need to log in
before you can comment on or make changes to this bug.
Description
•