Title: [126970] trunk
Revision
126970
Author
dmazz...@google.com
Date
2012-08-29 00:56:45 -0700 (Wed, 29 Aug 2012)

Log Message

AX: Focusable elements without a role should not be ignored
https://bugs.webkit.org/show_bug.cgi?id=94302

Reviewed by Chris Fleizach.

Source/WebCore: 

Changes the accessibility logic so that a generic element that's focusable is
not ignored for accessibility, and returns its inner text as its title. That way
if you Tab to the element, a reasonable accessibility notification is generated.

One exception is the body element, because focusing the body is equivalent to
blurring the current focused element and does not result in a "focus" accessibility
notification.

Also fixes logic that determined if an element was contentEditable by making
sure it catches the case with no attribute value (e.g. <div contentEditable>),
which also implies contentEditable=true according to the spec.

Test: accessibility/focusable-div.html

* accessibility/AccessibilityRenderObject.cpp:
(WebCore):
(WebCore::nodeHasContentEditableAttributeSet):
(WebCore::AccessibilityRenderObject::title):
(WebCore::AccessibilityRenderObject::accessibilityIsIgnored):

LayoutTests: 

Adds a new test to make sure that a generic focusable element (like a div with tabindex=0)
can get focus and return an appropriate title, just like a form control or element with
an ARIA role.

Modifies three existing tests that were previously assuming that a focusable node
with no role would be ignored for accessibility ("accessibilityIsIgnored").

* accessibility/editable-webarea-context-menu-point.html:
* accessibility/focusable-div-expected.txt: Added.
* accessibility/focusable-div.html: Added.
* accessibility/table-detection.html:
* platform/mac/accessibility/listbox-hit-test.html:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (126969 => 126970)


--- trunk/LayoutTests/ChangeLog	2012-08-29 07:48:28 UTC (rev 126969)
+++ trunk/LayoutTests/ChangeLog	2012-08-29 07:56:45 UTC (rev 126970)
@@ -1,3 +1,23 @@
+2012-08-28  Dominic Mazzoni  <dmazz...@google.com>
+
+        AX: Focusable elements without a role should not be ignored
+        https://bugs.webkit.org/show_bug.cgi?id=94302
+
+        Reviewed by Chris Fleizach.
+
+        Adds a new test to make sure that a generic focusable element (like a div with tabindex=0)
+        can get focus and return an appropriate title, just like a form control or element with
+        an ARIA role.
+
+        Modifies three existing tests that were previously assuming that a focusable node
+        with no role would be ignored for accessibility ("accessibilityIsIgnored").
+
+        * accessibility/editable-webarea-context-menu-point.html:
+        * accessibility/focusable-div-expected.txt: Added.
+        * accessibility/focusable-div.html: Added.
+        * accessibility/table-detection.html:
+        * platform/mac/accessibility/listbox-hit-test.html:
+
 2012-08-28  Joone Hur  <joone....@intel.com>
 
         [EFL] Unreviewed gardening on the 64bit build bot.

Modified: trunk/LayoutTests/accessibility/editable-webarea-context-menu-point.html (126969 => 126970)


--- trunk/LayoutTests/accessibility/editable-webarea-context-menu-point.html	2012-08-29 07:48:28 UTC (rev 126969)
+++ trunk/LayoutTests/accessibility/editable-webarea-context-menu-point.html	2012-08-29 07:56:45 UTC (rev 126970)
@@ -19,6 +19,7 @@
 
           var body = document.getElementById("body");
           body.focus();
+          var axWebArea = accessibilityController.focusedElement.parentElement();
 
           x = body.offsetLeft + 10;
           y = body.offsetTop + 10;
@@ -29,7 +30,7 @@
           eventSender.mouseDown();
           eventSender.mouseUp();
 
-          var clickPointX1 = accessibilityController.focusedElement.clickPointX;
+          var clickPointX1 = axWebArea.clickPointX;
 
           x = body.offsetLeft + 100 + 10;
           y = body.offsetTop + 10;
@@ -40,7 +41,7 @@
           eventSender.mouseDown();
           eventSender.mouseUp();
 
-          var clickPointX2 = accessibilityController.focusedElement.clickPointX;
+          var clickPointX2 = axWebArea.clickPointX;
           var succeeded = clickPointX2 != clickPointX1;
           shouldBe("succeeded", "true");
 

Added: trunk/LayoutTests/accessibility/focusable-div-expected.txt (0 => 126970)


