Bug 459635 - ARIA role should override name computation from subtree flag, r=marcoz, aaronlev

accessible/src/base/nsAccessible.cpp

@@ -1881,7 +1881,10 @@ nsresult nsAccessible::GetHTMLName(nsAString& aLabel, PRBool aCanAggregateSubtre
     }
   }
 
-  if (aCanAggregateSubtree) {
+  PRBool canAggregateName = mRoleMapEntry ?
+                            mRoleMapEntry->nameRule == eNameOkFromChildren :
+                            aCanAggregateSubtree;
+  if (canAggregateName) {
     // Don't use AppendFlatStringFromSubtree for container widgets like menulist
     nsresult rv = AppendFlatStringFromSubtree(content, &aLabel);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -1987,8 +1990,12 @@ nsresult nsAccessible::GetXULName(nsAString& aLabel, PRBool aCanAggregateSubtree
     parent = parent->GetParent();
   }
 
-  // Don't use AppendFlatStringFromSubtree for container widgets like menulist
+  PRBool canAggregateName = mRoleMapEntry ?
-  return aCanAggregateSubtree? AppendFlatStringFromSubtree(content, &aLabel) : NS_OK;
+                            mRoleMapEntry->nameRule == eNameOkFromChildren :
+                            aCanAggregateSubtree;
+
+  return canAggregateName ?
+         AppendFlatStringFromSubtree(content, &aLabel) : NS_OK;
 }
 
 PRBool nsAccessible::IsNodeRelevant(nsIDOMNode *aNode)
@@ -3587,18 +3594,15 @@ nsAccessible::GetARIAName(nsAString& aName)
 nsresult
 nsAccessible::GetNameInternal(nsAString& aName)
 {
-  PRBool canAggregateName = mRoleMapEntry &&
-    mRoleMapEntry->nameRule == eNameOkFromChildren;
-
   nsCOMPtr<nsIContent> content = GetRoleContent(mDOMNode);
   if (!content)
     return NS_OK;
 
   if (content->IsNodeOfType(nsINode::eHTML))
-    return GetHTMLName(aName, canAggregateName);
+    return GetHTMLName(aName, PR_FALSE);
 
   if (content->IsNodeOfType(nsINode::eXUL))
-    return GetXULName(aName, canAggregateName);
+    return GetXULName(aName, PR_FALSE);
 
   return NS_OK;
 }

accessible/src/html/nsHTMLFormControlAccessible.cpp

@@ -278,34 +278,34 @@ NS_IMETHODIMP nsHTMLButtonAccessible::GetRole(PRUint32 *_retval)
 nsresult
 nsHTMLButtonAccessible::GetNameInternal(nsAString& aName)
 {
-  nsAutoString name;
+  nsresult rv = nsAccessible::GetNameInternal(aName);
-  GetHTMLName(name, PR_FALSE);
+  if (!aName.IsEmpty())
+    return NS_OK;
 
-  if (name.IsEmpty()) {
+  nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
-    // no label from HTML or ARIA
+
-    nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
+  // No name from HTML or ARIA
-    if (!content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::value,
+  nsAutoString name;
-                          name) &&
+  if (!content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::value,
-        !content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::alt,
+                        name) &&
-                          name)) {
+      !content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::alt,
-      // Use the button's (default) label if nothing else works
+                        name)) {
-      nsIFrame* frame = GetFrame();
+    // Use the button's (default) label if nothing else works
-      if (frame) {
+    nsIFrame* frame = GetFrame();
-        nsIFormControlFrame* fcFrame;
+    if (frame) {
-        CallQueryInterface(frame, &fcFrame);
+      nsIFormControlFrame* fcFrame = nsnull;
-        if (fcFrame)
+      CallQueryInterface(frame, &fcFrame);
-          fcFrame->GetFormProperty(nsAccessibilityAtoms::defaultLabel, name);
+      if (fcFrame)
-      }
+        fcFrame->GetFormProperty(nsAccessibilityAtoms::defaultLabel, name);
-    }
-    if (name.IsEmpty() &&
-        !content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::title,
-                          name) &&
-        !content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::src,
-                          name)) {
-      content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::data, name);
     }
   }
 
+  if (name.IsEmpty() &&
+      !content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::src,
+                        name)) {
+    content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::data, name);
+  }
+
   name.CompressWhitespace();
   aName = name;
 
@@ -370,6 +370,12 @@ nsHTML4ButtonAccessible::GetState(PRUint32 *aState, PRUint32 *aExtraState)
   return NS_OK;
 }
 
+nsresult
+nsHTML4ButtonAccessible::GetNameInternal(nsAString& aName)
+{
+  return GetHTMLName(aName, PR_TRUE);
+}
+
 // --- textfield -----
 
 nsHTMLTextFieldAccessible::nsHTMLTextFieldAccessible(nsIDOMNode* aNode, nsIWeakReference* aShell):
