Fix bugs in accessible name calculation code exposed by Bryan Garaventa's test files.

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

 /*
  * Copyright (C) 2008, 2009, 2011 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 658 matching lines @@
     updateCachedAttributeValuesIfNeeded();
     return m_cachedHasInheritedPresentationalRole;
 }
 
 bool AXObject::isPresentationalChild() const
 {
     updateCachedAttributeValuesIfNeeded();
     return m_cachedIsPresentationalChild;
 }
 
+// Simplify and whitespace, but preserve a single leading and trailing whitespace

[aboxhall, 2015/11/30 19:20:19]

First clause seems a little weird - can't tell if it's missing a word or got an
extra one.

[dmazzoni, 2015/12/01 08:52:39]

Done.

+// character if it's present.
+// static
+String AXObject::collapseWhitespace(const String& str)
+{
+    StringBuilder result;
+    if (!str.isEmpty() && isHTMLSpace<UChar>(str[0]))
+        result.append(' ');
+    result.append(str.simplifyWhiteSpace(isHTMLSpace<UChar>));
+    if (!str.isEmpty() && isHTMLSpace<UChar>(str[str.length() - 1]))
+        result.append(' ');
+    return result.toString();
+}
+
 String AXObject::computedName() const
 {
     AXNameFrom nameFrom;
     AXObject::AXObjectVector nameObjects;
     return name(nameFrom, &nameObjects);
 }
 
 String AXObject::name(AXNameFrom& nameFrom, AXObject::AXObjectVector* nameObjects) const
 {
     HeapHashSet<Member<const AXObject>> visited;
     AXRelatedObjectVector relatedObjects;
     String text = textAlternative(false, false, visited, nameFrom, &relatedObjects, nullptr);
 
     if (!node() || !isHTMLBRElement(node()))
-        text = text.simplifyWhiteSpace(isHTMLSpace<UChar>, WTF::DoNotStripWhiteSpace);
+        text = collapseWhitespace(text);
 
     if (nameObjects) {
         nameObjects->clear();
         for (size_t i = 0; i < relatedObjects.size(); i++)
             nameObjects->append(relatedObjects[i]->object);
     }
     return text;
 }
 
 String AXObject::name(NameSources* nameSources) const
 {
     AXObjectSet visited;
     AXNameFrom tmpNameFrom;
     AXRelatedObjectVector tmpRelatedObjects;
     String text = textAlternative(false, false, visited, tmpNameFrom, &tmpRelatedObjects, nameSources);
     text = text.simplifyWhiteSpace(isHTMLSpace<UChar>);
     return text;
 }
 
 String AXObject::recursiveTextAlternative(const AXObject& axObj, bool inAriaLabelledByTraversal, AXObjectSet& visited)
 {
     AXNameFrom tmpNameFrom;
     return axObj.textAlternative(true, inAriaLabelledByTraversal, visited, tmpNameFrom, nullptr, nullptr);
 }
 
+bool AXObject::isHiddenForTextAlternativeCalculation() const
+{
+    if (equalIgnoringCase(getAttribute(aria_hiddenAttr), "false"))
+        return false;
+
+    if (layoutObject())
+        return layoutObject()->style()->visibility() != VISIBLE;
+
+    // This is an obscure corner case: if a node has no LayoutObject, that means it's not rendered,

[aboxhall, 2015/11/30 19:20:19]

Helpful comment, thanks!

+    // but we still may be exploring it as part of a text alternative calculation, for example if it
+    // was explicitly referenced by aria-labelledby. So we need to explicitly call the style resolver
+    // to check whether it's invisible or display:none, rather than relying on the style cached in the
+    // LayoutObject.
+    Document* doc = document();
+    if (doc && node() && node()->isElementNode()) {
+        RefPtr<ComputedStyle> style = doc->ensureStyleResolver().styleForElement(toElement(node()));
+        return style->display() == NONE || style->visibility() != VISIBLE;
+    }
+
+    return false;
+}
+
 String AXObject::ariaTextAlternative(bool recursive, bool inAriaLabelledByTraversal, AXObjectSet& visited, AXNameFrom& nameFrom, AXRelatedObjectVector* relatedObjects, NameSources* nameSources, bool* foundTextAlternative) const
 {
     String textAlternative;
     bool alreadyVisited = visited.contains(this);
     visited.add(this);
 
     // Step 2A from: http://www.w3.org/TR/accname-aam-1.1
     // If you change this logic, update AXNodeObject::nameFromLabelElement, too.
-    if (!recursive && layoutObject()
-        && layoutObject()->style()->visibility() != VISIBLE
-        && !equalIgnoringCase(getAttribute(aria_hiddenAttr), "false")) {
+    if (!inAriaLabelledByTraversal && isHiddenForTextAlternativeCalculation()) {

[aboxhall, 2015/11/30 19:20:19]

This looks much better now.

+        *foundTextAlternative = true;
         return String();
     }
 
     // Step 2B from: http://www.w3.org/TR/accname-aam-1.1
     // If you change this logic, update AXNodeObject::nameFromLabelElement, too.
     if (!inAriaLabelledByTraversal && !alreadyVisited) {
         const QualifiedName& attr = hasAttribute(aria_labeledbyAttr) && !hasAttribute(aria_labelledbyAttr) ? aria_labeledbyAttr : aria_labelledbyAttr;
         nameFrom = AXNameFromRelatedElement;
         if (nameSources) {
             nameSources->append(NameSource(*foundTextAlternative, attr));
@@ skipping 941 matching lines @@
 
 DEFINE_TRACE(AXObject)
 {
     visitor->trace(m_children);
     visitor->trace(m_parent);
     visitor->trace(m_cachedLiveRegionRoot);
     visitor->trace(m_axObjectCache);
 }
 
 } // namespace blink