AX: in an aria-labelledby computation, do not traverse into elements whose nameFrom...
authorcfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Sep 2014 18:18:44 +0000 (18:18 +0000)
committercfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Sep 2014 18:18:44 +0000 (18:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136714

Reviewed by Darin Adler.

Source/WebCore:

There are certain ARIA elements that tell us we should not query their children when determining the name of the object.
Those ones have the "nameFrom" property set to "author" instead of "contents." WebKit needs to honor that status.

Test: accessibility/aria-namefrom-author.html
      Modified: accessibility/aria-labelledby-with-descendants.html

* accessibility/AccessibilityNodeObject.cpp:
(WebCore::shouldUseAccessiblityObjectInnerText):
(WebCore::shouldAddSpaceBeforeAppendingNextElement):
(WebCore::appendNameToStringBuilder):
(WebCore::AccessibilityNodeObject::textUnderElement):
(WebCore::accessibleNameForNode):
(WebCore::AccessibilityNodeObject::accessibilityDescriptionForElements):
* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::accessibleNameDerivesFromContent):
(WebCore::initializeRoleMap):
* accessibility/AccessibilityObject.h:

LayoutTests:

* accessibility/aria-labelledby-with-descendants-expected.txt:
* accessibility/aria-labelledby-with-descendants.html:
* accessibility/aria-namefrom-author-expected.txt: Added.
* accessibility/aria-namefrom-author.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@174074 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/accessibility/aria-labelledby-with-descendants-expected.txt
LayoutTests/accessibility/aria-labelledby-with-descendants.html
LayoutTests/accessibility/aria-namefrom-author-expected.txt [new file with mode: 0644]
LayoutTests/accessibility/aria-namefrom-author.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityNodeObject.cpp
Source/WebCore/accessibility/AccessibilityObject.cpp
Source/WebCore/accessibility/AccessibilityObject.h

index 0270fbf..8729025 100644 (file)
@@ -1,3 +1,15 @@
+2014-09-29  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: in an aria-labelledby computation, do not traverse into elements whose nameFrom value does not include 'contents'
+        https://bugs.webkit.org/show_bug.cgi?id=136714
+
+        Reviewed by Darin Adler.
+
+        * accessibility/aria-labelledby-with-descendants-expected.txt:
+        * accessibility/aria-labelledby-with-descendants.html:
+        * accessibility/aria-namefrom-author-expected.txt: Added.
+        * accessibility/aria-namefrom-author.html: Added.
+
 2014-09-29  Diego Pino Garcia  <dpino@igalia.com>
 
         Missing changes from r174049
index 8c9fc99..e5c16fc 100644 (file)
@@ -1,9 +1,31 @@
+
+
 This tests that if aria-labelledby is pointing to nodes with descendants, it returns all text.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-aria-labelledby description: AXDescription: hello link world test1 test2 test3
+test 1: aria-labelledby description: AXDescription: hello link use world test1 test2 test3
+test 1: expected description: hello link use world test1 test2 test3
+
+test 2: aria-labelledby description: AXDescription: foo bar
+test 2: expected description: foo bar
+
+test 3: aria-labelledby description: AXDescription: foo bar
+test 3: expected description: foo bar
+
+test 4: aria-labelledby description: AXDescription: foo
+test 4: expected description: foo
+
+test 5: aria-labelledby description: AXDescription: Delete 
+test 5: expected description: Delete
+
+test 6: aria-labelledby description: AXDescription: Delete product name
+test 6: expected description: Delete product name
+
+test 7: aria-labelledby description: AXDescription: foo bar baz bop bap boom
+test 7: expected description: foo bar baz bop bap boom
+
 PASS successfullyParsed is true
 
 TEST COMPLETE
index a98ebe8..0091721 100644 (file)
@@ -7,18 +7,35 @@
 
 <div id="content">
 
-<div aria-labelledby="a b" id="group" role="group">
-group text
-</div>
+<div class="ex" data-label="hello link use world test1 test2 test3" aria-labelledby="a1 b1" id="test1" role="group">group text</div>
+<div id="a1">hello <a href="#">link</a> <div role="group">skip</div> <div role="group" aria-label="use">skip</div> world</div>
+<!-- paragraph role is ambiguous so okay to leave next example as it -->
+<div id="b1"><button>test1</button><p tabindex=0>test2 <span aria-hidden="true">hidden</span> test3</p></div>
 