--- trunk/LayoutTests/accessibility/focusable-div-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/accessibility/focusable-div-expected.txt	2012-08-29 07:56:45 UTC (rev 126970)
@@ -0,0 +1,20 @@
+A
+B
+C
+This test makes sure that a generic focusable div can get accessibility focus.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.activeElement == link is true
+PASS lastChar(axLink.title) is "A"
+PASS document.activeElement == div is true
+PASS lastChar(axDiv.title) is "B"
+PASS document.activeElement == div2 is true
+PASS lastChar(axDiv2.title) is "C"
+PASS document.activeElement == div3 is true
+PASS lastChar(axDiv3.description) is "D"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Property changes on: trunk/LayoutTests/accessibility/focusable-div-expected.txt
___________________________________________________________________

Added: svn:eol-style

Added: trunk/LayoutTests/accessibility/focusable-div.html (0 => 126970)


--- trunk/LayoutTests/accessibility/focusable-div.html	                        (rev 0)
+++ trunk/LayoutTests/accessibility/focusable-div.html	2012-08-29 07:56:45 UTC (rev 126970)
@@ -0,0 +1,51 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<script src=""
+
+<a id="link" href=""
+<div id="div" tabindex="0">B</div>
+<div id="div2" tabindex="0"><div></div>C</div>
+<div id="div3" tabindex="0" aria-label="D"></div>
+
+<div id="console"></div>
+<script>
+description("This test makes sure that a generic focusable div can get accessibility focus.");
+
+if (window.testRunner && window.accessibilityController) {
+    window.testRunner.dumpAsText();
+
+    function lastChar(str) {
+        return str.substr(str.length - 1);
+    }
+
+    var link = document.getElementById('link');
+    link.focus();
+    shouldBe("document.activeElement == link", "true");
+    window.axLink = accessibilityController.focusedElement;
+    shouldBe("lastChar(axLink.title)", "\"A\"");
+
+    var div = document.getElementById('div');
+    div.focus();
+    shouldBe("document.activeElement == div", "true");
+    window.axDiv = accessibilityController.focusedElement;
+    shouldBe("lastChar(axDiv.title)", "\"B\"");
+
+    var div2 = document.getElementById('div2');
+    div2.focus();
+    shouldBe("document.activeElement == div2", "true");
+    window.axDiv2 = accessibilityController.focusedElement;
+    shouldBe("lastChar(axDiv2.title)", "\"C\"");
+
+    var div3 = document.getElementById('div3');
+    div3.focus();
+    shouldBe("document.activeElement == div3", "true");
+    window.axDiv3 = accessibilityController.focusedElement;
+    shouldBe("lastChar(axDiv3.description)", "\"D\"");
+}
+
+</script>
+
+<script src=""
+</body>
+</html>
Property changes on: trunk/LayoutTests/accessibility/focusable-div.html
___________________________________________________________________

Added: svn:mime-type

Added: svn:eol-style

Modified: trunk/LayoutTests/accessibility/table-detection.html (126969 => 126970)


--- trunk/LayoutTests/accessibility/table-detection.html	2012-08-29 07:48:28 UTC (rev 126969)
+++ trunk/LayoutTests/accessibility/table-detection.html	2012-08-29 07:56:45 UTC (rev 126970)
@@ -99,12 +99,11 @@
     <table class="nmTB" cellpadding="0" cellspacing="0"><tr><td class="nmIBD" id="nmb" name="nmb" nm_sn="3032552" nm_suf="" CM_sf="Ex" CM="NewsMenuL1" pn="newsmenu" ct="nm0" cn="Politics"><a class="nmLBD" href="" class="nmIB" id="nmb" name="nmb" nm_sn="18970411" nm_suf="" CM_sf="Ex" CM="NewsMenuL1" pn="newsmenu" ct="nxf" cn="Decision '08"><a class="nmLB" href="" '08</a></td></tr><tr><td class="nmIB" id="nmb" name="nmb" nm_sn="18296896" nm_suf="" CM_sf="Ex" CM="NewsMenuL1" pn="newsmenu" ct="nxf" cn="The debates"><a class="nmLB" href="" debates</a></td></tr><tr><td class="nmIB" id="nmb" name="nmb" nm_sn="21491043" nm_suf="" CM_sf="Ex" CM="NewsMenuL1" pn="newsmenu" ct="nxf" cn="The White House"><a class="nmLB" href="" White House</a></td></tr><tr><td class="nmIB" id="nmb" name="nmb" nm_sn="21491571" nm_suf="" CM_sf="Ex" CM="NewsMenuL1" pn="newsmenu" ct="nxf" cn="Capitol Hill"><a class="nmLB" href="" Hill</a></td></tr><tr><td class="nmIB" id="nmb" name="nmb" nm_sn="14016004" nm_suf="" CM_sf="Ex" CM="NewsMenuL1" pn="newsmenu" ct="nxf" cn="National Journal"><a class="nmLB" href="" Journal</a></td></tr><tr><td class="nmIB" id="nmb" name="nmb" nm_sn="19748467" nm_suf="" CM_sf="Ex" CM="NewsMenuL1" pn="newsmenu" ct="nxf" cn="New York Times"><a class="nmLB" href="" York Times</a></td></tr></table>
 
     // this should be a table because it's editable
