432026 - Various accessibility fixes in mail/base/content XUL files

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Marco Zehe (:MarcoZ)]

A pass at all XUL files in mail/base/content needing accessibility fixes. This contains:
- Missing label/control associations.
- Addition of aria-labelledby associations where they make sense (e. g. in labels that are made up of several controls).

No l10n changes were necessary for these changes.

Comment 1

[Marco Zehe (:MarcoZ)]

Attachment: Patch

Described fixes in XUL.

Comment 2

[Phil Ringnalda (:philor)]

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

>Index: mail/base/content/FilterListDialog.xul
>-        <description control="filterTree">&filterHeader.label;</description>
>+        <description control="filterTree" id="filterHeader.label">&filterHeader.label;</description>

>-                datasources="rdf:msgfilters rdf:msgaccountmanager" flags="dont-build-content">
>+                datasources="rdf:msgfilters rdf:msgaccountmanager" flags="dont-build-content"
>+                aria-labelledby="filterHeader.label">

Does <description control="id"> work, or not? Or are there some things that work with that, and others that work with aria-labelledby? If we don't have to have both, I'd rather not (the less we can break, the less we will break), and if we're going to keep the aria-labelledby, I'd rather have it inserted before the last line, so it's just one add, not an add and a change.

>+  <description id="defaultClient-desc">&defaultClient.intro;</description>

>+  <description class="desc" id="MsgSelectDesc">&MsgSelectDesc.label;</description>

Some sort of standard way of picking an id, rather than having filterHeader.label to filterHeader.label, defaultClient.intro to defaultClient-desc, and MsgSelectDesc.label to MsgSelectDesc, would be nice. I'm not sure what it should be, other than having a vague feeling that using exactly the name of the string isn't a good idea. If people do a good job of naming their strings, dropping the ".label" from the name ought to produce a good id value, but I suppose you can't count on good string names.

Comment 3

[Marco Zehe (:MarcoZ)]