@@ -391,26 +397,20 @@ NS_IMETHODIMP nsHTMLTextFieldAccessible::GetRole(PRUint32 *aRole)
   return NS_OK;
 }
 
-NS_IMETHODIMP
+nsresult
-nsHTMLTextFieldAccessible::GetName(nsAString& aName)
+nsHTMLTextFieldAccessible::GetNameInternal(nsAString& aName)
 {
-  aName.Truncate();
+  nsresult rv = nsAccessible::GetNameInternal(aName);
-
-  if (IsDefunct())
-    return NS_ERROR_FAILURE;
-
-  nsresult rv = GetARIAName(aName);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (!aName.IsEmpty())
     return NS_OK;
 
   nsCOMPtr<nsIContent> content = do_QueryInterface(mDOMNode);
-  rv = nsAccessible::GetHTMLName(aName, PR_FALSE);
+  if (!content->GetBindingParent())
-  if (NS_FAILED(rv) || !aName.IsEmpty() || !content->GetBindingParent()) {
+    return NS_OK;
-    return rv;
-  }
 
+  // XXX: bug 459640
   // There's a binding parent.
   // This means we're part of another control, so use parent accessible for name.
   // This ensures that a textbox inside of a XUL widget gets

accessible/src/html/nsHTMLFormControlAccessible.h

@@ -101,10 +101,7 @@ public:
   NS_IMETHOD DoAction(PRUint8 index);
 
   // nsAccessible
-  virtual nsresult GetNameInternal(nsAString& aName)
+  virtual nsresult GetNameInternal(nsAString& aName); 
-  {
-    return GetHTMLName(aName, PR_TRUE);
-  }
 };
 
 class nsHTMLTextFieldAccessible : public nsHyperTextAccessibleWrap
@@ -119,7 +116,6 @@ public:
 
   // nsIAccessible
   NS_IMETHOD GetRole(PRUint32 *_retval); 
-  NS_IMETHOD GetName(nsAString& aName); 
   NS_IMETHOD GetValue(nsAString& _retval); 
   NS_IMETHOD GetState(PRUint32 *aState, PRUint32 *aExtraState);
   NS_IMETHOD GetNumActions(PRUint8 *_retval);
@@ -128,6 +124,9 @@ public:
 
   // nsIAccessibleEditableText
   NS_IMETHOD GetAssociatedEditor(nsIEditor **aEditor);
+
+  // nsAccessible
+  virtual nsresult GetNameInternal(nsAString& aName);
 };
 
 class nsHTMLGroupboxAccessible : public nsHyperTextAccessibleWrap

accessible/src/html/nsHTMLImageAccessible.cpp

@@ -139,7 +139,7 @@ nsHTMLImageAccessible::GetNameInternal(nsAString& aName)
   if (!aName.IsEmpty())
     return NS_OK;
 
