Closed Bug 1335322 Opened 7 years ago Closed 7 years ago

Add aria-pressed to the filter buttons

Categories

(DevTools :: Netmonitor, defect, P1)

Product:
DevTools
Component:
Netmonitor
Version:
54 Branch
Type:
defect

Tracking

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

Status:
VERIFIED FIXED
Milestone:
Firefox 54
Iteration:
54.2 - Feb 20
Tracking Flags:
Tracking Status
firefox52 --- verified
firefox-esr52 --- verified
firefox53 --- verified
firefox54 --- verified

People

(Reporter: derek.riemer, Assigned: rickychien)

References

(Blocks 1 open bug)

Details

(Keywords: access, regression, Whiteboard: [netmonitor])

Attachments

(3 files)

Bug 1335322 - Add aria-pressed to the filter buttons for beta
59 bytes, text/x-review-board-request
Honza
: review+
Details
patch for aurora
1.04 KB, patch
gchang
: approval-mozilla-aurora+
Details | Diff | Splinter Review
patch for beta
1.03 KB, patch
jcristau
: approval-mozilla-beta+
Details | Diff | Splinter Review 
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.
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
:( , 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: nobody → rchien
Status: NEW → ASSIGNED
Flags: needinfo?(rchien)
Component: Developer Tools: Console → Developer Tools: Netmonitor
Whiteboard: [netmonitor][triage]
Yura, patch has uploaded for addressing a11y issue, please take a look. Thanks!
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 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.
(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)
(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)
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)
(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
Blocks: netmonitor-html
Flags: qe-verify?
Iteration: --- → 54.1 - Feb 6
Flags: qe-verify? → qe-verify+
Priority: -- → P1
QA Contact: ciprian.georgiu
Whiteboard: [netmonitor][triage] → [netmonitor]
Summary: Add aria-pressed to the toggle buttons in the nightly developer console's filter bar → Add aria-pressed to the filter buttons
Iteration: 54.1 - Feb 6 → 54.2 - Feb 20
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+
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/71fcf02e3159
Add aria-pressed to filter buttons r=Honza,yzen
https://hg.mozilla.org/mozilla-central/rev/71fcf02e3159
Status: ASSIGNED → RESOLVED
Closed: 7 years 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)
Thanks for reminding, I will submit two different hotfix for aurora and beta.
Flags: needinfo?(rchien)
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?
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?
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)
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 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+
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+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: