Merge r221918 - AX: On generic container elements, WebKit should dist…

…inguish between tooltip (e.g. @title) and label (e.g. @aria-label) attributes https://bugs.webkit.org/show_bug.cgi?id=170475 <rdar://problem/31439222> Reviewed by Joanmarie Diggs. Source/WebCore: Test: accessibility/title-tag-on-unimportant-elements-is-help-text.html * accessibility/AccessibilityNodeObject.cpp: (WebCore::AccessibilityNodeObject::helpText): (WebCore::AccessibilityNodeObject::accessibilityDescription): (WebCore::AccessibilityNodeObject::roleIgnoresTitle): * accessibility/AccessibilityNodeObject.h: LayoutTests: * accessibility/title-tag-on-unimportant-elements-is-help-text-expected.txt: Added. * accessibility/title-tag-on-unimportant-elements-is-help-text.html: Added.
WebKit · Oct 16, 2017 · 2dcc37b · 2dcc37b
1 parent 872fe8e
commit 2dcc37b
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 3 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2017-09-12  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: On generic container elements, WebKit should distinguish between tooltip (e.g. @title) and label (e.g. @aria-label) attributes
+        https://bugs.webkit.org/show_bug.cgi?id=170475
+        <rdar://problem/31439222>
+
+        Reviewed by Joanmarie Diggs.
+
+        * accessibility/title-tag-on-unimportant-elements-is-help-text-expected.txt: Added.
+        * accessibility/title-tag-on-unimportant-elements-is-help-text.html: Added.
+
 2017-09-11  Joanmarie Diggs  <jdiggs@igalia.com>
 
         AX: [ATK] aria-autocomplete not exposed on comboboxes

diff --git a/LayoutTests/accessibility/title-tag-on-unimportant-elements-is-help-text-expected.txt b/LayoutTests/accessibility/title-tag-on-unimportant-elements-is-help-text-expected.txt
@@ -0,0 +1,18 @@
+a
+a
+
+This tests that if a semantically unimportant element has title text, that text is exposed as help text rather than descriptive text.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS platformValueForW3CDescription(accessibilityController.accessibleElementById('div1')) is 'test1'
+PASS platformValueForW3CName(accessibilityController.accessibleElementById('div1')) is ''
+PASS platformValueForW3CDescription(accessibilityController.accessibleElementById('div2')) is ''
+PASS platformValueForW3CName(accessibilityController.accessibleElementById('div2')) is 'test2'
+PASS platformValueForW3CDescription(accessibilityController.accessibleElementById('button1')) is ''
+PASS platformValueForW3CName(accessibilityController.accessibleElementById('button1')) is 'test5'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/accessibility/title-tag-on-unimportant-elements-is-help-text.html b/LayoutTests/accessibility/title-tag-on-unimportant-elements-is-help-text.html
@@ -0,0 +1,40 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+<script src="../resources/accessibility-helper.js"></script>
+</head>
+<body id="body">
+
+<div id="div1" title="test1">a</div>
+<div id="div2" role="group" title="test2">a</div>
+
+<button id="button1" title="test5"></button>
+
+<p id="description"></p>
+<div id="console"></div>
+
+<script>
+
+    description("This tests that if a semantically unimportant element has title text, that text is exposed as help text rather than descriptive text.");
+
+    if (window.accessibilityController) {
+
+          // div1 is unimportant, so description should be blank.
+          shouldBe("platformValueForW3CDescription(accessibilityController.accessibleElementById('div1'))", "'test1'");
+          shouldBe("platformValueForW3CName(accessibilityController.accessibleElementById('div1'))", "''");
+
+          // div2 has an author applied role, so title tag should be used for description.
+          shouldBe("platformValueForW3CDescription(accessibilityController.accessibleElementById('div2'))", "''");
+          shouldBe("platformValueForW3CName(accessibilityController.accessibleElementById('div2'))", "'test2'");
+
+          // button is a meaningful element, so it should have a description.
+          shouldBe("platformValueForW3CDescription(accessibilityController.accessibleElementById('button1'))", "''");
+          shouldBe("platformValueForW3CName(accessibilityController.accessibleElementById('button1'))", "'test5'");
+    }
+
+</script>
+
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,19 @@
+2017-09-12  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: On generic container elements, WebKit should distinguish between tooltip (e.g. @title) and label (e.g. @aria-label) attributes
+        https://bugs.webkit.org/show_bug.cgi?id=170475
+        <rdar://problem/31439222>
+
+        Reviewed by Joanmarie Diggs.
+
+        Test: accessibility/title-tag-on-unimportant-elements-is-help-text.html
+
+        * accessibility/AccessibilityNodeObject.cpp:
+        (WebCore::AccessibilityNodeObject::helpText):
+        (WebCore::AccessibilityNodeObject::accessibilityDescription):
+        (WebCore::AccessibilityNodeObject::roleIgnoresTitle):
+        * accessibility/AccessibilityNodeObject.h:
+
 2017-09-12  Antti Koivisto  <antti@apple.com>
 
         Remove RenderElement::isCSSAnimating boolean

diff --git a/Source/WebCore/accessibility/AccessibilityNodeObject.cpp b/Source/WebCore/accessibility/AccessibilityNodeObject.cpp
@@ -1452,7 +1452,7 @@ void AccessibilityNodeObject::helpText(Vector<AccessibilityText>& textOrder) con
     // type to HelpText.
     const AtomicString& title = getAttribute(titleAttr);
     if (!title.isEmpty()) {
-        if (!isMeter())
+        if (!isMeter() && !roleIgnoresTitle())
             textOrder.append(AccessibilityText(title, TitleTagText));
         else
             textOrder.append(AccessibilityText(title, HelpText));
@@ -1557,12 +1557,29 @@ String AccessibilityNodeObject::accessibilityDescription() const
     // Both are used to generate what a screen reader speaks.                                                           
     // If this point is reached (i.e. there's no accessibilityDescription) and there's no title(), we should fallback to using the title attribute.
     // The title attribute is normally used as help text (because it is a tooltip), but if there is nothing else available, this should be used (according to ARIA).
-    if (title().isEmpty())
+    // https://bugs.webkit.org/show_bug.cgi?id=170475: An exception is when the element is semantically unimportant. In those cases, title text should remain as help text.
+    if (title().isEmpty() && !roleIgnoresTitle())
         return getAttribute(titleAttr);
 
     return String();
 }
 
+// Returns whether the role was not intended to play a semantically meaningful part of the
+// accessibility hierarchy. This applies to generic groups like <div>'s with no role value set.
+bool AccessibilityNodeObject::roleIgnoresTitle() const
+{
+    if (ariaRoleAttribute() != UnknownRole)
+        return false;
+
+    switch (roleValue()) {
+    case DivRole:
+    case UnknownRole:
+        return true;
+    default:
+        return false;
+    }
+}
+
 String AccessibilityNodeObject::helpText() const
 {
     Node* node = this->node();

diff --git a/Source/WebCore/accessibility/AccessibilityNodeObject.h b/Source/WebCore/accessibility/AccessibilityNodeObject.h
@@ -189,7 +189,8 @@ class AccessibilityNodeObject : public AccessibilityObject {
     void ariaLabeledByText(Vector<AccessibilityText>&) const;
     bool computeAccessibilityIsIgnored() const override;
     bool usesAltTagForTextComputation() const;
-
+    bool roleIgnoresTitle() const;
+
     Node* m_node;
 };