-<div id="a">
-hello <a href="#">link</a> world
-</div>
+<input type="checkbox" aria-labelledby="reverselabelfor" id="test2" class="ex" data-label="foo bar" data-note="labelled by label element">
+<label id="reverselabelfor">foo <span role="text" aria-label="bar">skip</span></label>
+<br>
+
+<input type="checkbox" aria-labelledby="reverselabelforwithdiv" id="test3" class="ex" data-label="foo bar" data-note="labelled by div element">
+<div id="reverselabelforwithdiv">foo <span role="text" aria-label="bar">skip</span></div>
+<br>
+
+<!-- would be "bar" natively, but label explicitly overridden to "foo" by labelledby -->
+<div id="overridesnativelabel">foo</div><label for="nativelabeloverridden">bar</label>
+<input class="ex" data-label="foo" id="test4" type="text" aria-labelledby="overridesnativelabel" data-note="overrides native label">
+
+<!-- self-referencing labelledby in combination with external reference -->
+<div id="productname1"><nav role="navigation"><!-- nav does not get nameFrom:contents -->product name</nav></div>
+<button class="ex" data-label="Delete" aria-label="Delete" aria-labelledby="test5 productname1" id="test5" data-note="self-referencial labelledby includes nav">x</button>
+
+<!-- self-referencing labelledby in combination with external reference -->
+<div id="productname2"><nav role="heading"><!-- nav does not get nameFrom:contents, but heading does. -->product name</nav></div>
+<button class="ex" data-label="Delete product name" aria-label="Delete" aria-labelledby="test6 productname2" id="test6" data-note="self-referencial labelledby includes heading">x</button>
+
+<div class="ex" data-label="foo bar baz bop bap boom" aria-labelledby="a3 b3" id="test7" role="group" data-note="includes form elements">foo</div>
+<button id="a3"> foo <img src="#" alt="bar"> baz</button>
+<div id="b3">bop <input value="bap"> <input type="range" aria-valuetext="boom"></div>
 
-<div id="b">
-<button>test1</button> 
-<p tabindex=0>test2 <span aria-hidden="true">hidden</span> test3</p>
-</div>
 
 </div>
 
@@ -30,8 +47,11 @@ hello <a href="#">link</a> world
     description("This tests that if aria-labelledby is pointing to nodes with descendants, it returns all text.");
 
     if (window.accessibilityController) {
-          var group = accessibilityController.accessibleElementById("group");
-          debug("aria-labelledby description: " + group.description);
+          for (var k = 1; k < 8; k++) {
+              var test = accessibilityController.accessibleElementById("test" + k);
+              debug("test " + k + ": aria-labelledby description: " + test.description);
+              debug("test " + k + ": expected description: " + document.getElementById("test" + k).getAttribute("data-label") + "\n");
+          }
           document.getElementById("content").style.visibility = "hidden";
     }
 
