The about:addons navigation menu is not keyboard navigable
Categories
(Toolkit :: Add-ons Manager, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox65 | --- | wontfix |
firefox66 | --- | fixed |
firefox67 | --- | verified |
People
(Reporter: emilghitta, Assigned: mstriemer)
References
Details
(Keywords: regression)
Attachments
(2 files)
562.81 KB,
image/gif
|
Details | |
47 bytes,
text/x-phabricator-request
|
lizzard
:
approval-mozilla-beta+
|
Details | Review |
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
- Launch Firefox.
- Access the about:addons page.
- Select an option from the navigation menu (ex. “Extensions”).
- 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
-
Last good revision: 7be95c95a3a7254bcff022190ab2be27ca27ac50
-
First bad revision: 38844846c6ae6773afb8b9254bf3dbfec437d1ec
-
Potential regressor: Bug 1506787
Additional notes
- This issue is not reproducible with the about:preferences navigation menu.
Reporter | ||
Comment 1•5 years ago
|
||
Hi James,
It seems that mozregression pointed out Bug 1506787 for causing this regression.
Can you please take a look into this?
Thank you.
Comment 2•5 years ago
|
||
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.
Comment 3•5 years ago
|
||
Mark, can you look in to this? Can we remove the automatic focus change when changing panes on about:addons?
Assignee | ||
Comment 4•5 years ago
|
||
Looks like the view is being focuses here https://searchfox.org/mozilla-central/rev/9eb30227b21e0aa40d51d9f9b08bb0b113c5fadb/toolkit/mozapps/extensions/content/extensions.js#863.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 5•5 years ago
|
||
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.
Comment 6•5 years ago
|
||
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?
Comment 7•5 years ago
|
||
Actually, tabindex="0" really needs to be removed from these views (but not #detail-view) for two other reasons:
- 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.
- 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.
Comment 8•5 years ago
|
||
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?).
Assignee | ||
Comment 9•5 years ago
|
||
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.
Comment 10•5 years ago
|
||
Pushed by mstriemer@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c508599012ee Fix sidebar nav with keyboard in about:addons r=jaws
Updated•5 years ago
|
Comment 11•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Updated•5 years ago
|
Reporter | ||
Comment 12•5 years ago
|
||
This issue is verified fixed using Firefox 67.0a1 (BuildId:20190210213844) on Windows 10 64bit, macOS 10.13.6 and Ubuntu 14.04 64bit
Updated•5 years ago
|
Assignee | ||
Comment 13•5 years ago
|
||
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
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
Comment 14•5 years ago
|
||
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.
Updated•5 years ago
|
Comment 15•5 years ago
|
||
bugherder uplift |
Description
•