Enable accessible name of a control to include multiple <label> elements.

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 e27b54f495c82e0c75f2e48d521be16364cd9e35..2bec3fa21901b8037fc79c3605f8e27280d4bead 100644
--- a/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp
+++ b/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp
@@ -50,6 +50,7 @@
 #include "core/html/HTMLTableRowElement.h"
 #include "core/html/HTMLTableSectionElement.h"
 #include "core/html/HTMLTextAreaElement.h"
+#include "core/html/LabelsNodeList.h"
 #include "core/html/parser/HTMLParserIdioms.h"
 #include "core/html/shadow/MediaControlElements.h"
 #include "core/layout/LayoutBlockFlow.h"
@@ -643,24 +644,6 @@ bool AXNodeObject::isGenericFocusableElement() const
     return true;
 }
 
-HTMLLabelElement* AXNodeObject::labelForElement(const Element* element) const
-{
-    if (!element->isHTMLElement() || !toHTMLElement(element)->isLabelable())
-        return 0;
-
-    const AtomicString& id = element->getIdAttribute();
-    if (!id.isEmpty()) {
-        if (HTMLLabelElement* labelFor = element->treeScope().labelElementForId(id))
-            return labelFor;
-    }
-
-    HTMLLabelElement* labelWrappedElement = Traversal<HTMLLabelElement>::firstAncestor(*element);
-    if (labelWrappedElement && labelWrappedElement->control() == toLabelableElement(element))
-        return labelWrappedElement;
-
-    return 0;
-}
-
 AXObject* AXNodeObject::menuButtonForMenu() const
 {
     Element* menuItem = menuItemElementForMenu();
@@ -1614,7 +1597,7 @@ String AXNodeObject::textAlternative(bool recursive, bool inAriaLabelledByTraver
 
     nameFrom = AXNameFromUninitialized;
 
-    if (foundTextAlternative) {
+    if (nameSources && foundTextAlternative) {
         for (size_t i = 0; i < nameSources->size(); ++i) {
             if (!(*nameSources)[i].text.isNull() && !(*nameSources)[i].superseded) {
                 NameSource& nameSource = (*nameSources)[i];
@@ -1708,9 +1691,8 @@ bool AXNodeObject::nameFromLabelElement() const
     HTMLElement* htmlElement = nullptr;
     if (getNode()->isHTMLElement())
         htmlElement = toHTMLElement(getNode());
-    if (htmlElement && htmlElement->isLabelable()) {
-        HTMLLabelElement* label = labelForElement(htmlElement);
-        if (label)
+    if (htmlElement && isLabelableElement(htmlElement)) {
+        if (toLabelableElement(htmlElement)->labels() && toLabelableElement(htmlElement)->labels()->length() > 0)
             return true;
     }
 
@@ -2177,40 +2159,52 @@ String AXNodeObject::nativeTextAlternative(AXObjectSet& visited, AXNameFrom& nam
     HTMLElement* htmlElement = nullptr;
     if (getNode()->isHTMLElement())
         htmlElement = toHTMLElement(getNode());
+
     if (htmlElement && htmlElement->isLabelable()) {
-        // label
-        nameFrom = AXNameFromRelatedElement;
-        if (nameSources) {
-            nameSources->append(NameSource(*foundTextAlternative));
-            nameSources->last().type = nameFrom;
-            nameSources->last().nativeSource = AXTextFromNativeHTMLLabel;
-        }
-        HTMLLabelElement* label = labelForElement(htmlElement);
-        if (label) {
-            AXObject* labelAXObject = axObjectCache().getOrCreate(label);
-            // Avoid an infinite loop for label wrapped
-            if (labelAXObject && !visited.contains(labelAXObject)) {
-                textAlternative = recursiveTextAlternative(*labelAXObject, false, visited);
+        LabelsNodeList* labels = toLabelableElement(htmlElement)->labels();
+        if (labels && labels->length() > 0) {
+            nameFrom = AXNameFromRelatedElement;

[aboxhall, 2016/05/13 20:19:16]

This needs to be set outside this if block, as before. Perhaps it'd be less
confusing if we renamed this variable "potentialNameFrom"?

Also, need to create and append the NameSource here regardless of whether a
label is found, for the same reason. Have a look at how it's done for
aria-labelledby for how to handle multiple relatedObjects.

[dmazzoni, 2016/05/13 22:22:42]

OK, done. I was thinking it was okay to skip it if there weren't any label
elements, but we want to show the full possible name calculation either way.

I also moved *foundTextAlternative = true up so that it only sets it to true if
at least one label is actually found, just to be safe.

+            StringBuilder accumulatedText;
+            for (unsigned labelIndex = 0; labelIndex < labels->length(); ++labelIndex) {
+                Element* label = labels->item(labelIndex);
+                AXObject* labelAXObject = axObjectCache().getOrCreate(label);
+                // Avoid an infinite loop for label wrapped
+                if (labelAXObject && !visited.contains(labelAXObject)) {
+                    String result = recursiveTextAlternative(*labelAXObject, false, visited);
+                    if (!result.isEmpty()) {
+                        if (!accumulatedText.isEmpty())
+                            accumulatedText.append(" ");
+                        accumulatedText.append(result);
+                    }
 
-                if (relatedObjects) {
-                    localRelatedObjects.append(new NameSourceRelatedObject(labelAXObject, textAlternative));
-                    *relatedObjects = localRelatedObjects;
-                    localRelatedObjects.clear();
-                }
+                    if (relatedObjects) {
+                        localRelatedObjects.append(new NameSourceRelatedObject(labelAXObject, result));
+                        *relatedObjects = localRelatedObjects;
+                    }
 
-                if (nameSources) {
-                    NameSource& source = nameSources->last();
-                    source.relatedObjects = *relatedObjects;
-                    source.text = textAlternative;
-                    if (label->getAttribute(forAttr) == htmlElement->getIdAttribute())
-                        source.nativeSource = AXTextFromNativeHTMLLabelFor;
-                    else
-                        source.nativeSource = AXTextFromNativeHTMLLabelWrapped;
-                    *foundTextAlternative = true;
-                } else {
-                    return textAlternative;
+                    if (nameSources) {
+                        nameSources->append(NameSource(*foundTextAlternative));
+                        NameSource& source = nameSources->last();
+                        source.type = nameFrom;
+                        source.nativeSource = AXTextFromNativeHTMLLabel;
+                        source.relatedObjects = *relatedObjects;
+                        source.text = textAlternative;
+                        if (label->getAttribute(forAttr) == htmlElement->getIdAttribute())
+                            source.nativeSource = AXTextFromNativeHTMLLabelFor;
+                        else
+                            source.nativeSource = AXTextFromNativeHTMLLabelWrapped;
+                    }
                 }
             }
+
+            if (relatedObjects)
+                localRelatedObjects.clear();
+
+            textAlternative = accumulatedText.toString();
+            *foundTextAlternative = true;
+
+            if (!nameSources)
+                return textAlternative;
         }
     }