The XHTML role attribute module was voted to go to last call last week but has not been moved to the TR page yet yet. http://www.w3.org/MarkUp/Group/2006/WD-xhtml-role-20060608/ This means that ultimately we'll need to recognize the role attribute in 3 namespaces: 1. The official XHTML2 namespace (see bug 315711): http://www.w3.org/2006/xhtml2/ (this might change) 2. The deprecated unofficial XTHML2 namespace (see bug 315711): "http://www.w3.org/TR/xhtml2" 3. The XHTML 1.x namespace: "http://www.w3.org/1999/xhtml" Since it seems like #1 might change our best bet is to support #3 now, and get authors to move to that while supporting #2 for backwards compatibility.

The universal properties will fix bug 337128

Comment on attachment 227434 [details] [diff] [review] Multiple fixes: 1) Recognize xhtml 1.x namespace via Has/GetRoleAttribute(), 2) Implement universal attributes that require no role, 3) Implement id lists for labelledby/describedby, 4) Fix comments We should update the DHTML docs to mention the precedence order of role attributes, in the edge case where a single element has multiple role attributes defined in different namespaces. (The precedence order is the default namespace, then XHTML 1, then XHTML 2.) But we can handle that separately. r=me

Aha! Aaron has already opened a bug to track the documentation changes, bug 343123.

Roles only apply to XHTML docs, right?

Neil, the roles and properties apply to anything renderable, including XUL and SVG.

... but not "plain" HTML, right?

Yes, it works in plain HTML when you do setAttributeNS(), but in that case it needs to be namespaced to XHTML or XHTML2 (unfortunately).

(In reply to comment #7) >Yes, it works in plain HTML when you do setAttributeNS(), but in that case it >needs to be namespaced to XHTML or XHTML2 (unfortunately). Great, that answers my question, although I still think that it looks odd that you can write <div role="foo"> in XHTML but not in HTML.

Comment on attachment 227434 [details] [diff] [review] Multiple fixes: 1) Recognize xhtml 1.x namespace via Has/GetRoleAttribute(), 2) Implement universal attributes that require no role, 3) Implement id lists for labelledby/describedby, 4) Fix comments >+ // Return PR_TRUE if there is a role attribute >+ static PRBool HasRoleAttribute(nsIContent *aContent) >+ { >+ PRInt32 eltNameSpace = aContent->GetNameSpaceID(); >+ return (eltNameSpace == kNameSpaceID_XHTML && aContent->HasAttr(kNameSpaceID_None, nsAccessibilityAtoms::role)) || >+ aContent->HasAttr(kNameSpaceID_XHTML, nsAccessibilityAtoms::role) || >+ aContent->HasAttr(kNameSpaceID_XHTML2_Unofficial, nsAccessibilityAtoms::role); >+ } >+ >+ // Return PR_TRUE if there is a role attribute, and fill it into aRole >+ static PRBool GetRoleAttribute(nsIContent *aContent, nsAString& aRole) >+ { >+ aRole.Truncate(); >+ PRInt32 eltNameSpace = aContent->GetNameSpaceID(); >+ return (eltNameSpace == kNameSpaceID_XHTML && aContent->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::role, aRole)) || >+ aContent->GetAttr(kNameSpaceID_XHTML, nsAccessibilityAtoms::role, aRole) || >+ aContent->GetAttr(kNameSpaceID_XHTML2_Unofficial, nsAccessibilityAtoms::role, aRole); >+ } I don't see the point of the eltNameSpace temporary, you only use it once. You might want to bracket the first two conditions to make the precedence obvious. >+ // Support idlist as in aaa::labelledby="id1 id2 id3" >+ while (PR_TRUE) { >+ nsAutoString id; >+ PRInt32 idLength = ids.FindChar(' '); // -1 means end of string, not space found >+ NS_ASSERTION(idLength != 0, "Should not be 0 because of CompressWhitespace() call above"); >+ id = Substring(ids, 0, idLength); >+ if (idLength == -1) { >+ ids.Truncate(); >+ } >+ else { >+ ids.Cut(0, idLength + 1); >+ } >+ if (!id.IsEmpty() && id.Last() == ' ') { >+ id.Truncate(idLength - 1); // Eliminate trailing space if it's there >+ } >+ >+ if (id.IsEmpty()) { >+ break; >+ } I don't think your logic is correct here; firstly if idLength == -1 then ids holds the last (or only) id; secondly id.Last() will never be a space, because the substring doesn't include the space you found. Maybe this? while (!ids.IsEmpty()) { nsAutoString id; PRInt32 idLength = ids.FindChar(' '); NS_ASSERTION(idLength != 0, "Should not be 0 because of CompressWhitespace() call above"); if (idLength == kNotFound) { id = ids; ids.Truncate(); } else { id = Substring(ids, 0, idLength); ids.Cut(0, idLength + 1); } sr=me with this fixed.

(In reply to comment #8) > (In reply to comment #7) > >Yes, it works in plain HTML when you do setAttributeNS(), but in that case it > >needs to be namespaced to XHTML or XHTML2 (unfortunately). > Great, that answers my question, although I still think that it looks odd that > you can write <div role="foo"> in XHTML but not in HTML. I agree, it would be nicer just to allow <div role="foo"> in HTML. Maybe we should just do that. After all we have <canvas>. Why neuter things if we don't have to, or make XHTML 1.x more divergent from HTML than it needs to be.

Fixed on trunk.

Is this the only case (other than for case-sensitivity) where the set of recognized attributes and elements differs between HTML and XHTML? I'd rather not introduce differences, especially if there aren't any already.

(In reply to comment #12) > Is this the only case (other than for case-sensitivity) where the set of > recognized attributes and elements differs between HTML and XHTML? I'd rather > not introduce differences, especially if there aren't any already. I agreed in the end and what got checked in allows the role attribute in HTML

This broke the use of namespaced role attributes on branch. You have to use it without namespacing the role attribute at all, which regresses some current generation examples.

Comment on attachment 230695 [details] [diff] [review] Fix branch to use nsIContent:GetAttr() return values properly, not boolean as on trunk >- return (aContent->IsContentOfType(nsIContent::eHTML) && aContent->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::role, aRole)) || >- aContent->GetAttr(kNameSpaceID_XHTML, nsAccessibilityAtoms::role, aRole) || >- aContent->GetAttr(kNameSpaceID_XHTML2_Unofficial, nsAccessibilityAtoms::role, aRole); >+ return (aContent->IsContentOfType(nsIContent::eHTML) && aContent->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::role, aRole) != NS_CONTENT_ATTR_NOT_THERE) || >+ (aContent->GetAttr(kNameSpaceID_XHTML, nsAccessibilityAtoms::role, aRole) != NS_CONTENT_ATTR_NOT_THERE) || >+ (aContent->GetAttr(kNameSpaceID_XHTML2_Unofficial, nsAccessibilityAtoms::role, aRole) != NS_CONTENT_ATTR_NOT_THERE); The extra bracketing on the second and third lines is unnecessary, but if you like it you should also add it to the first line for consistency.

Not a blocker, but we'll take the patch.