Closed Bug 1567600 Opened 5 years ago Closed 5 years ago

Extensions list in Addon Manager: cannot access with JAWS/NVDA

Categories

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

68 Branch
Desktop
All
defect

Tracking

()

VERIFIED FIXED
mozilla70
Tracking Status
relnote-firefox --- 69+
firefox-esr60 --- unaffected
firefox-esr68 70+ verified
firefox68 --- wontfix
firefox69 + verified
firefox70 --- verified
firefox71 --- verified

People

(Reporter: sailesh.panchang, Assigned: mstriemer)

References

(Regressed 2 open bugs)

Details

(Keywords: access, regression)

Attachments

(10 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
8.74 KB, patch
Details | Diff | Splinter Review
10.13 KB, patch
Details | Diff | Splinter Review
16.05 KB, patch
Details | Diff | Splinter Review
8.73 KB, patch
Details | Diff | Splinter Review
10.13 KB, patch
Details | Diff | Splinter Review
16.05 KB, patch
Details | Diff | Splinter Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

ps:

  1. .Use JAWS or NVDA (2019 version) to access addon manager using Alt+T+A
  2. Tab to list with Extensions, Themes, ... Plug-ins. These can be navigated with up/down arrow keys.
  3. Select Extensions i.e. arrow to this choice

Actual results:

It does not work as expected in Firefox 68 anymore. A screenreader user cannot navigate to the klistbox of available extensions and arrow up/down these.
Visually the entire listbox appears to have focus and appears selected.

Expected results:

With Extensions selected in listbox, a few tabs down one should expect tthat keyboard focus moves to the list box with various extensions and one should be able to arrow up/down the list of available extensions .
tabbing should permit one to review options / disable the addon etc.
This is how it used to work previously in Firefox production release and still does in ESR60.

Component: Untriaged → Disability Access APIs
Product: Firefox → Core

[Tracking Requested - why for this release]: Accessibility regression in the Add-Ons Manager redesign that landed in Firefox 68. Once fixes are in place, uplifts should be considered to 69 beta and 68 ESR.

Status: UNCONFIRMED → NEW
Component: Disability Access APIs → Add-ons Manager
Ever confirmed: true
Keywords: access, regression
OS: Unspecified → All
Priority: -- → P2
Product: Core → Toolkit
Hardware: Unspecified → Desktop

Note that add-ons manager has been redesigned and that interaction is now slightly different. The page is now laid out more like a document rather than a traditional list of items.

The page in question has the following problems:

  1. After the redesign, the "Active" and "inactive" texts are proper Headings level 2. However, the add-ons themselves should then be part of the heading hierarchy so screen reader users can discover them easily, now that arrow navigation is no longer available and the whole page becomes a document. So the span element with class "addon-name" must be converted into an H3, or if that is for some reason not possible, role="heading" aria-level="3" be added to it. However, making it a proper h3 with proper styling is definitely the more preferred method.
  2. The image preceding the add-on name currently has no alt text. It should contain the add-on name. This is the img tag with class "card-heading-icon addon-icon". Also, since it acts as a button to load the details panel for the add-on, it should probably be turned into a proper button and the name changed to something like "Details for <addon name>".
  3. The button with class "more-options-button ghost-button" has no label. It should be given a label either via one of the button element's normal attributes, or via aria-label, and a localized string of "More Options". Moreover, to indicate it has a popup menu attached it needs the attributes aria-haspopup="menu" and aria-expanded="true" or "false", depending on whether that menu is currently showing or not.
  4. There is currently a tabstop on the span with class "addon-description", which is only read to screen readers as "text frame". If this was intended to give AT users a quick summary of what this add-on is, it should get an aria-label with name and description. However, if this is not at all actionable anyway, being able to set focus to the button for each add-on is probably enough. Keyboard users and screen reader users have slightly different interaction patterns, and with a proper heading hierarchy (see above), the tab stop on the description is not really needed.

The menu, done via a span with class "panel-list" has several problems of its own:

  1. It should get a role of "menu".
  2. Each button should then get a role of "menuitem"., the intermediary div should get a role of "presentation".
  3. Once the menu button is pressed via the keyboard, and the menu opens, focus should be set to the first item.
  4. Tab should be trapped, not be able to move anywhere outside that menu.
  5. Up and down arrows should move among menu items, wrap around at the top and bottom.
  6. Escape should close the menu and return focus to the More Options button that opened it.

The add-on details panel also has a bunch of problems of its own.

  1. When it loads, this is not being indicated because focus isn't set to anything.
  2. The main addon-card element should get a role of "dialog". It should also get an aria-label pointing to the heading that has the add-on name.
  3. As above, the add-on name should be a heading, but since this is an enclosed piece of information, it should probably be an h1 or h2 this time.
  4. The container for the buttons that contain Details and Permissions buttons, should be a role "tablist". The buttons themselves should consequentially get a role of "tab". Because that's what they effectively are. The currently active tab then gets aria-selected="true", the other (explicitly) aria-selected="false".
  5. Keyboard interaction is as follows:
    • One tab stop to go to the currently focused tab. Next tabstop moves into the panel with the details (the updates selection or the permission controls).
    • Left and right arrow when on one of the buttons should switch focus to the other tab, but not yet load it.
    • Space should load the new tab, and set aria-selected accordingly. Focus should then remain there and let the user tab away.
  6. The container for the controls, if any, should be of role "tabpanel"
  7. Escape should be mapped to do what the Back button way further up in the main add-on manager UI does, and close that panel/dalog and return to the previous page, setting focus back to the add-on button that opened the panel.

Also, visual styling needs to be checked to not show a focus outline around the whole document, to make sure the buttons are showing a focus outline when tabbed to them, etc. Also, it is worth checking the contrast throughout the whole page to make sure it meets WCAG 2.1 criteria. You can use the Firefox Accessibility Dev Tools auditing feature to do this in the browser toolbox.

Not sure if this was set to P2 intentionally or not so resetting to make sure this goes through triage (put it back if I made a mistake!)

Priority: P2 → --

Note that the details panel which appears when you click the graphic or the add-on name is different to the menu which appears when you click the .more-options-button button. (I only discovered this by mistake.) That panel has a bunch of other issues; e.g. buttons behaving like tabs but not indicating selection, should probably be exposed like a dialog, etc. Marco, would you mind taking a look at this and providing a more thorough review?

Flags: needinfo?(mzehe)

I updated my above comment with added information on the details panel.

Flags: needinfo?(mzehe)

Thanks for the review on this. Needinfoing myself to file bugs based on this.

Flags: needinfo?(mstriemer)
Priority: -- → P1

This doesn't seem big enough for specific bugs, I'll just attach a few patches here.

Attaching a first pass here, still need to do the details page and testing, but I added the suggestions for the list view and the more options button/menu.

I go on PTO tomorrow, so this might be stagnant for a bit. I started a try build at https://treeherder.mozilla.org/#/jobs?repo=try&revision=c68330d7872f64aa9a97534de54164827fd25b2d.

Flags: needinfo?(mstriemer)

Gah, I'm really embarrassed to have missed some of this when reviewing part of the rewrite. :-(

(In reply to Mark Striemer [:mstriemer] from comment #9)

This doesn't seem big enough for specific bugs, I'll just attach a few patches here.

But the l10n bits aren't upliftable, I think, so it might make sense to do those separately from the remainder of the work, so we can at least fix some of the behavioural/markup issues on 69 even if some labels might still be missing.

Alternatively, the only new string I see in the patch at the moment is an aria-label for the "More Options" icon that opens the menu, which we might be able to achieve on beta by reusing the expand-addon-button string (which seems to have been changed on central).

What's the current state of the patches?

(In reply to Marco Zehe (:MarcoZ) from comment #2)

  1. The image preceding the add-on name currently has no alt text. It should contain the add-on name. This is the img tag with class "card-heading-icon addon-icon". Also, since it acts as a button to load the details panel for the add-on, it should probably be turned into a proper button and the name changed to something like "Details for <addon name>".

In terms of mouse interactions for sighted users, the entire card acts as a button, not just the image, and the add-on name is repeated straight after the image. So repeating the add-on name on the image doesn't seem super helpful; It's just an icon that can be different for each add-on - or the same, if none got supplied. So it's decorative, so I'd use alt="".

However, there's no way to activate the card behaviour with the keyboard, and having nested focusable items (ie making the card focusable and then also making the menu in the card focusable) is a headache... There's a "more options" (or "manage" on nightly) item in the popup menu that achieves the same thing, but that's not particularly convenient for keyboard users. If I had to pick, it would seem easiest to just make the add-on name focusable (instead of the description) and have enter/space activate the card behaviour there. Does that sound right?

Flags: needinfo?(mzehe)
Flags: needinfo?(mstriemer)

(In reply to :Gijs (he/him) from comment #10)

In terms of mouse interactions for sighted users, the entire card acts as a button, not just the image, and the add-on name is repeated straight after the image. So repeating the add-on name on the image doesn't seem super helpful; It's just an icon that can be different for each add-on - or the same, if none got supplied. So it's decorative, so I'd use alt="".

Agreed, it would certainly reduce the noise, especially when combined with point below.

However, there's no way to activate the card behaviour with the keyboard, and having nested focusable items (ie making the card focusable and then also making the menu in the card focusable) is a headache... There's a "more options" (or "manage" on nightly) item in the popup menu that achieves the same thing, but that's not particularly convenient for keyboard users. If I had to pick, it would seem easiest to just make the add-on name focusable (instead of the description) and have enter/space activate the card behaviour there. Does that sound right?

Yes, that sounds reasonable.

Flags: needinfo?(mzehe)
Assignee: nobody → mstriemer
Attachment #9082761 - Attachment description: Bug 1567600 - Part 1: about:addons list view a11y → Bug 1567600 - Part 1: Better accessibility for about:addons list view r?Gijs
Attachment #9082762 - Attachment description: Bug 1567600 - Part 2: a11y for panel-list → Bug 1567600 - Part 2: Better accessibility for about:addons panel-list r?Gijs

(In reply to Marco Zehe (:MarcoZ) from comment #2)

  1. Each button should then get a role of "menuitem"., the intermediary div should get a role of "presentation".

The menu items are panel-item elements with a button included in shadow DOM. I've added the menuitem role to the button itself, but I'm not sure how that will interact with a screen reader and shadow DOM.

The intermediary div: is that the <div class="list"> div or how the panel-item is showing up? The panel-list element itself has shadow DOM with 3 divs: an arrow on the top, the list, and an arrow on the bottom. I put the role on all three of those for now.

The add-on details panel also has a bunch of problems of its own.

  1. When it loads, this is not being indicated because focus isn't set to anything.

I've set the focus to the more options button in this case. Does that seem reasonable or should it jump straight to the tab buttons?

  1. The main addon-card element should get a role of "dialog". It should also get an aria-label pointing to the heading that has the add-on name.

This card isn't really a dialog, it switches to a details page. I can add the role but I'm not sure it semantically makes sense.

  1. Escape should be mapped to do what the Back button way further up in the main add-on manager UI does, and close that panel/dalog and return to the previous page, setting focus back to the add-on button that opened the panel.

The browser history works on the detail page, so backspace can be used to go back to the list view. As I mentioned before this isn't really a dialog, but if that would help with accessibility I can add the dialog/escape stuff.

Also, visual styling needs to be checked to not show a focus outline around the whole document, to make sure the buttons are showing a focus outline when tabbed to them, etc. Also, it is worth checking the contrast throughout the whole page to make sure it meets WCAG 2.1 criteria. You can use the Firefox Accessibility Dev Tools auditing feature to do this in the browser toolbox.

Visual focus looks fine to me. The browser can be focused, but it isn't always focused. Contrast has one failure on the blue link text, which matches about:preferences and the photon style guide (actually it's slightly off, but the hover colour is correct and I can't tell the difference--it also fails).

There's a try build running at https://treeherder.mozilla.org/#/jobs?repo=try&revision=54f357e9b4e4d40185ba979da309dc05c3f3939d, can you check that this is working as expected Marco?

Flags: needinfo?(mstriemer) → needinfo?(mzehe)

Also, in bug 1560155 the plan is to make the badges link to their SUMO page. There are currently up to two badges, one if an add-on is recommended and another when it is allowed to run in private windows. This would mean there are 2-4 tab stops per add-on. Should the list of add-ons be updated to support up/down to navigate through add-ons instead of tabbing through the links/inputs in the cards?

(In reply to Mark Striemer [:mstriemer] from comment #13)

The menu items are panel-item elements with a button included in shadow DOM. I've added the menuitem role to the button itself, but I'm not sure how that will interact with a screen reader and shadow DOM.

Works fine in the try build.

The intermediary div: is that the <div class="list"> div or how the panel-item is showing up? The panel-list element itself has shadow DOM with 3 divs: an arrow on the top, the list, and an arrow on the bottom. I put the role on all three of those for now.

Works great.

The add-on details panel also has a bunch of problems of its own.

  1. When it loads, this is not being indicated because focus isn't set to anything.

I've set the focus to the more options button in this case. Does that seem reasonable or should it jump straight to the tab buttons?

No, that works OK.

  1. The main addon-card element should get a role of "dialog". It should also get an aria-label pointing to the heading that has the add-on name.

This card isn't really a dialog, it switches to a details page. I can add the role but I'm not sure it semantically makes sense.

Good point, this is more like an SPA, then, but something that replaces the full page with something else. Leave it as is.

  1. Escape should be mapped to do what the Back button way further up in the main add-on manager UI does, and close that panel/dalog and return to the previous page, setting focus back to the add-on button that opened the panel.
    The browser history works on the detail page, so backspace can be used to go back to the list view. As I mentioned before this isn't really a dialog, but if that would help with accessibility I can add the dialog/escape stuff.

Hm, since this is not strictly a browsing context that is recognizable as such, some users might not get the idea that the Back navigation via Backspace or Alt+LeftArrow would work in this case. Jamie, what do you think?

(In reply to Mark Striemer [:mstriemer] from comment #14)

Also, in bug 1560155 the plan is to make the badges link to their SUMO page. There are currently up to two badges, one if an add-on is recommended and another when it is allowed to run in private windows. This would mean there are 2-4 tab stops per add-on. Should the list of add-ons be updated to support up/down to navigate through add-ons instead of tabbing through the links/inputs in the cards?

Yes, that could help for sighted keyboard users, screen reader users would probably not make use of this much, since they'l be in a browsing context with this new panel. So if you add those, that'll greatly help sighted, non-screen reader, keyboard users, but won't have an adverse effect on screen reader users AFAICS.

Flags: needinfo?(mzehe) → needinfo?(jteh)

(In reply to Marco Zehe (:MarcoZ) from comment #15)

The browser history works on the detail page, so backspace can be used to go back to the list view. As I mentioned before this isn't really a dialog, but if that would help with accessibility I can add the dialog/escape stuff.

Hm, since this is not strictly a browsing context that is recognizable as such, some users might not get the idea that the Back navigation via Backspace or Alt+LeftArrow would work in this case. Jamie, what do you think?

I don't think this is immediately obvious. I also note that there's a "Go Back" button (#go-back) that appears outside of the document which is probably what sighted users will press. This isn't obvious to screen reader users because it's outside of the document and they're not likely to shift+tab out of the document to find a back button. If it's not too much trouble, I think I'd like to see escape go back here. As Marco said, we probably don't need the dialog stuff though.

Flags: needinfo?(jteh)

Too late for Fx69 at this point with the RC coming next week.

Is there an appeal process for uplift denials? This is a significant breaking regression in a popular feature. Are we simply past the deadline or is this seen as not important enough to uplift or too risky to uplift. (If mouse users couldn't use the add-ons manager effectively, would we take an uplift to fix that?)

Asa, I really wanted to get this fix into Fx69, but at this point we're out of betas for it. Also, this isn't a new regression in 69 and it appears that at least one patch adds new strings when we're way past string freeze. If you feel super-strongly about taking this still, I would suggest going the exception request route. Feel free to ping me offline if you want to do that.

Flags: needinfo?(asa)
Pushed by mstriemer@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/538e93047081 Part 1: Better accessibility for about:addons list view r=fluent-reviewers,flod,Gijs https://hg.mozilla.org/integration/autoland/rev/fb9da1d831a3 Part 2: Better accessibility for about:addons panel-list r=Gijs https://hg.mozilla.org/integration/autoland/rev/1d8cca181d35 Part 3: Better accessibility for named-deck and about:addons detail view r=Gijs https://hg.mozilla.org/integration/autoland/rev/06aca0857b32 Part 4: Add aria-label to about:addons more options button r=Gijs,fluent-reviewers,flod

We'd like to get some extra QA attention on this in the 70 betas as we may be eventually be looking to include some version of these fixes in an Fx69 dot release.

Flags: qe-verify+

I have reproduced the issue on the affected Nightly build 70.0a1 (2019-07-19) on Windows 10 x64.
Verified - fixed on Firefox 70.0b3 and 71.0a1 (2019-09-06) on Windows 10 x64.
When accessing the Extensions section the user is able to press the menu button via keyboard, and the menu opens, focus is set to the first item.
Tab is also trapped, the user not being able to move anywhere outside that menu.
Up and down arrows move among menu items, wrap around at the top and bottom.
Escape closes the menu and returns focus to the More Options button that opened it.
When the area loads the focus is set on the on the more options button.
The buttons are read with the roles or features now and are not read as something generic.
If opened the reader will point it out and read the options that are selected.
Keyboard interaction works as intended: "one tab stop to go to the currently focused tab. Next tabstop moves into the panel with the details (the updates selection or the permission controls)". The left and right arrows switch focus to the other tab, without yet loading it.
Space sets the aria-selected accordingly, while focus remains on there.
Escape does what the Back button is supposed to do, closes the panels/dialog and returns to the previous page setting focus back to the add-on button that opened the panel.

Like I mentioned before, I tested this issue on Windows 10, please let me know if it's necessary to test it on Ubuntu or not.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

Beta/Release Uplift Approval Request

  • User impact if declined: Screen reader accessibility is lacking in about:addons.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky):
  • String changes made/needed:
Attachment #9093020 - Flags: approval-mozilla-release?
Attachment #9093018 - Flags: approval-mozilla-release?
Attachment #9093019 - Flags: approval-mozilla-release?

Patches rebased onto release attached.

Comment on attachment 9093018 [details] [diff] [review] about-addons-a11y-1.patch Addresses a regression in Addons Manager usability for screen reader users. Exception approved by Joe & Reese. Approved for 69.0.1.
Flags: needinfo?(asa)
Attachment #9093018 - Flags: approval-mozilla-release? → approval-mozilla-release+
Attachment #9093019 - Flags: approval-mozilla-release? → approval-mozilla-release+
Attachment #9093020 - Flags: approval-mozilla-release? → approval-mozilla-release+

Added to the 69.0.1 relnotes:

Usability improvements to the Add-ons Manager for users with screen readers

Verified - fixed on Firefox 69.0.1 on Windows 10 x64.

Hi Mark, should we uplift this to ESR68 also? If so, please nominate them for approval.

Flags: needinfo?(mstriemer)

It probably should be. I rebased the patches onto ESR, but there are test failures I'll need to look into still.

Flags: needinfo?(mstriemer)

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Improvements to accessibility of about:addons
  • User impact if declined: Poor screen reader accessibility support in about:addons
  • Fix Landed on Version: 69
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This has already been uplifted to release successfully, good test coverage.
  • String or UUID changes made by this patch:
Attachment #9100488 - Flags: approval-mozilla-esr68?
Attachment #9100486 - Flags: approval-mozilla-esr68?
Attachment #9100487 - Flags: approval-mozilla-esr68?
Comment on attachment 9100486 [details] [diff] [review] about-addons-a11y-esr68-1.patch Fixes some critical a11y regressions in about:addons. Approved for 68.2esr.
Attachment #9100486 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Attachment #9100487 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Attachment #9100488 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Verified - fixed on Firefox 68.2.0esr on Windows 10 x64.

QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Regressions: 1623723
Regressions: 1706486
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: