Bhavya Shah Reporter
	Description • 9 years ago

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20150108202552

Steps to reproduce:

 1 Download, install and open NVDA screen reader from www.nvaccess.org
 2 Open the Add-ons Manager in Mozilla Firefox by:
 Going to Tools submenu > Add-ons
 OR
 Pressing Ctrl + Shift + A
 3 As an example, choose Plugins in the Add-on Type list box, and select a
 plugin and then Tab to and hit Enter on the More button.



Actual results:

NVDA says ‘Unknown’ for a control in the Add-ons Manager
 now, and there doesn’t seem to be any details about the add-on provided.


Expected results:

Mozilla Firefox will present some information about the
 add-on, that NVDA will read.
 Actual Result : NVDA says ‘Unknown’ for a control in the Add-ons Manager

	James Teh [:Jamie]
	Comment 1 • 9 years ago

This is more a UI coding issue and could be fixed with some ARIA. The element in question has an id of "detail-view". I'd probably:
1. Set role="group".
2. Set aria-labelledby to include the name and version of the add-on.
3. Set aria-describedby to point at the other text that can't be reached with the tab key; i.e. the description and last update info.

NVDA issue ticket: http://community.nvda-project.org/ticket/4809

	Marco Zehe (:MarcoZ) Assignee
	Comment 2 • 9 years ago

Gijs, know of anybody good for tackling this UI bug in Add-Ons Manager? See Jamie's comment #1 for a suggested fix.

	:Gijs (he/him)
	Comment 3 • 9 years ago

