342979 - Role attribute should be recognized in XHTML 1.x namespace

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

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.

Comment 1

[Aaron Leventhal]

Attachment: 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

The universal properties will fix bug 337128

Comment 2

[Mark Pilgrim (inactive)]

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

Comment 3

[Mark Pilgrim (inactive)]

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

Comment 4

[[e-mail]]

Roles only apply to XHTML docs, right?

Comment 5

[Aaron Leventhal]

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

Comment 6

[[e-mail]]

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

Comment 7

[Aaron Leventhal]

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

Comment 8

[[e-mail]]

(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 9

[[e-mail]]

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.

Comment 10

[Aaron Leventhal]

(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.

Comment 11

[Aaron Leventhal]

Fixed on trunk.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 13

[Aaron Leventhal]

(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

Comment 14

[Aaron Leventhal]

Attachment: Fix branch to use nsIContent:GetAttr() return values properly, not boolean as on trunk

Comment 15

[Aaron Leventhal]

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 16

[Aaron Leventhal]

Attachment: Testcase: three ways of using role attribute in an xhtml 1 file, all must work

Comment 17

[[e-mail]]

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.

Comment 18

[Mike Connor [:mconnor]]

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