Bug 443081 – ARIA name from child content is incorrectly including child elements that have display:none, r=aaronlev, MarcoZ
authorAlexander Surkov <surkov.alexander@gmail.com>
Wed, 16 Jul 2008 23:24:36 +0800
changeset 15971 9e360506ae23a45122ed5ab036c821c76e8c104a
parent 15970 895b09efac22a6bbd2d466006b2d5975286616aa
child 15972 a61f4bfced4eb57d58332f49ecda7950e5deea15
push idunknown
push userunknown
push dateunknown
reviewersaaronlev, MarcoZ
bugs443081
milestone1.9.1a1pre
Bug 443081 – ARIA name from child content is incorrectly including child elements that have display:none, r=aaronlev, MarcoZ
accessible/src/base/nsAccessible.cpp
accessible/src/base/nsAccessible.h
accessible/tests/mochitest/test_nsIAccessible_name.html
accessible/tests/mochitest/test_nsIAccessible_name.xul
--- a/accessible/src/base/nsAccessible.cpp
+++ b/accessible/src/base/nsAccessible.cpp
@@ -1588,25 +1588,16 @@ nsresult nsAccessible::AppendFlatStringF
       }
     }
     return NS_OK;
   }
 
   nsAutoString textEquivalent;
   if (!aContent->IsNodeOfType(nsINode::eHTML)) {
     if (aContent->IsNodeOfType(nsINode::eXUL)) {
-      nsCOMPtr<nsIPresShell> shell = GetPresShell();
-      if (!shell) {
-        return NS_ERROR_FAILURE;  
-      }
-      nsIFrame *frame = shell->GetPrimaryFrameFor(aContent);
-      if (!frame || !frame->GetStyleVisibility()->IsVisible()) {
-        return NS_OK;
-      }
-
       nsCOMPtr<nsIDOMXULLabeledControlElement> labeledEl(do_QueryInterface(aContent));
       if (labeledEl) {
         labeledEl->GetLabel(textEquivalent);
       }
       else {
         if (aContent->NodeInfo()->Equals(nsAccessibilityAtoms::label, kNameSpaceID_XUL)) {
           aContent->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::value, textEquivalent);
         }
@@ -1656,18 +1647,27 @@ nsresult nsAccessible::AppendFlatStringF
 
 
 nsresult nsAccessible::AppendFlatStringFromSubtree(nsIContent *aContent, nsAString *aFlatString)
 {
   static PRBool isAlreadyHere; // Prevent recursion which can cause infinite loops
   if (isAlreadyHere) {
     return NS_OK;
   }
+
   isAlreadyHere = PR_TRUE;
-  nsresult rv = AppendFlatStringFromSubtreeRecurse(aContent, aFlatString);
+
+  nsCOMPtr<nsIPresShell> shell = GetPresShell();
+  NS_ENSURE_TRUE(shell, NS_ERROR_FAILURE);
+
+  nsIFrame *frame = shell->GetPrimaryFrameFor(aContent);
+  PRBool isHidden = (!frame || !frame->GetStyleVisibility()->IsVisible());
+  nsresult rv = AppendFlatStringFromSubtreeRecurse(aContent, aFlatString,
+                                                   isHidden);
+
   isAlreadyHere = PR_FALSE;
 
   if (NS_SUCCEEDED(rv) && !aFlatString->IsEmpty()) {
     nsAString::const_iterator start, end;
     aFlatString->BeginReading(start);
     aFlatString->EndReading(end);
 
     PRInt32 spacesToTruncate = 0;
@@ -1676,37 +1676,55 @@ nsresult nsAccessible::AppendFlatStringF
 
     if (spacesToTruncate > 0)
       aFlatString->Truncate(aFlatString->Length() - spacesToTruncate);
   }
 
   return rv;
 }
 
-nsresult nsAccessible::AppendFlatStringFromSubtreeRecurse(nsIContent *aContent, nsAString *aFlatString)
+nsresult
+nsAccessible::AppendFlatStringFromSubtreeRecurse(nsIContent *aContent,
+                                                 nsAString *aFlatString,
+                                                 PRBool aIsRootHidden)
 {
   // Depth first search for all text nodes that are decendants of content node.
   // Append all the text into one flat string
   PRUint32 numChildren = 0;
   nsCOMPtr<nsIDOMXULSelectControlElement> selectControlEl(do_QueryInterface(aContent));
   if (!selectControlEl) {  // Don't walk children of elements with options, just get label directly
     numChildren = aContent->GetChildCount();
   }
 
   if (numChildren == 0) {
     // There are no children or they are irrelvant: get the text from the current node
     AppendFlatStringFromContentNode(aContent, aFlatString);
     return NS_OK;
   }
 
   // There are relevant children: use them to get the text.
+  nsCOMPtr<nsIPresShell> shell = GetPresShell();
+  NS_ENSURE_TRUE(shell, NS_ERROR_FAILURE);
+
   PRUint32 index;
   for (index = 0; index < numChildren; index++) {
-    AppendFlatStringFromSubtreeRecurse(aContent->GetChildAt(index), aFlatString);
+    nsCOMPtr<nsIContent> childContent = aContent->GetChildAt(index);
+
+    // Walk into hidden subtree if the the root parent is also hidden. This
+    // happens when the author explictly uses a hidden label or description.
+    if (!aIsRootHidden) {
+      nsIFrame *childFrame = shell->GetPrimaryFrameFor(childContent);
+      if (!childFrame || !childFrame->GetStyleVisibility()->IsVisible())
+        continue;
+    }
+
+    AppendFlatStringFromSubtreeRecurse(childContent, aFlatString,
+                                       aIsRootHidden);
   }
+
   return NS_OK;
 }
 
 nsIContent *nsAccessible::GetLabelContent(nsIContent *aForNode)
 {
   if (aForNode->IsNodeOfType(nsINode::eXUL))
     return nsAccUtils::FindNeighbourPointingToNode(aForNode, nsAccessibilityAtoms::control,
                                                    nsAccessibilityAtoms::label);
--- a/accessible/src/base/nsAccessible.h
+++ b/accessible/src/base/nsAccessible.h
@@ -192,17 +192,32 @@ protected:
   nsresult AppendNameFromAccessibleFor(nsIContent *aContent, nsAString *aFlatString,
                                        PRBool aFromValue = PR_FALSE);
   nsresult AppendFlatStringFromContentNode(nsIContent *aContent, nsAString *aFlatString);
   nsresult AppendStringWithSpaces(nsAString *aFlatString, const nsAString& textEquivalent);
 
   // helper method to verify frames
   static nsresult GetFullKeyName(const nsAString& aModifierName, const nsAString& aKeyName, nsAString& aStringOut);
   static nsresult GetTranslatedString(const nsAString& aKey, nsAString& aStringOut);
-  nsresult AppendFlatStringFromSubtreeRecurse(nsIContent *aContent, nsAString *aFlatString);
+
+  /**
+   * Walk into subtree and calculate the string which is used as the accessible
+   * name or description.
+   *
+   * @param aContent      [in] traversed content
+   * @param aFlatString   [in, out] result string
+   * @param aIsRootHidden [in] specifies whether root content (we started to
+   *                      traverse from) is hidden, in this case the result
+   *                      string is calculated from hidden children
+   *                      (this is used when hidden root content is explicitly
+   *                      specified as label or description by author)
+   */
+  nsresult AppendFlatStringFromSubtreeRecurse(nsIContent *aContent,
+                                              nsAString *aFlatString,
+                                              PRBool aIsRootHidden);
 
   // Helpers for dealing with children
   virtual void CacheChildren();
   
   // nsCOMPtr<>& is useful here, because getter_AddRefs() nulls the comptr's value, and NextChild
   // depends on the passed-in comptr being null or already set to a child (finding the next sibling).
   nsIAccessible *NextChild(nsCOMPtr<nsIAccessible>& aAccessible);
     
--- a/accessible/tests/mochitest/test_nsIAccessible_name.html
+++ b/accessible/tests/mochitest/test_nsIAccessible_name.html
@@ -53,17 +53,25 @@
       testName("btn_labelledby_texts", "text1 text2");
 
 
       //////////////////////////////////////////////////////////////////////////
       // Name from subtree (single relation labelled_by).
       
       // Gets the name from text nodes contained by nested elements
       testName("btn_labelledby_mixed", "nomore text");
-  
+
+      // Gets the name from text nodes contained by nested elements, ignores
+      // hidden elements (bug 443081).
+      testName("btn_labelledby_mixed_hidden_child", "nomore text2");
+
+      // Gets the name from hidden text nodes contained by nested elements, 
+      // (label element is hidden entirely), (bug 443081).
+      testName("btn_labelledby_mixed_hidden", "lala more hidden text");
+
       // Gets the name from text nodes contained by nested elements having block
       // representation (every text node value in the name should be devided by
       // spaces)
       testName("btn_labelledby_mixed_block", "text more text");
 
       // Gets the name from text nodes contained by html:td (every text node
       // value in the name should be devided by spaces).
       // XXX: this case is rather a feature than strong wanted behaviour.
@@ -108,16 +116,20 @@
 
       //////////////////////////////////////////////////////////////////////////
       // name from children
 
       // ARIA role button is presented allowing the name calculation from
       // children.
       testName("btn_children", "14");
 
+      // ARIA role option is presented allowing the name calculation from
+      // visible children (bug 443081).
+      testName("lb_opt1_children_hidden", "i am visible");
+
 
       //////////////////////////////////////////////////////////////////////////
       // title attribute
 
       // If nothing is left. Let's try title attribute.
       testName("btn_title", "title");
 
       SimpleTest.finish();
@@ -155,16 +167,35 @@
   <br/>
 
   <!-- the name from subtree, mixed content -->
   <span id="labelledby_mixed">no<span>more text</span></span>
   <button id="btn_labelledby_mixed"
           aria-labelledby="labelledby_mixed">3</button>
   <br/>
 
+  <!-- the name from subtree, mixed/hidden content -->
+  <span id="labelledby_mixed_hidden_child">
+    no<span>more 
+      <span style="display: none;">hidden</span>
+      text2
+      <span style="visibility: hidden">hidden2</span>
+    </span>
+  </span>
+  <button id="btn_labelledby_mixed_hidden_child"
+          aria-labelledby="labelledby_mixed_hidden_child">3.1</button>
+  <br/>
+
+  <!-- the name from subtree, mixed/completely hidden content -->
+  <span id="labelledby_mixed_hidden"
+        style="display: none;">lala <span>more hidden </span>text</span></span>
+  <button id="btn_labelledby_mixed_hidden"
+          aria-labelledby="labelledby_mixed_hidden">3.2</button>
+  <br/>
+
   <!-- the name from subtree, mixed content, block structure -->
   <div id="labelledby_mixed_block"><div>text</div>more text</div></div>
   <button id="btn_labelledby_mixed_block"
           aria-labelledby="labelledby_mixed_block">4</button>
   <br/>
 
   <!-- the name from subtree, mixed content, table structure -->
   <table><tr>
@@ -223,12 +254,20 @@
 
   <!-- label element, label and the button are in the same document -->
   <label for="btn_label_indocument">in document</label>
   <button id="btn_label_indocument">13</button>
 
   <!-- name from children -->
   <span id="btn_children" role="button">14</span>
 
+  <!-- name from children, hidden children -->
+  <div role="listbox" tabindex="0">
+    <div id="lb_opt1_children_hidden" role="option" tabindex="0">
+      <span>i am visible</span>
+      <span style="display:none">i am hidden</span>
+    </div>
+  </div>
+
   <!-- name from title attribute -->
   <span id="btn_title" role="group" title="title">15</span>
 </body>
 </html>
--- a/accessible/tests/mochitest/test_nsIAccessible_name.xul
+++ b/accessible/tests/mochitest/test_nsIAccessible_name.xul
@@ -67,16 +67,24 @@
       // Gets the name from text nodes contained by nested elements.
       testName("btn_labelledby_mixed", "nomore text");
 
       // Gets the name from text nodes and selected item of menulist
       // (other items are ignored).
       testName("btn_labelledby_mixed_menulist",
                "nomore text selected item more text");
       
+      // Gets the name from text nodes contained by nested elements, ignores
+      // hidden elements (bug 443081).
+      testName("btn_labelledby_mixed_hidden_child", "nomore text2");
+
+      // Gets the name from hidden text nodes contained by nested elements, 
+      // (label element is hidden entirely), (bug 443081)
+      testName("btn_labelledby_mixed_hidden", "lala more hidden text");
+
 
       //////////////////////////////////////////////////////////////////////////
       // Name for nsIDOMXULLabeledControlElement.
 
       // Gets the name from @label attribute.
       testName("btn_nsIDOMXULLabeledControlElement", "labeled element");
 
 
@@ -134,16 +142,17 @@
            "Wrong label for anonymous textbox of " + ID);
       }
 
 
       //////////////////////////////////////////////////////////////////////////
       // tooltiptext (if nothing above isn't presented then tooltiptext is used)
       testName("box_tooltiptext", "tooltiptext label");
 
+
       //////////////////////////////////////////////////////////////////////////
       // Name from the @title attribute of <toolbaritem/> (original bug 237249).
 
       // Direct child of toolbaritem.
       var textboxAcc = testName("toolbaritem_textbox", "ooospspss");
 
       // Element from anonymous content of direct child of toolbaritem.
       var dropmarkerAcc = textboxAcc.lastChild;
@@ -158,16 +167,20 @@
 
       //////////////////////////////////////////////////////////////////////////
       // Name from children
 
       // ARIA role button is presented allowing the name calculation from
       // children.
       testName("box_children", "14");
 
+      // ARIA role option is presented allowing the name calculation from
+      // the visible children (bug 443081)
+      testName("lb_opt1_children_hidden", "i am visible");
+
 
       SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
     addLoadEvent(doTest);
   ]]>
   </script>