(In reply to Marco Zehe (:MarcoZ) from comment #2)
> Gijs, know of anybody good for tackling this UI bug in Add-Ons Manager? See
> Jamie's comment #1 for a suggested fix.

I will suggest adding it to our prioritized backlog... if it's not picked up in the next 2 weeks, please ping me again. At the moment I don't have the bandwidth to work on it myself, sorry.

	:Gijs (he/him)
	Comment 5 • 9 years ago

Comment on attachment 8551293 [details] [diff] [review]
Patch

Review of attachment 8551293 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Marco Zehe (:MarcoZ) from comment #4)
> Created attachment 8551293 [details] [diff] [review]
> Patch

I'm assuming you tested this yields the right result in NVDA? I'm a little confused because you're labelling a groupbox, which is a container... but sure, in principle this looks correct.

The only issue I have is that there are some notifications/warnings in the XUL markup that you're not including, like the "(disabled)" and "Update" bits (under #detail-name-container), but also the stuff in #detail-notifications - which maybe should get role="alert" instead? Not sure if that would be enough to convince NVDA to read the contents if not hidden. (Ditto, not sure what happens if you add invisible items to the labelled/describedby bits (for the (disabled) and Update bits, they should obviously only be listed if visible...)

Marco, can you look at those notifications as well?

	James Teh [:Jamie]
	Comment 6 • 9 years ago

(In reply to :Gijs Kruitbosch from comment #5)
> I'm a little
> confused because you're labelling a groupbox, which is a container...
It's quite common for containers/groupings to be labelled; e.g. in Settings dialogs.

	:Gijs (he/him)
	Comment 7 • 9 years ago

(In reply to James Teh [:Jamie] from comment #6)
> (In reply to :Gijs Kruitbosch from comment #5)
> > I'm a little
> > confused because you're labelling a groupbox, which is a container...
> It's quite common for containers/groupings to be labelled; e.g. in Settings
> dialogs.

Yes, I know! I guess my confusion stems from the fact that if anything, the "more details" page for an add-on / plugin, to me as a sighted user, looks more like a "document" than an "application", to put it in "role" attribute language. :-)

I guess there is real UI there (namely, the "ask to activate" dropdown at the bottom), but everything else looks like plain flat text to me that doesn't fit the "labeled controls" idiom.

In any case, to be 100% clear, I have no objections to doing whatever is necessary to make this as easily accessible as possible for people who aren't "guckis" (and I suspect making it work like a document with a single thing that happens to be a control might be harder than making it like an app that happens to have a wall of text we somehow need to convey) - I was just a little surprised.

	Marco Zehe (:MarcoZ) Assignee
	Comment 8 • 9 years ago

(In reply to :Gijs Kruitbosch from comment #7)
> Yes, I know! I guess my confusion stems from the fact that if anything, the
> "more details" page for an add-on / plugin, to me as a sighted user, looks
> more like a "document" than an "application", to put it in "role" attribute
> language. :-)

Interesting...Because all we see in the markup are label, description elements and label elements that are made to be links, and all in XUL. So semantically, these are UI elements, not document elements. If these are made up to look like a document, why wasn't HTML embedded in the first place instead? Might have given us some things for free that may just have been styled with some effort now.

Anyway, I'll also experiment with the notion that this is a document and give the xul:scrollbox a role of "document" instead and see what happens. Not sure if we've done this before, or what the accessibility will be like.

	Marco Zehe (:MarcoZ) Assignee
	Comment 9 • 9 years ago

It turns out that turning the scroll box into a document is all that is needed to make the whole thing accessible. Also, the fact that some things may be hidden or showing is thus taken into account, and one doesn't need to choose what to label a group with. This way, when invoking the More Information button, NVDA immediately goes into virtual mode, and one can browse all of the add-on information, links, and other info, at one's own leisure.

	:Gijs (he/him)
	Comment 10 • 9 years ago

(In reply to Marco Zehe (:MarcoZ) from comment #8)
> If these are
> made up to look like a document, why wasn't HTML embedded in the first place
> instead? Might have given us some things for free that may just have been
> styled with some effort now.

I mean, the background is still grey and I guess from that perspective, stylistically, it looks like a dialog - but it's a "dialog" that mostly just presents a lot of text to the user.

Getting the layout right in HTML would have been difficult, and I expect that is why we didn't embed HTML here; mixing XUL and HTML is usually a bit tricky when it gets beyond just a single <span> shoved into some text. :-(

> Anyway, I'll also experiment with the notion that this is a document and
> give the xul:scrollbox a role of "document" instead and see what happens.
> Not sure if we've done this before, or what the accessibility will be like.

Glad this worked!

	:Gijs (he/him)
	Comment 11 • 9 years ago

Comment on attachment 8551689 [details] [diff] [review]
Patch 2, sometimes it's simpler than one thinks. :)

Review of attachment 8551689 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

	Marco Zehe (:MarcoZ) Assignee
	Comment 12 • 9 years ago

Thanks Gijs! Could you check this into fx-team for me since I am currently in the middle of bisecting something else? Thanks!

	James Teh [:Jamie]
	Comment 13 • 9 years ago

It's worth noting that this might confuse some users, as they're interacting with this like a diaog and then suddenly get thrown into a document containing a restricted view. That does occur elsewhere, but normally, you can press escape or similar to get out of it. In this case, you have to tab out of the document, which I'm not sure is obvious.

	Marco Zehe (:MarcoZ) Assignee
	Comment 14 • 9 years ago

We could file a follow-up bug to make Escape close the detail view and return focus to the current list item in the Extensions richlistbox. But since shift-tabbing also works from the top of the document, and they'd have to do that anyway, I guess it's not such a big deal.

	:Gijs (he/him)
	Comment 15 • 9 years ago

(In reply to Marco Zehe (:MarcoZ) from comment #14)
> We could file a follow-up bug to make Escape close the detail view and
> return focus to the current list item in the Extensions richlistbox. But
> since shift-tabbing also works from the top of the document, and they'd have
> to do that anyway, I guess it's not such a big deal.

Right. FWIW, for sighted users, there's no real obvious way to 'get out' either - I think the idea is even that you use the browser's back button (or re-click an item in the category list on the left, I suppose)

	:Gijs (he/him)
	Comment 16 • 9 years ago

remote:   https://hg.mozilla.org/integration/fx-team/rev/424565376b6c

	Marco Zehe (:MarcoZ) Assignee
	Comment 17 • 9 years ago

Filed follow-up bug 1123635 for escaping out of the Details view.

	Ryan VanderMeulen [:RyanVM]
	Comment 18 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/424565376b6c

	[SV Manager] Florin Mezei, QA (:FlorinMezei)
	Comment 19 • 9 years ago

Reproduced the original issue in Firefox 35.0.1 on Windows 7 x64, when the More button in the Add-ons Manager is pressed (click, <enter>, <space>) for any category (Extensions, Plugins, Search, Experiments...). The issue no longer reproduces in the latest Firefox 38 Nightly (BuildID=20150203062848).