diff --git a/LayoutTests/accessibility/aria-namefrom-author-expected.txt b/LayoutTests/accessibility/aria-namefrom-author-expected.txt
new file mode 100644 (file)
index 0000000..7d0716b
--- /dev/null
@@ -0,0 +1,10 @@
+This tests all the cases where nameFrom: author is used instead of nameFrom: contents. This means that if these elements are used in aria-labelledby they should not return their inner text. The button should retain its aria-label as the description.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Button description: AXDescription: text
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/accessibility/aria-namefrom-author.html b/LayoutTests/accessibility/aria-namefrom-author.html
new file mode 100644 (file)
index 0000000..0e29539
--- /dev/null
@@ -0,0 +1,78 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body id="body">
+
+<div id="content">
+
+<div id="test1" class="test" role="alert">alert</div>
+<div id="test2" class="test" role="alertdialog">alertdialog</div>
+<div id="test3" class="test" role="dialog">dialog</div>
+<div id="test4" class="test" role="log">log</div>
+<div id="test5" class="test" role="marquee">marquee</div>
+<div id="test6" class="test" role="status">status</div>
+<div id="test7" class="test" role="timer">timer</div>
+<div id="test8" class="test" role="combobox">combobox</div>
+<div id="test9" class="test" role="definition">definition</div>
+<div id="test10" class="test" role="document">document</div>
+<div id="test11" class="test" role="article">article</div>
+<div id="test12" class="test" role="math">math</div>
+<div id="test13" class="test" role="note">note</div>
+<div id="test14" class="test" role="region">region</div>
+<div id="test15" class="test" role="form">form</div>
+<div id="test16" class="test" role="grid">grid</div>
+<div id="test17" class="test" role="group">group</div>
+<div id="test18" class="test" role="img">img</div>
+<div id="test19" class="test" role="list">list</div>
+<div id="test20" class="test" role="listbox">listbox</div>
+<div id="test21" class="test" role="application">application</div>
+<div id="test22" class="test" role="banner">banner</div>
+<div id="test23" class="test" role="complementary">complementary</div>
+<div id="test24" class="test" role="contentinfo">contentinfo</div>
+<div id="test25" class="test" role="navigation">navigation</div>
+<div id="test26" class="test" role="main">main</div>
+<div id="test27" class="test" role="search">search</div>
+<div id="test28" class="test" role="menu">menu</div>
+<div id="test29" class="test" role="menubar">menubar</div>
+<div id="test30" class="test" role="progressbar">progressbar</div>
+<div id="test31" class="test" role="radiogroup">radiogroup</div>
+<div id="test32" class="test" role="scrollbar">scrollbar</div>
+<div id="test33" class="test" role="slider">slider</div>
+<div id="test34" class="test" role="spinbutton">spinbutton</div>
+<div id="test35" class="test" role="separator">separator</div>
+<div id="test36" class="test" role="tablist">tablist</div>
+<div id="test37" class="test" role="tabpanel">tabpanel</div>
+<div id="test38" class="test" role="textbox">textbox</div>
+<div id="test39" class="test" role="toolbar">toolbar</div>
+<div id="test40" class="test" role="treegrid">treegrid</div>
+<div id="test41" class="test" role="tree">tree</div>
+
+
+<div role="button" id="button" aria-label="text">button text</div>
+</div>
+
+<p id="description"></p>
+<div id="console"></div>
+
+<script>
+
+    description("This tests all the cases where nameFrom: author is used instead of nameFrom: contents. This means that if these elements are used in aria-labelledby they should not return their inner text. The button should retain its aria-label as the description.");
+
+    var labelledby = "";
+    for (var k = 1; k < 42; k++) {
+        labelledby += "test" + k + " ";
+    }
+    document.getElementById("button").setAttribute("aria-labelledby", labelledby);
+
+    if (window.accessibilityController) {
+        debug("Button description: " + accessibilityController.accessibleElementById("button").description);
+        document.getElementById("content").style.visibility = "hidden";
+    }
+
+</script>
+
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
index 038758f..a470c02 100644 (file)
@@ -1,3 +1,28 @@
+2014-09-29  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: in an aria-labelledby computation, do not traverse into elements whose nameFrom value does not include 'contents'
+        https://bugs.webkit.org/show_bug.cgi?id=136714
+
+        Reviewed by Darin Adler.
+
+        There are certain ARIA elements that tell us we should not query their children when determining the name of the object.
+        Those ones have the "nameFrom" property set to "author" instead of "contents." WebKit needs to honor that status.
+
+        Test: accessibility/aria-namefrom-author.html
+              Modified: accessibility/aria-labelledby-with-descendants.html
+
+        * accessibility/AccessibilityNodeObject.cpp:
+        (WebCore::shouldUseAccessiblityObjectInnerText):
+        (WebCore::shouldAddSpaceBeforeAppendingNextElement):
+        (WebCore::appendNameToStringBuilder):
+        (WebCore::AccessibilityNodeObject::textUnderElement):
+        (WebCore::accessibleNameForNode):
+        (WebCore::AccessibilityNodeObject::accessibilityDescriptionForElements):
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::accessibleNameDerivesFromContent):
+        (WebCore::initializeRoleMap):
+        * accessibility/AccessibilityObject.h:
+
 2014-09-29  Eric Carlson  <eric.carlson@apple.com>
 
         [iOS] Optimize media controls AirPlay discovery