-    <div contenteditable>
-      <table style='border: 1px solid black'>
-        <tr><td >asdf</td><td>asdf</td></tr>
-      </table>
-    </div>
 
+    <table style='border: 1px solid black' contenteditable>
+      <tr><td >asdf</td><td>asdf</td></tr>
+    </table>
+
     <div id="result"></div>
     
     <script>

Modified: trunk/LayoutTests/platform/mac/accessibility/listbox-hit-test.html (126969 => 126970)


--- trunk/LayoutTests/platform/mac/accessibility/listbox-hit-test.html	2012-08-29 07:48:28 UTC (rev 126969)
+++ trunk/LayoutTests/platform/mac/accessibility/listbox-hit-test.html	2012-08-29 07:56:45 UTC (rev 126970)
@@ -5,7 +5,7 @@
 </head>
 <body id="body">
 
-<div tabindex=0 id="region">
+<div tabindex=0 id="region" title="region">
 <select size=20>
 <option>test option that spans the width of the cell. test option that spans the width of the cell</option>
 <option>test option that spans the width of the cell. test option that spans the width of the cell</option>

Modified: trunk/Source/WebCore/ChangeLog (126969 => 126970)


--- trunk/Source/WebCore/ChangeLog	2012-08-29 07:48:28 UTC (rev 126969)
+++ trunk/Source/WebCore/ChangeLog	2012-08-29 07:56:45 UTC (rev 126970)
@@ -1,3 +1,30 @@
+2012-08-28  Dominic Mazzoni  <dmazz...@google.com>
+
+        AX: Focusable elements without a role should not be ignored
+        https://bugs.webkit.org/show_bug.cgi?id=94302
+
+        Reviewed by Chris Fleizach.
+
+        Changes the accessibility logic so that a generic element that's focusable is
+        not ignored for accessibility, and returns its inner text as its title. That way
+        if you Tab to the element, a reasonable accessibility notification is generated.
+
+        One exception is the body element, because focusing the body is equivalent to
+        blurring the current focused element and does not result in a "focus" accessibility
+        notification.
+
+        Also fixes logic that determined if an element was contentEditable by making
+        sure it catches the case with no attribute value (e.g. <div contentEditable>),
+        which also implies contentEditable=true according to the spec.
+
+        Test: accessibility/focusable-div.html
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore):
+        (WebCore::nodeHasContentEditableAttributeSet):
+        (WebCore::AccessibilityRenderObject::title):
+        (WebCore::AccessibilityRenderObject::accessibilityIsIgnored):
+
 2012-08-29  Adam Barth  <aba...@webkit.org>
 
         Deploy ASCIILiteral hotness throughout WebCore

Modified: trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp (126969 => 126970)


--- trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp	2012-08-29 07:48:28 UTC (rev 126969)
+++ trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp	2012-08-29 07:56:45 UTC (rev 126970)
@@ -384,4 +384,15 @@
     return role;
 }   
 
+// If you call node->rendererIsEditable() since that will return true if an ancestor is editable.
+// This only returns true if this is the element that actually has the contentEditable attribute set.
+bool AccessibilityNodeObject::hasContentEditableAttributeSet() const
+{
+    if (!hasAttribute(contenteditableAttr))
+        return false;
+    const AtomicString& contentEditableValue = getAttribute(contenteditableAttr);
+    // Both "true" (case-insensitive) and the empty string count as true.
+    return contentEditableValue.isEmpty() || equalIgnoringCase(contentEditableValue, "true");
+}
+
 } // namespace WebCore

Modified: trunk/Source/WebCore/accessibility/AccessibilityNodeObject.h (126969 => 126970)


--- trunk/Source/WebCore/accessibility/AccessibilityNodeObject.h	2012-08-29 07:48:28 UTC (rev 126969)
+++ trunk/Source/WebCore/accessibility/AccessibilityNodeObject.h	2012-08-29 07:56:45 UTC (rev 126970)
@@ -98,6 +98,7 @@
     AccessibilityRole ariaRoleAttribute() const;
     AccessibilityRole determineAriaRoleAttribute() const;
     AccessibilityRole remapAriaRoleDueToParent(AccessibilityRole) const;