(In reply to comment #2)
> (From update of attachment 319187 [details] [diff] [review])
> >Index: mail/base/content/FilterListDialog.xul
> >-        <description control="filterTree">&filterHeader.label;</description>
> >+        <description control="filterTree" id="filterHeader.label">&filterHeader.label;</description>
> 
> >-                datasources="rdf:msgfilters rdf:msgaccountmanager" flags="dont-build-content">
> >+                datasources="rdf:msgfilters rdf:msgaccountmanager" flags="dont-build-content"
> >+                aria-labelledby="filterHeader.label">
> 
> Does <description control="id"> work, or not? Or are there some things that
> work with that, and others that work with aria-labelledby? If we don't have to
> have both, I'd rather not (the less we can break, the less we will break), and
> if we're going to keep the aria-labelledby, I'd rather have it inserted before
> the last line, so it's just one add, not an add and a change.

If we convert the xul:description into a xul:label, we can get rid of aria-labelledby alltogether. If the text remains within the opening and closing label tags, it will wrap just like description does.

I agree that the IDs should be consistent if possible.

Comment 4

[Phil Ringnalda (:philor)]

But do we actually have to convert the description to a label?

http://mxr.mozilla.org/mozilla/source/accessible/src/base/nsAccessible.cpp#307 certainly seems to look for <description control="id"> (though I didn't trace back to where it is or isn't called), and Fx3 is shipping at least 10 descriptions with a control attribute, between browser and toolkit. Will they just not work for some or all consumers, or is that use deprecated and likely to stop working at some point, or is it just not stylish?

Comment 5

[Marco Zehe (:MarcoZ)]

(In reply to comment #4)
> But do we actually have to convert the description to a label?
> 
> http://mxr.mozilla.org/mozilla/source/accessible/src/base/nsAccessible.cpp#307
> certainly seems to look for <description control="id"> (though I didn't trace
> back to where it is or isn't called), and Fx3 is shipping at least 10
> descriptions with a control attribute, between browser and toolkit. Will they
> just not work for some or all consumers, or is that use deprecated and likely
> to stop working at some point, or is it just not stylish?

No, there is a difference between accessibleDescription and accessibleName. The goal in my patches is to give the controls names by adding either control to a label or aria-labelledby to the control itself. An accessibleDescription is *additional* information that can be asked for via a hotkey, but is usually not spoken automatically. The instances in Firefox that you mention are exactly htat: Additional info.

So for those that obviously label a control, it would be either converting it from xul:description to xul:label, which would make it the accessibleName instead of accessibleDescription, or use aria-labelledby and keep the xul:description.

Comment 6

[Phil Ringnalda (:philor)]

Ah, I finally get it: you're in the textbox to change the master password, you're told it's "Enter new password:", you think "wtf?" and get the longer description of "A Master Password is used to protect sensitive information...". Explain it to me enough times, eventually it'll sink in :)

Comment 7

[Marco Zehe (:MarcoZ)]

Attachment: Updated patch

Converts those xul:description elements that are actually labels into such and thus gets rid of several aria-labelledby constructs. Also, gets rid of inconsistencies with IDs for those xul:description elements.

Comment 8

[Phil Ringnalda (:philor)]

Comment on attachment 320659 [details] [diff] [review]
Updated patch

Now that I understand the difference, I almost think filterHeader.label is more of a description than a label, but I think more strongly that we should trust your instincts over mine, so r=philringnalda, and thanks for sticking with me.

Comment 9

[Marco Zehe (:MarcoZ)]

Checking in mail/base/content/FilterListDialog.xul;
/cvsroot/mozilla/mail/base/content/FilterListDialog.xul,v  <--  FilterListDialog.xul
new revision: 1.8; previous revision: 1.7
done
Checking in mail/base/content/customizeToolbar.xul;
/cvsroot/mozilla/mail/base/content/customizeToolbar.xul,v  <--  customizeToolbar.xul
new revision: 1.15; previous revision: 1.14
done
Checking in mail/base/content/customizeToolbarSheet.xul;
/cvsroot/mozilla/mail/base/content/customizeToolbarSheet.xul,v  <--  customizeToolbarSheet.xul
new revision: 1.3; previous revision: 1.2
done
Checking in mail/base/content/defaultClientDialog.xul;
/cvsroot/mozilla/mail/base/content/defaultClientDialog.xul,v  <--  defaultClientDialog.xul
new revision: 1.2; previous revision: 1.1
done
Checking in mail/base/content/msgHdrViewOverlay.xul;
/cvsroot/mozilla/mail/base/content/msgHdrViewOverlay.xul,v  <--  msgHdrViewOverlay.xul
new revision: 1.29; previous revision: 1.28
done
Checking in mail/base/content/msgSelectOffline.xul;
/cvsroot/mozilla/mail/base/content/msgSelectOffline.xul,v  <--  msgSelectOffline.xul
new revision: 1.9; previous revision: 1.8
done

Comment 10

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 320659 [details] [diff] [review]
Updated patch

Requesting a1.8.1, since this patch also an some ids to defaultClientDialog.xul that is useful for calendar integration, allowing addition of a checkbox for what default clients to use.

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 320659 [details] [diff] [review]
Updated patch

This seems to be low-risk from reading it; approving.

Comment 12

[Philipp Kewisch [:Fallen][📱📆]]

Checking in FilterListDialog.xul;
/cvsroot/mozilla/mail/base/content/FilterListDialog.xul,v  <--  FilterListDialog.xul
new revision: 1.3.12.2; previous revision: 1.3.12.1
done
Checking in customizeToolbar.xul;
/cvsroot/mozilla/mail/base/content/customizeToolbar.xul,v  <--  customizeToolbar.xul
new revision: 1.9.4.1; previous revision: 1.9
done
Checking in defaultClientDialog.xul;
/cvsroot/mozilla/mail/base/content/defaultClientDialog.xul,v  <--  defaultClientDialog.xul
new revision: 1.1.2.2; previous revision: 1.1.2.1
done
Checking in msgHdrViewOverlay.xul;
/cvsroot/mozilla/mail/base/content/msgHdrViewOverlay.xul,v  <--  msgHdrViewOverlay.xul
new revision: 1.14.2.9; previous revision: 1.14.2.8
done
Checking in msgSelectOffline.xul;
/cvsroot/mozilla/mail/base/content/msgSelectOffline.xul,v  <--  msgSelectOffline.xul
new revision: 1.6.2.1; previous revision: 1.6
done


-> fixed1.8.1.15