Closed Bug 414717 Opened 12 years ago Closed 12 years ago

New Extensions Manager UI has accessibility issues

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla1.9

People

(Reporter: MarcoZ, Assigned: surkov)

References

Details

(Keywords: access)

Attachments

(1 file, 1 obsolete file)

A.1 Open Tools/Add-Ons, and go to "Get Add-Ons" tab.
1. The Search Edit has no accessible name/label.
2. The button that is next in the tab order has no accessible name/label.

A.2 After performing a Search:
1. The list of search results is correctly exposed as a listbox, and the items are listbox items. However, the AccessibleName for any of the search results is "No Preview Learn More Extension Theme Installing… Install Failed Install Complete". It builds up the accessible name from some of the labels, but not the appropriate ones.
Flags: blocking-firefox3?
Summary: New Extensions Managger UI has accessibility issues → New Extensions Manager UI has accessibility issues
The list pane is also used for supplementary useful messages like "Retrieving blah blah" or an error message (something like "The list could not be retrieved").

These pieces of info are no doubt inaccessible.
(In reply to comment #0)
> A.1 Open Tools/Add-Ons, and go to "Get Add-Ons" tab.
> 1. The Search Edit has no accessible name/label.

It's interesting but they use @emptyText which itns' supported yet (bug 406095)

(In reply to comment #1)
> The list pane is also used for supplementary useful messages like "Retrieving
> blah blah" or an error message (something like "The list could not be
> retrieved").
> 
> These pieces of info are no doubt inaccessible.
> 

I haven't idea how they got an idea to put into richlistbox everything. Possibly strong schema/DTD usage will save from this? :)
(In reply to comment #1)
> These pieces of info are no doubt inaccessible.

Do they need to be? Would changing the tooltip achieve the desired effect, or does that not trigger a re-read? I'm not really sure that the nice feedback messages are really required, as they're just interstitials before the dialogs pop up.

Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P3
Gikving this a target milestone so it doesn't slip the radar. Part of it is now fixed due to the fix for bug 406095, but the results list is still a problem and makes the dialog virutually unusable.
Target Milestone: --- → Firefox 3
Flags: in-testsuite+
Flags: in-litmus+
Flags: blocking1.8.1.14?
Flags: blocking1.8.1.13?
Flags: blocking-firefox3?
Flags: blocking-firefox3+
Flags: in-testsuite+
Flags: in-litmus+
Flags: blocking1.8.1.14?
Flags: blocking1.8.1.13?
Flags: blocking-firefox3? → blocking-firefox3+
All we need is to throw out vboxes representing labels (like "recommended" and "see all recommended addons") from richlistbox? Marco, do I get this correctly?
(In reply to comment #5)
> All we need is to throw out vboxes representing labels (like "recommended" and
> "see all recommended addons") from richlistbox? Marco, do I get this correctly?

The problem is that none of the relevant info from the RichListbox item is currently being shown. If you look at the items with DOM Inspector, you'll see what parts get pulled into each Richlistbox item.
(In reply to comment #6)
> (In reply to comment #5)
> > All we need is to throw out vboxes representing labels (like "recommended" and
> > "see all recommended addons") from richlistbox? Marco, do I get this correctly?
> 
> The problem is that none of the relevant info from the RichListbox item is
> currently being shown. If you look at the items with DOM Inspector, you'll see
> what parts get pulled into each Richlistbox item.
> 

Do you mean richlistitem doesn't expose correct accessible name?

Also does it mean that labels inside richlistbox are accessible and don't broke accessibility of richlistbox?
(In reply to comment #7)
> Do you mean richlistitem doesn't expose correct accessible name?

Correct. The AccessibleName of the RichListItem is what screen readers read, and that one currently is total bogus.

> Also does it mean that labels inside richlistbox are accessible and don't broke
> accessibility of richlistbox?

Correct.
Attached patch simple fix (obsolete) — Splinter Review
Marco, could you run through the tabs of addons dialog and test because:
1) there is a lot of similar widgets I don't how they are used but their accessible name may be incorrect
2) there is some strange names that includes all information and I'm not sure it's wanted.
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #307483 - Flags: review?(marco.zehe)
Comment on attachment 307483 [details] [diff] [review]
simple fix

This gives me the name, which is good, but there is also a description which is not accessible. I twould be good if that one would also be added to the label. Other lists in the dialog are not affected by this change, so no regressions.
Attachment #307483 - Flags: review?(marco.zehe) → review-
(In reply to comment #10)
> (From update of attachment 307483 [details] [diff] [review])
> This gives me the name, which is good, but there is also a description which is
> not accessible. I twould be good if that one would also be added to the label.
> Other lists in the dialog are not affected by this change, so no regressions.
> 

their names should look similar with names of items from installed extensions list for example?
Just wonder, is it comfortable to read/listen long names? I mean should we expose the rest information by another way, for example, via description or allow to catch it from navigation down of the tree?
(In reply to comment #12)
> Just wonder, is it comfortable to read/listen long names? I mean should we
> expose the rest information by another way, for example, via description or
> allow to catch it from navigation down of the tree?

The lists on the other pages already give the description following the name. I believe that's fine, as long as we put the name first. And the other lists already worked, it is only the search results list on the "Get Add-Ons" page that does not work.
Attached patch patchSplinter Review
does it work for you?
Attachment #307483 - Attachment is obsolete: true
Attachment #307646 - Flags: review?(marco.zehe)
Comment on attachment 307646 [details] [diff] [review]
patch

> +          return this.getAttribute("name") + this.getAttribute("version") +
> +            this.getAttribute("description");

Please put spaces between the three items when you add them up, otherwise they'll be run together. With that fixed upon checkin, r=me. You may need SR from somebody on this one...
Attachment #307646 - Flags: review?(marco.zehe) → review+
Attachment #307646 - Flags: superreview?(neil)
Attachment #307646 - Flags: review?(mano)
Comment on attachment 307646 [details] [diff] [review]
patch

I don't think this needs sr but if Mano says it does then sr=me
Attachment #307646 - Flags: superreview?(neil)
Who can review this? I mano available or should we ask someone else?
Attachment #307646 - Flags: approval1.9?
Alex, I don't believe you need approval to land the patch right now since it is blocking Firefox3+.  Would you mind fixing the nit and checking in the patch so we can move forward? Thanks!
Priority: P3 → P2
Comment on attachment 307646 [details] [diff] [review]
patch

I forgot this bug is blocking. Thanks you Marco for catching.
Attachment #307646 - Flags: approval1.9?
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.xml,v  <--  extensions.xml
new revision: 1.62; previous revision: 1.61
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008032904 Minefield/3.0pre.
Status: RESOLVED → VERIFIED
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.