Last Comment Bug 649929 - Improve screen reader readability of the Help/About window
: Improve screen reader readability of the Help/About window
Status: VERIFIED FIXED
:
Product: Firefox
Classification: Client Software
Component: Disability Access (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 6
Assigned To: Marco Zehe (:MarcoZ)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-14 00:24 PDT by Marco Zehe (:MarcoZ)
Modified: 2011-05-03 08:56 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (5.56 KB, patch)
2011-04-14 02:44 PDT, Marco Zehe (:MarcoZ)
no flags Details | Diff | Splinter Review
Updated patch (4.70 KB, patch)
2011-05-02 03:55 PDT, Marco Zehe (:MarcoZ)
gavin.sharp: review+
Details | Diff | Splinter Review

Description Marco Zehe (:MarcoZ) 2011-04-14 00:24:54 PDT
Screen readers that use fuzzy logic to try and determine the text associated with each deck panel of this window currently grab stuff from various sources which is not always accurate. This is to improve the semantic markup used in the XUL of this window to tell screen readers what we actually want read.

As a side, it will also fix the label/menulist association of the update channel choser, this is currently missing the control attribute.
Comment 1 Marco Zehe (:MarcoZ) 2011-04-14 02:41:57 PDT
Try-server builds will appear here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/mzehe@mozilla.com-3a553fc7d935
Comment 2 Marco Zehe (:MarcoZ) 2011-04-14 02:44:27 PDT
Created attachment 525966 [details] [diff] [review]
Patch

1. Add WAI-ARIA magic to the various panels to make them read the text in proper order. Some don't need the magic, since there is only one text child, which is OK.
2. Also, properly label the combobox for the update channels, and make sure to assign the description for each channel, so it can be read by screen reader users when asking for the focused item, without them having to navigate around and look for the channel description themselves.
Comment 3 Marco Zehe (:MarcoZ) 2011-04-14 09:26:05 PDT
Comment on attachment 525966 [details] [diff] [review]
Patch

try-server run completed with only two random oranges in reftests. The one thing I couldn't test is the association of the description to the combobox of the channel selector. Gavin, is this approach correct in principle?
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-04-14 10:20:25 PDT
Comment on attachment 525966 [details] [diff] [review]
Patch

I think you should be able to just have the one line from setAccChannelDescription added directly to selectChannel. setChannelMenuitem triggers a select event which in turn calls selectChannel, so you shouldn't need to call it explicitly in gChannelSelector.init().

Should the code in gChannelSelector.init() prepend "currentChannelText" to the detailsBox's aria-describedby when it unhides that description? (I'm not sure I understand why that aria-describedby or the one on the dialog are necessary - shouldn't things be "described by" their visible labels/descriptions automatically?)
Comment 5 Marco Zehe (:MarcoZ) 2011-04-14 10:26:19 PDT
(In reply to comment #4)
> I think you should be able to just have the one line from
> setAccChannelDescription added directly to selectChannel. setChannelMenuitem
> triggers a select event which in turn calls selectChannel, so you shouldn't
> need to call it explicitly in gChannelSelector.init().

Thanks for the clarification, fixed locally!

> Should the code in gChannelSelector.init() prepend "currentChannelText" to the
> detailsBox's aria-describedby when it unhides that description? (I'm not sure I
> understand why that aria-describedby or the one on the dialog are necessary -
> shouldn't things be "described by" their visible labels/descriptions
> automatically?)

The label/accessibleName is the initial label that appends/prepends the actual select. That's the stuff that gets spoken automatically by screen readers. The description below that, the one that talks about the cutting edge stuff or the "stable code", is additional info that gets put into the accessibleDescription of the combobox object. NVDA reads this automatically on focus, others read it when asked for the info. It's info you might want to hear, but do not necessarily need to hear each time the element gets focus. So accName is what's spoken always, accDescription is additional info that is optionally spoken *after* the name and current value.

So no, the texts as they are now will be fine, they will all be spoken once, and no redundandy is necessary.

The same goes for the tab panels: Screen readers have a fuzzy logic that tries to determine which of the text children *might* be the description for the panel. By setting aria-describedby explicitly, we can prevent them from applying the fuzzy logic, and explicitly control what they'll speak to the user when focus enters the tab panel.
Comment 6 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-04-20 18:15:58 PDT
Comment on attachment 525966 [details] [diff] [review]
Patch

Thanks for the explanation! Looks good to me with that change, but I'd like to see the final patch before r+ing.
Comment 7 Marco Zehe (:MarcoZ) 2011-05-02 03:55:46 PDT
Created attachment 529444 [details] [diff] [review]
Updated patch
Comment 8 Marco Zehe (:MarcoZ) 2011-05-02 09:06:45 PDT
Landed on nightly: http://hg.mozilla.org/mozilla-central/rev/f6ae3b6615f2
Comment 9 James Teh [:Jamie] 2011-05-03 08:56:28 PDT
Thanks! This is much nicer. :)

Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110503 Firefox/6.0a1

Note You need to log in before you can comment on or make changes to this bug.