414717 - New Extensions Manager UI has accessibility issues

Status: VERIFIED FIXED

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

Description

[Marco Zehe (:MarcoZ)]

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.

Comment 1

[Aaron Leventhal]

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.

Comment 2

[alexander :surkov (:asurkov)]

(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? :)

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

(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.

Comment 4

[Marco Zehe (:MarcoZ)]

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.

Comment 5

[alexander :surkov (:asurkov)]

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?

Comment 6

[Marco Zehe (:MarcoZ)]

(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.

Comment 7

[alexander :surkov (:asurkov)]

(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?

Comment 8

[Marco Zehe (:MarcoZ)]

(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.

Comment 9

[alexander :surkov (:asurkov)]

Attachment: simple fix

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.

Comment 10

[Marco Zehe (:MarcoZ)]

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.

Comment 11

[alexander :surkov (:asurkov)]

(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?

Comment 12

[alexander :surkov (:asurkov)]

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?

Comment 13

[Marco Zehe (:MarcoZ)]

(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.

Comment 14

[alexander :surkov (:asurkov)]

Attachment: patch

does it work for you?

Comment 15

[Marco Zehe (:MarcoZ)]

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...

Comment 16

[[e-mail]]

Comment on attachment 307646 [details] [diff] [review]
patch

I don't think this needs sr but if Mano says it does then sr=me

Comment 17

[Marco Zehe (:MarcoZ)]

Who can review this? I mano available or should we ask someone else?

Comment 18

[Asaf Romano (gone)]

Comment on attachment 307646 [details] [diff] [review]
patch

r=mano

Comment 19

[Marco Zehe (:MarcoZ)]

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!

Comment 20

[alexander :surkov (:asurkov)]

Comment on attachment 307646 [details] [diff] [review]
patch

I forgot this bug is blocking. Thanks you Marco for catching.

Comment 21

[alexander :surkov (:asurkov)]

/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.xml,v  <--  extensions.xml
new revision: 1.62; previous revision: 1.61

Comment 22

[Marco Zehe (:MarcoZ)]

Verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008032904 Minefield/3.0pre.

Comment 23

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381