Closed Bug 1525569 Opened 5 years ago Closed 5 years ago

The about:addons navigation menu is not keyboard navigable

Categories

(Toolkit :: Add-ons Manager, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla67
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- wontfix
firefox66 --- fixed
firefox67 --- verified

People

(Reporter: emilghitta, Assigned: mstriemer)

References

Details

(Keywords: regression)

Attachments

(2 files)

Attached image aboutaddons.gif

Affected versions

  • Firefox 65.0 (BuildId:20190124174741).
  • Firefox 66.0b5 (BuildId:20190204181317).
  • Firefox 67.0a1 (BuildId:20190205215922).

Unaffected versions

  • Firefox 60.5.0esr (BuildId:20190124141046).

Affected platforms

  • Windows 10 64bit.
  • macOS 10.11.6
  • Ubuntu 18.04 64bit.

Steps to reproduce

  1. Launch Firefox.
  2. Access the about:addons page.
  3. Select an option from the navigation menu (ex. “Extensions”).
  4. Press the up or down arrow key several times in order to navigate between the about:addons pages.

Expected result

  • Keyboard navigation between the about:addons page is successfully performed.

Actual result

  • Only 1 keyboard input is received and the navigation menu seems to loose focus.

Regression range

Additional notes

  • This issue is not reproducible with the about:preferences navigation menu.

Hi James,

It seems that mozregression pointed out Bug 1506787 for causing this regression.

Can you please take a look into this?

Thank you.

Flags: needinfo?(jteh)

A quick try shows that, upon each navigation between one page and another, focus is being grabbed by the vbox with id list-view or the like, e. g. an automatic focus jump, so you have to shift-tab several times to get back to the list view of Addons Manager pages.

Mark, can you look in to this? Can we remove the automatic focus change when changing panes on about:addons?

Flags: needinfo?(mstriemer)
Assignee: nobody → mstriemer

Previously the new views root node would be focused when the view changed, this
made it so that if you wanted to switch by more than one item up or down you'd
need to tab back to the sidebar each time you changed the view.

It's true that bug 1506787 technically regressed this. However, that's only because it made tabindex on non-control XUL elements behave like it does on HTML elements. Previously, <vbox tabindex="0"> would not make the vbox focusable. Now, it does. This means the .focus() call on the view would have previously been a no-op.

The question is: why was there a tabindex on that vbox even though it was a no-op? Given it was previously a no-op, should it simply be removed now?

Flags: needinfo?(jteh)

Actually, tabindex="0" really needs to be removed from these views (but not #detail-view) for two other reasons:

  1. Even if we don't force focus when the user navigates with the keyboard, the view container is still in the tab order, which is extraneous and wasn't the case before.
  2. For some reason I don't quite understand yet, when the #discover-view deck has tabindex="0", the document beneath gets completely removed from the accessibility tree. That's probably a bug somewhere in a11y, but there's no good reason for that deck to get focus anyway, and removing tabindex="0" fixes the issue.

From an accessibility perspective, these issues are pretty high priority, since they cause severe breakage in the add-ons manager for screen reader users.

I'm happy to provide a patch which removes tabindex="0" (and the then no-op .focus call) from these views, since I'm technically the one who regressed this. However, I also see this patch sets the keyboard-navigation attribute and I don't really understand what that does (there's some reference in the CSS?).

See Also: → 1525937

I also noticed that and started removing them then left them since I wasn't sure what they were added for. I filed bug 1525937 to clean that up. I'll try to put a patch together today and send it your way to see if it fixes everything.

The keyboard-navigation attribute is just used to avoid showing the outline when you click on the sidebar items with the mouse. It's just used for this CSS https://searchfox.org/mozilla-central/rev/e00ea598e52bbb35f8c45abf9c2eade17962bb5e/toolkit/themes/shared/in-content/common.inc.css#613.

Pushed by mstriemer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c508599012ee
Fix sidebar nav with keyboard in about:addons r=jaws
Priority: -- → P2
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Flags: qe-verify+
Component: Keyboard: Navigation → Add-ons Manager
Product: Core → Toolkit

This issue is verified fixed using Firefox 67.0a1 (BuildId:20190210213844) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 14.04 64bit

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Has Regression Range: --- → yes

Comment on attachment 9041966 [details]
Bug 1525569 - Fix sidebar nav with keyboard in about:addons r?jaws

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1506787

User impact if declined

Navigating between pages in about:addons with the keyboard is difficult and slow.

Is this code covered by automated tests?

No

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

Yes

If yes, steps to reproduce

See comment 0.

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Removes a focus() call and adds keyboard navigation CSS. Has been verified.

String changes made/needed

Attachment #9041966 - Flags: approval-mozilla-beta?

Comment on attachment 9041966 [details]
Bug 1525569 - Fix sidebar nav with keyboard in about:addons r?jaws

Fix for keyboard navigation, verified in Nightly.
OK for uplift for beta 8.

Attachment #9041966 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: