397097 - Account Manager, Preferences and Import Dialogs: Several labels are not correctly marked up for accessibility

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Marco Zehe (:MarcoZ)]

On several pages of the Account Manager, Preferences, and Import Wizard dialogs, there are several labels that are not roperly marked up for accessibility. For example, none of the radio groups have a properly assigned label. Screen Readers therefore cannot read the associated prompt with these radio buttons.

Comment 1

[Marco Zehe (:MarcoZ)]

Attachment: This patch addresses the labelling issues.

1. Associated labels with radio groups by setting the label's control attribute to the id of the radio group.
2. Converted some <description> elements which are clearly labels to <label> elements and associated them to controls they are labelling.
3. Converted some for attributes of some <label> elements to convert attributes for consistency.

Comment 2

[Aaron Leventhal]

Comment on attachment 281861 [details] [diff] [review]
This patch addresses the labelling issues.

This one looks to have an incorrect > in the value attribute (am-server-advanced.xul):
+      <label value=">&pop3Desc.label;" control="folderStorage"/>
Also, why does it remove the flex="1" attribute?

And all of the ones in AccountWizard.xul now have the control attribute twice.

The rest looks good, I think.

I'm not officially allowed to review this, so when you post a new patch, request a review from a Thunderbird peer. You can search for that on the web, or ask on #maildev.

Comment 3

[Marco Zehe (:MarcoZ)]

Attachment: Second patch, addressing Aaron's comments.

This addresses Aaron's comments:
1. in accountwizard.xul, the control attribute now only shows up once, but the for attribute has been removed to be consistent with other labels throughout Thunderbird.
2. Corrected typo, and added the flex attribute back into am-server-advanced.xul.

Comment 4

[Wayne Mery (:wsmwk)]

possible additional opportunities for you :)    (or dupes)
 bug 369159    	
 bug 376486
 bug 387479
 bug 313736    	
 bug 327908

Comment 5

[Marco Zehe (:MarcoZ)]

Attachment: Third patch, missed a few files last time.

This patch adds a few more files from the mailnews/compose and mailnews/extensions folders that all expose the same problem: Unlabelled radio groups for a11y.

Comment 6

[Phil Ringnalda (:philor)]

Archaeological digression: those apparently senseless for attributes are there because bug 107642 changed <text for="control"> to <label control="control"> in the half-dozen separate files like http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla%2Fmailnews%2Fbase%2Fprefs%2Fresources%2Fcontent%2FAttic%2Faw-accname.xul&rev=&cvsroot=%2Fcvsroot but then bug 101873 did a broken merge, putting the separate files into AccountWizard.xul with <label for="control">, and then bug 24873 fixed them by adding control="control" without removing the for attributes.

Comment 7

[Phil Ringnalda (:philor)]

Don't the account type and server type radiogroups in AccountWizard.xul also need to have <description> -> <label>? Other than that, looks good to me (assuming that there's just no good solution for constructs like am-copiesOverlay.xul, where the label for a checkbox is also the label for a radiogroup).

Comment 8

[Marco Zehe (:MarcoZ)]

Attachment: Fourth patch, adressing two radio groups in account wizard.

From a usability standpoint, the instances in am-copiesOverlay.xul are not such a problem: When you tab through the dialog, you first land on the checkbox, and then on the radiogroup, which gives you an in-context understanding of the meaning.

Comment 9

[Phil Ringnalda (:philor)]

Comment on attachment 281993 [details] [diff] [review]
Fourth patch, adressing two radio groups in account wizard.

r=philringnalda, and thanks!

Since Mnyromyr passed on reviewing for SeaMonkey, opting for beating us up later if it breaks, let's make Neil sr, so we can blame him :)

Comment 10

[[e-mail]]

Comment on attachment 281993 [details] [diff] [review]
Fourth patch, adressing two radio groups in account wizard.

sr=me if you change all the <description>&foo;</description> back to <label>&foo;</label> rather than the <label value="&foo"/> that you've used here.

The "When starting up:" menulist in offline preferences could do with an accesskey, presumably "W", although you don't need to do that in this bug.

Comment 11

[Marco Zehe (:MarcoZ)]

Attachment: Final patch for checkin

This patch addresses Neil's comments regarding the labels that were converted from XUL:description elements. The value attribute has been removed, and the label's caption been put inside the tags so they can wrap if need be.

Comment 12

[Aaron Leventhal]

Do we need approval for this patch?

Comment 13

[Scott MacGregor]

Aaron, the patch doesn't need approval since it doesn't effect Firefox, so it's good to go.

Comment 14

[Nick Kreeger]

Checked into TRUNK. Thanks Marco!