Add aria-pressed to the filter buttons

VERIFIED FIXED in Firefox 52

Status

()

Firefox
Developer Tools: Netmonitor
P1
normal
VERIFIED FIXED
10 months ago
10 months ago

People

(Reporter: derek.riemer, Assigned: rickychien)

Tracking

(Blocks: 1 bug, {access, regression})

54 Branch
Firefox 54
access, regression
Points:
---

Firefox Tracking Flags

(firefox52 verified, firefox-esr52 verified, firefox53 verified, firefox54 verified)

Details

(Whiteboard: [netmonitor])

MozReview Requests

()

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

Attachments

(3 attachments)

(Reporter)

Description

10 months ago
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170130030205

Steps to reproduce:

1. Press control+shift+k
2. Shift tab backwards to find the "toggle filter bar" button.
3. press it. It changes, but this information isn't expressed to screen readers.
4. tab through the filter bar.
5. Press space on debug.
6. Note that the pressed state iisn't added. A class of "Checked" is however added if I look at the IA2Text NVDA gets.

Proposed fix (I'm not familiar with firefox so correct me if this isn't being done with html).

add aria-pressed, and set it to true when appropriate.
(Reporter)

Comment 1

10 months ago
Release Note Request (optional, but appreciated)
[Why is this notable]:
[Affects Firefox for Android]:
[Suggested wording]:
[Links (documentation, blog post, etc)]:
Flags: needinfo?
Keywords: access
Yura, are you and Nancy tracking these? Seems some rewrites caused regressions here, too.
Flags: needinfo? → needinfo?(yzenevich)
Status: UNCONFIRMED → NEW
Component: Untriaged → Developer Tools: Console
Ever confirmed: true

Comment 3

