Handle aria-owns and presentational children in AX name calc.

third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp

Index: third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp
diff --git a/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp b/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp
index 96bdd2579a833fc65c58ac72478832d680bde08a..36f47d6b5b19b3972c90f28efe693be5ea44ff9c 100644
--- a/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp
+++ b/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp
@@ -1474,7 +1474,7 @@ static bool isInSameNonInlineBlockFlow(LayoutObject* r1, LayoutObject* r2)
 
 AXObject* AXNodeObject::findChildWithTagName(const HTMLQualifiedName& tagName) const
 {
-    for (AXObject* child = rawFirstChild(); child; child = child->rawFirstSibling()) {
+    for (AXObject* child = rawFirstChild(); child; child = child->rawNextSibling()) {
         Node* childNode = child->node();
         if (childNode && childNode->hasTagName(tagName))
             return child;
@@ -1543,7 +1543,7 @@ String AXNodeObject::textAlternative(bool recursive, bool inAriaLabelledByTraver
         else if (isHTMLBRElement(node))
             textAlternative = String("\n");
         else
-            textAlternative = textFromDescendants(visited);
+            textAlternative = textFromDescendants(visited, false);
 
         if (!textAlternative.isEmpty()) {
             if (nameSources) {
@@ -1589,11 +1589,26 @@ String AXNodeObject::textAlternative(bool recursive, bool inAriaLabelledByTraver
     return String();
 }
 
-String AXNodeObject::textFromDescendants(AXObjectSet& visited) const
+String AXNodeObject::textFromDescendants(AXObjectSet& visited, bool recursive) const

[aboxhall, 2015/12/14 22:40:37]

Could 'recursive' be renamed to something like 'inRecursiveCall'? 'recursive'
seems to have too many potential meanings...

[dmazzoni, 2015/12/14 22:51:06]

This is the same as textAlternative so based on our chat let's leave it for now
or change both later.

 {
+    if (!canHaveChildren() && recursive)
+        return String();
+
     StringBuilder accumulatedText;
     AXObject* previous = nullptr;
-    for (AXObject* child = rawFirstChild(); child; child = child->rawFirstSibling()) {
+
+    AXObjectVector children;
+
+    HeapVector<Member<AXObject>> ownedChildren;
+    computeAriaOwnsChildren(ownedChildren);
+    for (AXObject* obj = rawFirstChild(); obj; obj = obj->rawNextSibling()) {

[aboxhall, 2015/12/14 22:40:37]

Where else do we conflate "native" and aria-owned children? Do we need to add a
note to keep them in sync; or, alternatively, should we make a method to iterate
over the combined set of children?

[dmazzoni, 2015/12/14 22:51:06]

Great question, I just searched the code and there's no other use of
rawFirstChild that doesn't handle aria-owns already if it should. The one
exception is dead code which I'm fixing in another change I just mailed out.

Basically 90% of the time we should just call children(), which does the right
thing.

In the few cases where we need to explore the tree with fewer side effects, like
here, we need to take aria-owns into account. It's only a couple of extra lines
of code.

+        if (!axObjectCache().isAriaOwned(obj))
+            children.append(obj);
+    }
+    for (const auto& ownedChild : ownedChildren)
+        children.append(ownedChild);
+
+    for (AXObject* child : children) {
         // Skip hidden children
         if (child->isInertOrAriaHidden())
             continue;
@@ -1608,7 +1623,11 @@ String AXNodeObject::textFromDescendants(AXObjectSet& visited) const
                 accumulatedText.append(' ');
         }
 
-        String result = recursiveTextAlternative(*child, false, visited);
+        String result;
+        if (child->isPresentational())
+            result = child->textFromDescendants(visited, true);
+        else
+            result = recursiveTextAlternative(*child, false, visited);
         accumulatedText.append(result);
         previous = child;
     }
@@ -1745,7 +1764,7 @@ AXObject* AXNodeObject::rawFirstChild() const
     return axObjectCache().getOrCreate(firstChild);
 }
 
-AXObject* AXNodeObject::rawFirstSibling() const
+AXObject* AXNodeObject::rawNextSibling() const
 {
     if (!node())
         return 0;
@@ -2081,7 +2100,7 @@ void AXNodeObject::updateAccessibilityRole()
         childrenChanged();
 }
 
-void AXNodeObject::computeAriaOwnsChildren(HeapVector<Member<AXObject>>& ownedChildren)
+void AXNodeObject::computeAriaOwnsChildren(HeapVector<Member<AXObject>>& ownedChildren) const
 {
     if (!hasAttribute(aria_ownsAttr))
         return;
@@ -2606,7 +2625,7 @@ String AXNodeObject::description(AXNameFrom nameFrom, AXDescriptionFrom& descrip
         }
 
         AXObjectSet visited;
-        description = textFromDescendants(visited);
+        description = textFromDescendants(visited, false);
 
         if (!description.isEmpty()) {
             if (descriptionSources) {