397664 - Don't process xhtml2:role

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

According to http://simon.html5.org/specs/aria-proposal we should not process xhtml2:role attribute. We should allow xhtml:role on SVG or XUL (non-HTML).

This means we need to change all of the XUL files which use xhtml2:role, to just use xhtml:role

Comment 1

[Aaron Leventhal]

I think we should wait for the final community word on the spec before we do this. That part might be controversial.

Comment 2

[Aaron Leventhal]

Attachment: 1) Remove xhtml2:role, 2) Change XUL Files to use xhtml:role, 3) Remove role when it was just used to set an aaa property that is now universal & requires no role, 4) Marco's s/groupbox/group fix

Comment 3

[Asaf Romano (gone)]

Comment on attachment 282751 [details] [diff] [review]
1) Remove xhtml2:role, 2) Change XUL Files to use xhtml:role, 3) Remove role when it was just used to set an aaa property that is now universal & requires no role, 4) Marco's s/groupbox/group fix

r=mano

Comment 4

[Aaron Leventhal]

Comment on attachment 282751 [details] [diff] [review]
1) Remove xhtml2:role, 2) Change XUL Files to use xhtml:role, 3) Remove role when it was just used to set an aaa property that is now universal & requires no role, 4) Marco's s/groupbox/group fix

Surkov, for nsAccessNode.cpp change

Neil, for everything else

Comment 5

[Aaron Leventhal]

Comment on attachment 282751 [details] [diff] [review]
1) Remove xhtml2:role, 2) Change XUL Files to use xhtml:role, 3) Remove role when it was just used to set an aaa property that is now universal & requires no role, 4) Marco's s/groupbox/group fix

Phil, for mail

Comment 6

[[e-mail]]

Comment on attachment 282751 [details] [diff] [review]
1) Remove xhtml2:role, 2) Change XUL Files to use xhtml:role, 3) Remove role when it was just used to set an aaa property that is now universal & requires no role, 4) Marco's s/groupbox/group fix

I'd prefer if you used xhtml (or even html as used in datetimepicker.xml for some reason) instead of xh which is virtually meaningless.

Comment 7

[Phil Ringnalda (:philor)]

Comment on attachment 282751 [details] [diff] [review]
1) Remove xhtml2:role, 2) Change XUL Files to use xhtml:role, 3) Remove role when it was just used to set an aaa property that is now universal & requires no role, 4) Marco's s/groupbox/group fix

>Index: accessible/src/base/nsAccessNode.cpp
>   // XXX We still support XHTML2 for now because of XUL content, but will migrate to XHTML after FF3
>-  else if (!aContent->GetAttr(kNameSpaceID_XHTML, nsAccessibilityAtoms::role, aRole) &&
>-           !aContent->GetAttr(kNameSpaceID_XHTML2_Unofficial, nsAccessibilityAtoms::role, aRole)) {
>+  else if (!aContent->GetAttr(kNameSpaceID_XHTML, nsAccessibilityAtoms::role, aRole)) {
>     return PR_FALSE;

Unless I'm reviewing for Moz2, which seems unlikely, that comment needs to be fixed.

>Index: mail/components/preferences/advanced.xul
>             <checkbox id="enableAppUpdate"
>                       label="&enableAppUpdate.label;"
>                       accesskey="&enableAppUpdate.accesskey;"
>                       preference="app.update.enabled"
>-                      xhtml2:role="wairole:checkbox" aaa:describedby="updateInfo"/>
>+                      aaa:describedby="updateInfo"/>
>             <checkbox id="enableAddonUpdate"
>                       label="&enableAddonsUpdate.label;"
>                       accesskey="&enableAddonsUpdate.accesskey;"
>                       preference="extensions.update.enabled"
>-                      xhtml2:role="wairole:checkbox" aaa:describedby="updateInfo"/>
>+                      aaa:describedby="updateInfo"/>

Those two lines came from bug 306897, which was porting Firefox's bug 302359, but you didn't actually land those changes for Fx. Does it need that aaa:describedby, or does Tb not need it? (Or, is there some subtle difference I'm not seeing?)

Comment 8

[Aaron Leventhal]

Attachment: Fix Neil & Phil's comments

Phil, nice catch. In fact that describedby usage was wrong. They don't even point to an element -- that ID doesn't exist in the file.

It should be like what Firefox has now, which is to have the vbox look like a groupbox via xhtml:role="group" and a the label for the checkbox group needs to point to that.

Comment 9

[alexander :surkov (:asurkov)]

Comment on attachment 282872 [details] [diff] [review]
Fix Neil & Phil's comments

you should remove xhtml2 namespace usage from attribute changed handler, with that r=me

Comment 10

[Phil Ringnalda (:philor)]

Comment on attachment 282872 [details] [diff] [review]
Fix Neil & Phil's comments

r=philringnalda on the mail bits, thanks for fixing us up!