10 months ago
:( , regression from bug 1309191.

Unfortunate as this is an indicator of things to come with the move away from XUL.

Checked on XUL button was not used just for styling purposes. It is a valid attribute that carries accessibility information.
Flags: needinfo?(yzenevich) → needinfo?(rchien)
Keywords: regression
(Assignee)

Updated

10 months ago
Assignee: nobody → rchien
Status: NEW → ASSIGNED
Flags: needinfo?(rchien)
Comment hidden (mozreview-request)
(Assignee)

Updated

10 months ago
Component: Developer Tools: Console → Developer Tools: Netmonitor
Whiteboard: [netmonitor][triage]
Comment hidden (mozreview-request)
(Assignee)

Comment 6

10 months ago
Yura, patch has uploaded for addressing a11y issue, please take a look. Thanks!

Comment 7

10 months ago
mozreview-review
Comment on attachment 8832740 [details]
Bug 1335322 - Add aria-pressed to the filter buttons for beta

https://reviewboard.mozilla.org/r/108960/#review110162

Looks good thanks. Would you mind adding (or in case there are already, expanding) a test for this change too so we do not regress in the future?
Also forwarding the r? to Honza, since I'm not a devtools peer.
Attachment #8832740 - Flags: review?(yzenevich) → review+

Updated

10 months ago
Attachment #8832740 - Flags: review?(odvarko)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 10

10 months ago
Updated patch for supporting aria-pressed attribute check in testFilterButtonsCustom() helper function. Therefore, aria-pressed a11y test will run on all browser_net_filter-xx.js tests.

Comment 11

10 months ago
(In reply to Ricky Chien [:rickychien] from comment #10)
> Updated patch for supporting aria-pressed attribute check in
> testFilterButtonsCustom() helper function. Therefore, aria-pressed a11y test
> will run on all browser_net_filter-xx.js tests.

This is great, thanks!
@Yura: the patch looks good and seem to be fixing Filter buttons in the Network panel. But, the bug's name indicates that there is a problem with Console's filter bar. Shouldn't we rename the bug?

Honza
Flags: needinfo?(yzenevich)

Comment 13

10 months ago
(In reply to Jan Honza Odvarko [:Honza] from comment #12)
> @Yura: the patch looks good and seem to be fixing Filter buttons in the
> Network panel. But, the bug's name indicates that there is a problem with
> Console's filter bar. Shouldn't we rename the bug?
> 
> Honza

Duh, totally missed that. Yes this is also the case where aria-pressed was supposed to be added. May I suggest, if possible, adding this button component (with fixed a11y) to shared components and using it in both places. If not , new console filter button should be fixed as well..
Flags: needinfo?(yzenevich) → needinfo?(rchien)
(Assignee)

Comment 14

10 months ago
Ah, good catch! Filter buttons should move to shared component for webconsole and netmonitor. I'd prefer to fix aria-pressed for new console filter in a separate bug. And then create a a11y supported filter buttons shared component for general purpose.
Flags: needinfo?(rchien)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(In reply to Ricky Chien [:rickychien] from comment #14)
> I'd prefer to fix aria-pressed for new console
> filter in a separate bug. And then create a a11y supported filter buttons
> shared component for general purpose.
I agree.

Honza

Updated

10 months ago
Blocks: 1307743
Flags: qe-verify?

Updated

10 months ago
Iteration: --- → 54.1 - Feb 6
Flags: qe-verify? → qe-verify+
Priority: -- → P1
QA Contact: ciprian.georgiu
Whiteboard: [netmonitor][triage] → [netmonitor]
(Assignee)

Updated

10 months ago
Summary: Add aria-pressed to the toggle buttons in the nightly developer console's filter bar → Add aria-pressed to the filter buttons

Updated

10 months ago
Iteration: 54.1 - Feb 6 → 54.2 - Feb 20
Attachment #8832740 - Flags: review+

Comment 18

10 months ago
mozreview-review
Comment on attachment 8832740 [details]
Bug 1335322 - Add aria-pressed to the filter buttons for beta

https://reviewboard.mozilla.org/r/108960/#review111524

R+, but please fix the commit message.

It should be:
Bug 1335322 - Add aria-pressed to filter buttons r?honza


Honza
Attachment #8832740 - Flags: review?(odvarko) → review+
Comment hidden (mozreview-request)

Comment 20

10 months ago
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/71fcf02e3159
Add aria-pressed to filter buttons r=Honza,yzen

Comment 21

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/71fcf02e3159
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
IIUC, 52/53 are also affected here, so please request Aurora/Beta approval on this when you get a chance.
status-firefox52: --- → affected
status-firefox53: --- → affected
Flags: needinfo?(rchien)
(Assignee)

Comment 23

10 months ago
Thanks for reminding, I will submit two different hotfix for aurora and beta.
Flags: needinfo?(rchien)
Comment hidden (mozreview-request)
(Assignee)

Comment 25

10 months ago
Created attachment 8834747 [details] [diff] [review]
patch for aurora
Comment hidden (mozreview-request)
(Assignee)

Comment 27

10 months ago
Created attachment 8834749 [details] [diff] [review]
patch for beta
(Assignee)

Comment 28

10 months ago
Comment on attachment 8834747 [details] [diff] [review]
patch for aurora

Approval Request Comment
[Feature/Bug causing the regression]: bug 1309191
[User impact if declined]: minor
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: yes, see https://bugzilla.mozilla.org/show_bug.cgi?id=1335322#c0
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no, very low
[Why is the change risky/not risky?]: a11y regression
[String changes made/needed]: no
Attachment #8834747 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 29

10 months ago
Comment on attachment 8834749 [details] [diff] [review]
patch for beta

Approval Request Comment
[Feature/Bug causing the regression]: bug 1309191
[User impact if declined]: minor
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: yes, see https://bugzilla.mozilla.org/show_bug.cgi?id=1335322#c0
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no, very low
[Why is the change risky/not risky?]: a11y regression
[String changes made/needed]: no
Attachment #8834749 - Flags: approval-mozilla-beta?

Comment 30

10 months ago
Hi Rares,
could you help find someone to verify if this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(rares.bologa)
(Assignee)

Comment 31

10 months ago
Cristian, could you help us verify this issue? Thanks!
Flags: needinfo?(rares.bologa) → needinfo?(cristian.comorasu)
(In reply to Ricky Chien [:rickychien] from comment #31)
> Cristian, could you help us verify this issue? Thanks!

Hello Ricky!
The QA contact for this bug is my colleague Ciprian, he will verify this bug as soon as possible!

Cheers!
Flags: needinfo?(cristian.comorasu) → needinfo?(ciprian.georgiu)
Comment on attachment 8834749 [details] [diff] [review]
patch for beta

devtools a11y fix, beta52+
Attachment #8834749 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 34

10 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/e51baf47ffeb
status-firefox52: affected → fixed

Comment 35

10 months ago
Comment on attachment 8834747 [details] [diff] [review]
patch for aurora

Fix a a11y issue related to devtools. Aurora53+.
Attachment #8834747 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 36

10 months ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/a57e10371afcb34942a2df45004348b96f429919
status-firefox53: affected → fixed

Comment 37

10 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr52/rev/e51baf47ffeb
status-firefox-esr52: --- → fixed
(Assignee)

Updated

10 months ago
Flags: needinfo?(ciprian.georgiu)
I've managed to verify this bug using Browser Toolbox, and I can confirm that, all the network filter buttons have now the HTML atribute set "aria-pressed".

This is verified fixed on latest Nightly 54.0a1 (2017-02-26), latest Aurora 53.0a2 (2017-02-26), 52.0b9 (20170223185858) and esr52 tinderbox build (20170226071209). Tests were done under Windows 10 x64, Ubuntu 16.04 x64 LTS and Mac OS X 10.11.6.
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
status-firefox53: fixed → verified
status-firefox54: fixed → verified
status-firefox-esr52: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.