index b721d37..1c3739c 100644 (file)
@@ -1609,6 +1609,11 @@ static bool shouldUseAccessiblityObjectInnerText(AccessibilityObject* obj, Acces
     // quite long. As a heuristic, skip links, controls, and elements that are usually
     // containers with lots of children.
 
+    // ARIA states that certain elements are not allowed to expose their children content for name calculation.
+    if (mode.childrenInclusion == AccessibilityTextUnderElementMode::TextUnderElementModeIncludeNameFromContentsChildren
+        && !obj->accessibleNameDerivesFromContent())
+        return false;
+    
     if (equalIgnoringCase(obj->getAttribute(aria_hiddenAttr), "true"))
         return false;
     
@@ -1628,7 +1633,7 @@ static bool shouldUseAccessiblityObjectInnerText(AccessibilityObject* obj, Acces
     return true;
 }
 
-static bool shouldAddSpaceBeforeAppendingNextElement(StringBuilder& builder, String& childText)
+static bool shouldAddSpaceBeforeAppendingNextElement(StringBuilder& builder, const String& childText)
 {
     if (!builder.length() || !childText.length())
         return false;
@@ -1636,6 +1641,13 @@ static bool shouldAddSpaceBeforeAppendingNextElement(StringBuilder& builder, Str
     // We don't need to add an additional space before or after a line break.
     return !(isHTMLLineBreak(childText[0]) || isHTMLLineBreak(builder[builder.length() - 1]));
 }
+    
+static void appendNameToStringBuilder(StringBuilder& builder, const String& text)
+{
+    if (shouldAddSpaceBeforeAppendingNextElement(builder, text))
+        builder.append(' ');
+    builder.append(text);
+}
 
 String AccessibilityNodeObject::textUnderElement(AccessibilityTextUnderElementMode mode) const
 {
@@ -1651,6 +1663,13 @@ String AccessibilityNodeObject::textUnderElement(AccessibilityTextUnderElementMo
 
     StringBuilder builder;
     for (AccessibilityObject* child = firstChild(); child; child = child->nextSibling()) {
+        
+        bool shouldDeriveNameFromAuthor = (mode.childrenInclusion == AccessibilityTextUnderElementMode::TextUnderElementModeIncludeNameFromContentsChildren && !child->accessibleNameDerivesFromContent());
+        if (shouldDeriveNameFromAuthor) {
+            appendNameToStringBuilder(builder, accessibleNameForNode(child->node()));
+            continue;
+        }
+        
         if (!shouldUseAccessiblityObjectInnerText(child, mode))
             continue;
 
@@ -1658,19 +1677,14 @@ String AccessibilityNodeObject::textUnderElement(AccessibilityTextUnderElementMo
             Vector<AccessibilityText> textOrder;
             toAccessibilityNodeObject(child)->alternativeText(textOrder);
             if (textOrder.size() > 0 && textOrder[0].text.length()) {
-                if (shouldAddSpaceBeforeAppendingNextElement(builder, textOrder[0].text))
-                    builder.append(' ');
-                builder.append(textOrder[0].text);
+                appendNameToStringBuilder(builder, textOrder[0].text);
                 continue;
             }
         }
-
+        
         String childText = child->textUnderElement(mode);
-        if (childText.length()) {
-            if (shouldAddSpaceBeforeAppendingNextElement(builder, childText))
-                builder.append(' ');
-            builder.append(childText);
-        }
+        if (childText.length())
+            appendNameToStringBuilder(builder, childText);
     }
 
     return builder.toString().stripWhiteSpace().simplifyWhiteSpace(isHTMLSpaceButNotLineBreak);
@@ -1839,19 +1853,26 @@ static String accessibleNameForNode(Node* node)
     const AtomicString& alt = element->fastGetAttribute(altAttr);
     if (!alt.isEmpty())
         return alt;
+
+    // If the node can be turned into an AX object, we can use standard name computation rules.
+    // If however, the node cannot (because there's no renderer e.g.) fallback to using the basic text underneath.
+    AccessibilityObject* axObject = node->document().axObjectCache()->getOrCreate(node);
+    if (axObject) {
+        String valueDescription = axObject->valueDescription();
+        if (!valueDescription.isEmpty())
+            return valueDescription;
+    }
     
     if (is<HTMLInputElement>(node))
         return downcast<HTMLInputElement>(*node).value();
     
-    // If the node can be turned into an AX object, we can use standard name computation rules.
-    // If however, the node cannot (because there's no renderer e.g.) fallback to using the basic text underneath.
-    AccessibilityObject* axObject = node->document().axObjectCache()->getOrCreate(node);
     String text;
-    if (axObject)
-        text = axObject->textUnderElement(AccessibilityTextUnderElementMode(AccessibilityTextUnderElementMode::TextUnderElementModeSkipIgnoredChildren, true));
-    else
+    if (axObject) {
+        if (axObject->accessibleNameDerivesFromContent())
+            text = axObject->textUnderElement(AccessibilityTextUnderElementMode(AccessibilityTextUnderElementMode::TextUnderElementModeIncludeNameFromContentsChildren, true));
+    } else
         text = element->innerText();
-    
+
     if (!text.isEmpty())
         return text;
     
@@ -1866,12 +1887,8 @@ String AccessibilityNodeObject::accessibilityDescriptionForElements(Vector<Eleme
 {
     StringBuilder builder;
     unsigned size = elements.size();
-    for (unsigned i = 0; i < size; ++i) {
-        if (i)
-            builder.append(' ');
-        
-        builder.append(accessibleNameForNode(elements[i]));
-    }
+    for (unsigned i = 0; i < size; ++i)
+        appendNameToStringBuilder(builder, accessibleNameForNode(elements[i]));
     return builder.toString();
 }
 
index 689f1d6..2cf40c3 100644 (file)
@@ -287,6 +287,70 @@ bool AccessibilityObject::accessibilityObjectContainsText(String* text) const
         || accessibilityDescription().contains(*text, false)
         || stringValue().contains(*text, false);
 }
+
+// ARIA marks elements as having their accessible name derive from either their contents, or their author provide name.
+bool AccessibilityObject::accessibleNameDerivesFromContent() const
+{
+    // First check for objects specifically identified by ARIA.
+    switch (ariaRoleAttribute()) {
+    case ApplicationAlertRole:
+    case ApplicationAlertDialogRole:
+    case ApplicationDialogRole:
+    case ApplicationLogRole:
+    case ApplicationMarqueeRole:
+    case ApplicationStatusRole:
+    case ApplicationTimerRole:
+    case ComboBoxRole:
+    case DefinitionRole:
+    case DocumentRole:
+    case DocumentArticleRole:
+    case DocumentMathRole:
+    case DocumentNoteRole:
+    case DocumentRegionRole:
+    case FormRole:
+    case GridRole:
+    case GroupRole:
+    case ImageRole:
+    case ListRole:
+    case ListBoxRole:
+    case LandmarkApplicationRole:
+    case LandmarkBannerRole:
+    case LandmarkComplementaryRole:
+    case LandmarkContentInfoRole:
+    case LandmarkNavigationRole:
+    case LandmarkMainRole:
+    case LandmarkSearchRole:
+    case MenuRole:
+    case MenuBarRole:
+    case ProgressIndicatorRole:
+    case RadioGroupRole:
+    case ScrollBarRole:
+    case SliderRole:
+    case SpinButtonRole:
+    case SplitterRole:
+    case TableRole:
+    case TabListRole:
+    case TabPanelRole:
+    case TextAreaRole:
+    case TextFieldRole:
+    case ToolbarRole:
+    case TreeGridRole:
+    case TreeRole:
+        return false;
+    default:
+        break;
+    }
+    
+    // Now check for generically derived elements now that we know the element does not match a specific ARIA role.
+    switch (roleValue()) {
+    case SliderRole:
+        return false;
+    default:
+        break;
+    }
+    
+    return true;
+}
     
 String AccessibilityObject::computedLabel()
 {
@@ -1795,6 +1859,7 @@ static void initializeRoleMap()
         { "combobox", ComboBoxRole },
         { "definition", DefinitionRole },
         { "document", DocumentRole },
+        { "form", FormRole },
         { "rowheader", RowHeaderRole },
         { "group", GroupRole },
         { "heading", HeadingRole },
index 0af09dd..b111901 100644 (file)
@@ -248,6 +248,7 @@ struct AccessibilityTextUnderElementMode {
     enum ChildrenInclusion {
         TextUnderElementModeSkipIgnoredChildren,
         TextUnderElementModeIncludeAllChildren,
+        TextUnderElementModeIncludeNameFromContentsChildren, // This corresponds to ARIA concept: nameFrom
     };
     
     ChildrenInclusion childrenInclusion;
@@ -667,6 +668,7 @@ public:
     virtual String ariaLabeledByAttribute() const { return String(); }
     virtual String ariaDescribedByAttribute() const { return String(); }
     const AtomicString& placeholderValue() const;
+    bool accessibleNameDerivesFromContent() const;
     
     // Abbreviations
     virtual String expandedTextValue() const { return String(); }