+    bool hasContentEditableAttributeSet() const;
 
 private:
     Node* m_node;

Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.cpp (126969 => 126970)


--- trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2012-08-29 07:48:28 UTC (rev 126969)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2012-08-29 07:56:45 UTC (rev 126970)
@@ -1294,6 +1294,19 @@
     return ariaInvalid;
 }
  
+bool AccessibilityObject::hasAttribute(const QualifiedName& attribute) const
+{
+    Node* elementNode = node();
+    if (!elementNode)
+        return false;
+    
+    if (!elementNode->isElementNode())
+        return false;
+    
+    Element* element = static_cast<Element*>(elementNode);
+    return element->fastHasAttribute(attribute);
+}
+    
 const AtomicString& AccessibilityObject::getAttribute(const QualifiedName& attribute) const
 {
     Node* elementNode = node();

Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.h (126969 => 126970)


--- trunk/Source/WebCore/accessibility/AccessibilityObject.h	2012-08-29 07:48:28 UTC (rev 126969)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.h	2012-08-29 07:56:45 UTC (rev 126970)
@@ -585,6 +585,7 @@
     bool isAncestorOfObject(const AccessibilityObject*) const;
     
     static AccessibilityRole ariaRoleToWebCoreRole(const String&);
+    bool hasAttribute(const QualifiedName&) const;
     const AtomicString& getAttribute(const QualifiedName&) const;
 
     virtual VisiblePositionRange visiblePositionRange() const { return VisiblePositionRange(); }

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (126969 => 126970)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2012-08-29 07:48:28 UTC (rev 126969)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2012-08-29 07:56:45 UTC (rev 126970)
@@ -1401,7 +1401,12 @@
     
     if (isHeading() || isLink())
         return textUnderElement();
-    
+
+    // If it's focusable but it's not content editable or a known control type, then it will appear to
+    // the user as a single atomic object, so we should use its text as the default title.
+    if (isGenericFocusableElement())
+        return textUnderElement();
+
     return String();
 }
 
@@ -1938,12 +1943,8 @@
     // Anything that is content editable should not be ignored.
     // However, one cannot just call node->rendererIsEditable() since that will ask if its parents
     // are also editable. Only the top level content editable region should be exposed.
-    if (node && node->isElementNode()) {
-        Element* element = static_cast<Element*>(node);
-        const AtomicString& contentEditable = element->getAttribute(contenteditableAttr);
-        if (equalIgnoringCase(contentEditable, "true"))
-            return false;
-    }
+    if (hasContentEditableAttributeSet())
+        return false;
     
     // List items play an important role in defining the structure of lists. They should not be ignored.
     if (roleValue() == ListItemRole)
@@ -1953,7 +1954,7 @@
     if (supportsARIAAttributes())
         return false;
     
-    if (m_renderer->isBlockFlow() && m_renderer->childrenInline())
+    if (m_renderer->isBlockFlow() && m_renderer->childrenInline() && !canSetFocusAttribute())
         return !toRenderBlock(m_renderer)->firstLineBox() && !mouseButtonListener();
     
     // ignore images seemingly used as spacers
@@ -3118,6 +3119,31 @@
     }
     return false;
 }
+
+bool AccessibilityRenderObject::isGenericFocusableElement() const
+{
+    if (!canSetFocusAttribute())
+        return false;
+
+    // If it's a control, it's not generic.
+    if (isControl())
+        return false;
+
+    // If the content editable attribute is set on this element, that's the reason
+    // it's focusable, and existing logic should handle this case already - so it's not a
+    // generic focusable element.
+    if (hasContentEditableAttributeSet())
+        return false;
+
+    // The web area and body element are both focusable, but existing logic handles these
+    // cases already, so we don't need to include them here.
+    if (roleValue() == WebAreaRole)
+        return false;
+    if (node() && node()->hasTagName(bodyTag))
+        return false;
+
+    return true;
+}
     
 AccessibilityRole AccessibilityRenderObject::determineAccessibilityRole()
 {

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h (126969 => 126970)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h	2012-08-29 07:48:28 UTC (rev 126969)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h	2012-08-29 07:56:45 UTC (rev 126970)
@@ -300,6 +300,8 @@
     bool renderObjectIsObservable(RenderObject*) const;
     RenderObject* renderParentObject() const;
     bool isDescendantOfElementType(const QualifiedName& tagName) const;
+    // This returns true if it's focusable but it's not content editable and it's not a control or ARIA control.
+    bool isGenericFocusableElement() const;
 
     void addTextFieldChildren();
     void addImageMapChildren();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to