-  nsresult rv = GetHTMLName(aName, PR_FALSE);
+  nsresult rv = nsAccessible::GetNameInternal(aName);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (aName.IsVoid() && hasAltAttrib) {

accessible/src/html/nsHTMLLinkAccessible.cpp

@@ -59,7 +59,7 @@ NS_IMPL_ISUPPORTS_INHERITED1(nsHTMLLinkAccessible, nsHyperTextAccessibleWrap,
 
 nsresult
 nsHTMLLinkAccessible::GetNameInternal(nsAString& aName)
-{ 
+{
   nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
   nsresult rv = AppendFlatStringFromSubtree(content, &aName);
   NS_ENSURE_SUCCESS(rv, rv);

accessible/src/html/nsHTMLSelectAccessible.h

@@ -84,12 +84,6 @@ public:
   nsHTMLSelectableAccessible(nsIDOMNode* aDOMNode, nsIWeakReference* aShell);
   virtual ~nsHTMLSelectableAccessible() {}
 
-  // nsAccessible
-  virtual nsresult GetNameInternal(nsAString &aName)
-  {
-    return GetHTMLName(aName, PR_FALSE);
-  }
-
 protected:
 
   NS_IMETHOD ChangeSelection(PRInt32 aIndex, PRUint8 aMethod, PRBool *aSelState);

accessible/tests/mochitest/Makefile.in

@@ -70,6 +70,7 @@ _TEST_FILES =\
 		test_nsIAccessible_editablebody.html \
 		test_nsIAccessible_editabledoc.html \
 		test_nsIAccessible_name.html \
+		test_nsIAccessible_name_button.html \
 		test_nsIAccessible_name.xul \
  		test_nsIAccessible_selects.html \
 		test_nsIAccessible_states.html \

accessible/tests/mochitest/common.js

@@ -91,7 +91,7 @@ function getAccessible(aAccOrElmOrID, aInterfaces, aElmObj)
   } else {
     var elm = document.getElementById(aAccOrElmOrID);
     if (!elm) {
-      ok(false, "Can't get DOM element for " + aID);
+      ok(false, "Can't get DOM element for " + aAccOrElmOrID);
       return null;
     }
   }

accessible/tests/mochitest/test_nsIAccessible_name_button.html

@@ -0,0 +1,142 @@
+<html>
+
+<head>
+  <title>nsIAccessible::name calculation for HTML buttons</title>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css" />
+
+  <script type="application/javascript"
+          src="chrome://mochikit/content/MochiKit/packed.js"></script>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/a11y/accessible/common.js"></script>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/a11y/accessible/nsIAccessible_name.js"></script>
+
+  <script type="application/javascript">
+    function doTest()
+    {
+      // html:button, aria-label
+      testName("btn_aria_label", "button label");
+
+      // html:button, aria-labelledby
+      testName("btn_aria_labelledby_text", "text");
+
+      // html:button, html:label
+      testName("btn_labelled", "label");
+
+      // html:button, name from content
+      testName("btn_namefromcontent", "1");
+
+      // html:button, no name from content
+      testName("btn_nonamefromcontent", null);
+
+      // @html:button, title
+      testName("btn_title", "title");
+
+      // html:input, aria-label
+      testName("input_aria_label", "button label");
+
+      // html:input, aria-labelledby
+      testName("input_aria_labelledby_text", "text");
+
+      // html:input, html:label
+      testName("input_labelled", "label");
+
+      // html:input, @title
+      testName("input_title", "title");
+
+      // html:input, @value
+      testName("input_value", "1");
+
+      // html:input, @alt
+      testName("input_alt", "alt");
+
+      // html:input, @src
+      testName("input_src", "src");
+
+      // html:input, @data
+      testName("input_data", "data");
+
+      SimpleTest.finish();
+    }
+
+    SimpleTest.waitForExplicitFinish();
+    addLoadEvent(doTest);
+  </script>
+
+</head>
+
+<body>
+
+  <a target="_blank"
+     href="https://bugzilla.mozilla.org/show_bug.cgi?id=459635"
+     title="nsIAccessible::name calculation for HTML buttons">
+    Mozilla Bug 459635
+  </a>
+  <p id="display"></p>
+  <div id="content" style="display: none"></div>
+  <pre id="test">
+  </pre>
+
+  <!-- button, aria-label, preferred to aria-labelledby -->
+  <button id="btn_aria_label"
+          aria-label="button label"
+          aria-labelledby="btn_aria_labelledby_text">1</button>
+  <br/>
+
+  <!-- button, aria-labelledby, preferred to html:label -->
+  <span id="aria_labelledby_text">text</span>
+  <label for="btn_aria_labelledby_text">label</label>
+  <button id="btn_aria_labelledby_text"
+          aria-labelledby="aria_labelledby_text">1</button>
+  <br/>
+
+  <!-- button, label, preferred to name from content -->
+  <label for="btn_labelled">label</label>
+  <button id="btn_labelled">1</button>
+
+  <!-- button, name from content, preferred to @title -->
+  <button id="btn_namefromcontent" title="title">1</button>
+
+  <!-- button, no name from content, ARIA role overrides this rule -->
+  <button id="btn_nonamefromcontent" role="presentation">1</button>
+
+  <!-- button, no content, name from @title -->
+  <button id="btn_title" title="title"></button>
+
+  <!-- input, aria-label, preferred to aria-labelledby -->
+  <input type="button" id="input_aria_label"
+          aria-label="button label"
+          aria-labelledby="aria_labelledby_text_for_input"
+          value="1"/>
+  <br/>
+
+  <!-- aria-labelledby, preferred to html:label -->
+  <span id="aria_labelledby_text_for_input">text</span>
+  <label for="input_aria_labelledby_text">label</label>
+  <input type="button" id="input_aria_labelledby_text"
+         aria-labelledby="aria_labelledby_text_for_input"
+         value="1"/>
+  <br/>
+
+  <!-- label, preferred to @title -->
+  <label for="input_labelled">label</label>
+  <input type="button" id="input_labelled" value="1" title="title"/>
+
+  <!-- name from @title, prefered to @value -->
+  <input type="button" id="input_title" title="title" value="1"/>
+
+  <!-- name from @value, prefered to @alt -->
+  <input type="button" id="input_value" value="1" alt="alt"/>
+
+  <!-- name from @alt, preferred to @src -->
+  <input type="button" id="input_alt" alt="alt" @src="src"/>
+
+  <!-- name from @src, preferred to @data -->
+  <input type="button" id="input_src" src="src" data="data"/>
+
+  <!-- name from @data -->
+  <input type="button" id="input_data" data="data"/>
+</body>
+</html>