Closed Bug 1473199 Opened 6 years ago Closed 6 years ago

[a11y] Permission controls not tabbable in identity popup

Categories

(Firefox :: Site Identity, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Firefox 63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 + fixed
firefox63 --- fixed

People

(Reporter: Jamie, Assigned: Jamie)

References

Details

(Keywords: access, regression)

Attachments

(1 file)

STR:
1. Open a page which asks for permission to display notifications and allow it to do so; e.g. http://www.bennish.net/web-notifications.html
2. Open the identity popup.
3. Try to tab to the "Clear this permission and ask again" button.
Expected: You should be able to.
Actual: you can't; it's not in the tab order.
4. Open a page which tries to open a pop-up; e.g. http://www.popuptest.com/popuptest1.html
5. Open the identity popup.
6. Try to tab to the choice concerning the "Allow pop-ups" permission.
Expected: You should be able to.
Actual: You can't; it's not in the tab order.

This could be solved now by applying the "subviewkeynav" class to these controls in _createPermissionItem in browser-siteIdentity.js. However, in bug 1467385 comment 7, it was suggested that:

> We should actually remove support for the "subviewkeynav" class and just include all buttons, toolbarbuttons, and menuitems in the navigable elements. We can add a "nokeynav" attribute if needed, although I can't think of any case right now where we want focus but only when using the mouse.

If we go down this path, we also need to include menulist (for the block pop-ups permission) and hyperlinked labels (for the "Show n blocked pop-ups" link). I'm not sure how to select hyperlinked labels. They have a "text-link" class, but that's not necessarily much more "future proof" than subviewkeynav.
Actually, this is a regression: it works fine in Firefox 60. I guess this is the same weirdness as bug 1467385. (I still don't quite understand why that is; see bug 1467385 comment 9.)

[Tracking Requested - why for this release]: This is an accessibility regression introduced in Firefox 62.
In the interest of getting this fixed for 62 (and thus minimising uplift risk), this patch just adds subviewkeynav where necessary, rather than altering the selector and eliminating subviewkeynav altogether.
Assignee: nobody → jteh
Comment on attachment 8989652 [details]
Bug 1473199: Make permission controls tabbable again in the identity popup.

https://reviewboard.mozilla.org/r/254656/#review262010

Nice, thank you!
Attachment #8989652 - Flags: review?(jhofmann) → review+
Priority: -- → P1
Status: NEW → ASSIGNED
Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4caf4cf4cf37
Make permission controls tabbable again in the identity popup. r=johannh
https://hg.mozilla.org/mozilla-central/rev/4caf4cf4cf37
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
New regression, tracking for 62.  
Jamie, can you request uplift to beta?
Flags: needinfo?(jteh)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #7)
> Jamie, can you request uplift to beta?

Shall do. I was originally waiting on Johann to request uplift for bug 1467385, but it seems the patches aren't dependent, just closely related. (They are very similar regressions.)
Flags: needinfo?(jteh)
Comment on attachment 8989652 [details]
Bug 1473199: Make permission controls tabbable again in the identity popup.

Approval Request Comment
[Feature/Bug causing the regression]: Bug https://bugzilla.mozilla.org/show_bug.cgi?id=1462469.
[User impact if declined]: Keyboard users won't be able to tab to the controls to adjust permissions in the site identity pop-up.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: This doesn't explicitly depend on anything, but bug 1467385 is very closely related and should also be uplifted.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Trivially adds an attribute with very well defined, easily verified behaviour.
[String changes made/needed]: None.
Attachment #8989652 - Flags: approval-mozilla-beta?
Blocks: 1462469
Blocks: 1473200
Comment on attachment 8989652 [details]
Bug 1473199: Make permission controls tabbable again in the identity popup.

Fix for a11y regression, let's uplift for beta 8.
Attachment #8989652 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.