@@ -198,16 +211,28 @@
 
   <!-- the name from subtree, mixed content -->
   <description id="labelledby_mixed">
     no<description>more text</description>
   </description>
   <button id="btn_labelledby_mixed"
           aria-labelledby="labelledby_mixed"/>
 
+  <!-- the name from subtree, mixed/hidden content -->
+  <description id="labelledby_mixed_hidden_child">no<description>more <description hidden="true">hidden</description>text2</description></description>
+  <button id="btn_labelledby_mixed_hidden_child"
+          aria-labelledby="labelledby_mixed_hidden_child"/>
+
+  <!-- the name from subtree, mixed/completely hidden content -->
+  <description id="labelledby_mixed_hidden"
+               hidden="true">lala <description>more hidden </description>text</description>
+  <button id="btn_labelledby_mixed_hidden"
+          aria-labelledby="labelledby_mixed_hidden"/>
+  <br/>
+
   <!-- the name from subtree, mixed content, ignore items of menulist -->
   <description id="labelledby_mixed_menulist">
     no<description>more text</description>
     <menulist>
       <menupopup>
         <menuitem label="selected item"/>
         <menuitem label="item"/>
       </menupopup>
@@ -276,10 +301,18 @@
         </hbox>
       </textbox>
     </toolbaritem>
   </toolbar>
 
   <!-- name from children -->
   <box id="box_children" role="button">14</box>
 
+  <!-- name from children, hidden children -->
+  <vbox role="listbox" tabindex="0">
+    <hbox id="lb_opt1_children_hidden" role="option" tabindex="0">
+      <description>i am visible</description>
+      <description style="display:none">i am hidden</description>
+    </hbox>
+  </vbox>
+
 </window>