AX: role="none" (or presentation) does not work on iframes
authorcfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 3 Jul 2017 17:18:34 +0000 (17:18 +0000)
committercfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 3 Jul 2017 17:18:34 +0000 (17:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173930
<rdar://problem/33034347>

Reviewed by Ryosuke Niwa.

Source/WebCore:

Support setting a presentational role on an iframe so that the AXWebArea disappears from the hierarchy.
Accomplish this by adding children for attachment and scroll view elements the way other children are added.
That is, only add the non-ignored children directly (which means move the addChild logic into AccessibilityObject.)

Test: accessibility/presentation-role-iframe.html

* accessibility/AccessibilityNodeObject.cpp:
(WebCore::AccessibilityNodeObject::AccessibilityNodeObject):
(WebCore::AccessibilityNodeObject::insertChild): Deleted.
(WebCore::AccessibilityNodeObject::addChild): Deleted.
* accessibility/AccessibilityNodeObject.h:
* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::AccessibilityObject):
(WebCore::AccessibilityObject::insertChild):
(WebCore::AccessibilityObject::addChild):
(WebCore::nodeHasPresentationRole):
* accessibility/AccessibilityObject.h:
(WebCore::AccessibilityObject::addChild): Deleted.
(WebCore::AccessibilityObject::insertChild): Deleted.
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::webAreaIsPresentational):
(WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored):
(WebCore::AccessibilityRenderObject::addAttachmentChildren):
* accessibility/AccessibilityScrollView.cpp:
(WebCore::AccessibilityScrollView::addChildren):

LayoutTests:

* accessibility/presentation-role-iframe-expected.txt: Added.
* accessibility/presentation-role-iframe.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/accessibility/presentation-role-iframe-expected.txt [new file with mode: 0644]
LayoutTests/accessibility/presentation-role-iframe.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityNodeObject.cpp
Source/WebCore/accessibility/AccessibilityNodeObject.h
Source/WebCore/accessibility/AccessibilityObject.cpp
Source/WebCore/accessibility/AccessibilityObject.h
Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Source/WebCore/accessibility/AccessibilityScrollView.cpp

index a2d50ca..41908a1 100644 (file)
@@ -1,3 +1,14 @@
+2017-07-03  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: role="none" (or presentation) does not work on iframes
+        https://bugs.webkit.org/show_bug.cgi?id=173930
+        <rdar://problem/33034347>
+
+        Reviewed by Ryosuke Niwa.
+
+        * accessibility/presentation-role-iframe-expected.txt: Added.
+        * accessibility/presentation-role-iframe.html: Added.
+
 2017-07-03  Alex Christensen  <achristensen@webkit.org>
 
         Rebase test after r219024
diff --git a/LayoutTests/accessibility/presentation-role-iframe-expected.txt b/LayoutTests/accessibility/presentation-role-iframe-expected.txt
new file mode 100644 (file)
index 0000000..3cc958c
--- /dev/null
@@ -0,0 +1,18 @@
+
+
+
+This tests that setting role=presentation on an iframe/object hides the AXWebArea.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+1. Content child: AXRole: AXGroup
+1. Content grand child: AXRole: AXButton
+2. Content child: AXRole: AXScrollArea
+2. Content grand child: AXRole: AXWebArea
+3. Content child: AXRole: AXGroup
+3. Content grand child: AXRole: AXButton
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/accessibility/presentation-role-iframe.html b/LayoutTests/accessibility/presentation-role-iframe.html
new file mode 100644 (file)
index 0000000..7a28466
--- /dev/null
@@ -0,0 +1,57 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test.js"></script>
+</head>
+
+<body id="body">
+
+<div id="content1" role="group">
+<iframe onload="runTest(1)" id="iframe1" role='presentation' src="data:text/html,<body><button>Click me</button></body>"></iframe>
+</div>
+
+<div id="content2" role="group">
+<iframe onload="runTest(2)" id="iframe2" src="data:text/html,<body><button>Click me</button></body>"></iframe>
+</div>
+
+<div id="content3" role="group">
+<object type="text/html" onload="runTest(3)" role="none" id="object1" data="data:text/html,<body><div role='group'><button>Click me</button></div></body>"></object>
+</div>
+
+<p id="description"></p>
+<div id="console"></div>
+
+<script>
+
+description("This tests that setting role=presentation on an iframe/object hides the AXWebArea.");
+
+jsTestIsAsync = true;
+var test1Ran = false;
+var test2Ran = false;
+var test3Ran = false;
+
+// Run the test for a iframe that has presentation role and one that does not.
+// The children should be different.
+function runTest(testNumber)
+{
+  if (window.accessibilityController) {
+    var content = accessibilityController.accessibleElementById("content" + testNumber);
+    debug(testNumber + ". Content child: " + content.childAtIndex(0).role);
+    debug(testNumber + ". Content grand child: " + content.childAtIndex(0).childAtIndex(0).role);
+    if (testNumber == 1) {
+         test1Ran = true;
+    } else if (testNumber == 2) {
+         test2Ran = true;
+    } else if (testNumber == 3) {
+         test3Ran = true;
+    }
+
+    if (test3Ran && test2Ran && test1Ran) {
+        finishJSTest();
+    }
+  }
+}
+</script>
+
+</body>
+</html>
index 5cc6c75..992faeb 100644 (file)
@@ -1,3 +1,37 @@
+2017-07-03  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: role="none" (or presentation) does not work on iframes
+        https://bugs.webkit.org/show_bug.cgi?id=173930
+        <rdar://problem/33034347>
+
+        Reviewed by Ryosuke Niwa.
+
+        Support setting a presentational role on an iframe so that the AXWebArea disappears from the hierarchy.
+        Accomplish this by adding children for attachment and scroll view elements the way other children are added.
+        That is, only add the non-ignored children directly (which means move the addChild logic into AccessibilityObject.)
+
+        Test: accessibility/presentation-role-iframe.html
+
+        * accessibility/AccessibilityNodeObject.cpp:
+        (WebCore::AccessibilityNodeObject::AccessibilityNodeObject):
+        (WebCore::AccessibilityNodeObject::insertChild): Deleted.
+        (WebCore::AccessibilityNodeObject::addChild): Deleted.
+        * accessibility/AccessibilityNodeObject.h:
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::AccessibilityObject):
+        (WebCore::AccessibilityObject::insertChild):
+        (WebCore::AccessibilityObject::addChild):
+        (WebCore::nodeHasPresentationRole):
+        * accessibility/AccessibilityObject.h:
+        (WebCore::AccessibilityObject::addChild): Deleted.
+        (WebCore::AccessibilityObject::insertChild): Deleted.
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::webAreaIsPresentational):
+        (WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored):
+        (WebCore::AccessibilityRenderObject::addAttachmentChildren):
+        * accessibility/AccessibilityScrollView.cpp:
+        (WebCore::AccessibilityScrollView::addChildren):
+
 2017-07-03  Matt Lewis  <jlewis3@apple.com>
 
         Unreviewed, rolling out r219024.
index 10ee001..9aedda9 100644 (file)
@@ -84,8 +84,6 @@ static String accessibleNameForNode(Node* node, Node* labelledbyNode = nullptr);
 AccessibilityNodeObject::AccessibilityNodeObject(Node* node)
     : AccessibilityObject()
     , m_ariaRole(UnknownRole)
-    , m_childrenDirty(false)
-    , m_subtreeDirty(false)
     , m_roleForMSAA(UnknownRole)
 #ifndef NDEBUG
     , m_initialized(false)
@@ -337,51 +335,6 @@ AccessibilityRole AccessibilityNodeObject::determineAccessibilityRole()
     return UnknownRole;
 }
 
-void AccessibilityNodeObject::insertChild(AccessibilityObject* child, unsigned index)
-{
-    if (!child)
-        return;
-    
-    // If the parent is asking for this child's children, then either it's the first time (and clearing is a no-op),
-    // or its visibility has changed. In the latter case, this child may have a stale child cached.
-    // This can prevent aria-hidden changes from working correctly. Hence, whenever a parent is getting children, ensure data is not stale.
-    // Only clear the child's children when we know it's in the updating chain in order to avoid unnecessary work.
-    if (child->needsToUpdateChildren() || m_subtreeDirty) {
-        child->clearChildren();
-        // Pass m_subtreeDirty flag down to the child so that children cache gets reset properly.
-        if (m_subtreeDirty)
-            child->setNeedsToUpdateSubtree();
-    } else {
-        // For some reason the grand children might be detached so that we need to regenerate the
-        // children list of this child.
-        for (const auto& grandChild : child->children(false)) {
-            if (grandChild->isDetachedFromParent()) {
-                child->clearChildren();
-                break;
-            }
-        }
-    }
-    
-    setIsIgnoredFromParentDataForChild(child);
-    if (child->accessibilityIsIgnored()) {
-        const auto& children = child->children();
-        size_t length = children.size();
-        for (size_t i = 0; i < length; ++i)
-            m_children.insert(index + i, children[i]);
-    } else {
-        ASSERT(child->parentObject() == this);
-        m_children.insert(index, child);
-    }
-    
-    // Reset the child's m_isIgnoredFromParentData since we are done adding that child and its children.
-    child->clearIsIgnoredFromParentData();
-}
-
-void AccessibilityNodeObject::addChild(AccessibilityObject* child)
-{
-    insertChild(child, m_children.size());
-}
-
 void AccessibilityNodeObject::addChildren()
 {
     // If the need to add more children in addition to existing children arises, 
index 587af73..2a9b04f 100644 (file)
@@ -145,8 +145,6 @@ protected:
     explicit AccessibilityNodeObject(Node*);
 
     AccessibilityRole m_ariaRole;
-    bool m_childrenDirty;
-    bool m_subtreeDirty;
     mutable AccessibilityRole m_roleForMSAA;
 #ifndef NDEBUG
     bool m_initialized;
@@ -156,8 +154,6 @@ protected:
 
     virtual AccessibilityRole determineAccessibilityRole();
     void addChildren() override;
-    void addChild(AccessibilityObject*) override;
-    void insertChild(AccessibilityObject*, unsigned index) override;
 
     bool canHaveChildren() const override;
     AccessibilityRole ariaRoleAttribute() const override;
index 91438af..4aff022 100644 (file)
@@ -88,6 +88,8 @@ AccessibilityObject::AccessibilityObject()
     , m_role(UnknownRole)
     , m_lastKnownIsIgnoredValue(DefaultBehavior)
     , m_isIgnoredFromParentData(AccessibilityIsIgnoredFromParentData())
+    , m_childrenDirty(false)
+    , m_subtreeDirty(false)
 #if PLATFORM(GTK)
     , m_wrapper(nullptr)
 #endif
@@ -551,6 +553,51 @@ static void appendAccessibilityObject(AccessibilityObject* object, Accessibility
         results.append(object);
 }
     
+void AccessibilityObject::insertChild(AccessibilityObject* child, unsigned index)
+{
+    if (!child)
+        return;
+    
+    // If the parent is asking for this child's children, then either it's the first time (and clearing is a no-op),
+    // or its visibility has changed. In the latter case, this child may have a stale child cached.
+    // This can prevent aria-hidden changes from working correctly. Hence, whenever a parent is getting children, ensure data is not stale.
+    // Only clear the child's children when we know it's in the updating chain in order to avoid unnecessary work.
+    if (child->needsToUpdateChildren() || m_subtreeDirty) {
+        child->clearChildren();
+        // Pass m_subtreeDirty flag down to the child so that children cache gets reset properly.
+        if (m_subtreeDirty)
+            child->setNeedsToUpdateSubtree();
+    } else {
+        // For some reason the grand children might be detached so that we need to regenerate the
+        // children list of this child.
+        for (const auto& grandChild : child->children(false)) {
+            if (grandChild->isDetachedFromParent()) {
+                child->clearChildren();
+                break;
+            }
+        }
+    }
+    
+    setIsIgnoredFromParentDataForChild(child);
+    if (child->accessibilityIsIgnored()) {
+        const auto& children = child->children();
+        size_t length = children.size();
+        for (size_t i = 0; i < length; ++i)
+            m_children.insert(index + i, children[i]);
+    } else {
+        ASSERT(child->parentObject() == this);
+        m_children.insert(index, child);
+    }
+    
+    // Reset the child's m_isIgnoredFromParentData since we are done adding that child and its children.
+    child->clearIsIgnoredFromParentData();
+}
+    
+void AccessibilityObject::addChild(AccessibilityObject* child)
+{
+    insertChild(child, m_children.size());
+}
+    
 static void appendChildrenToArray(AccessibilityObject* object, bool isForward, AccessibilityObject* startObject, AccessibilityObject::AccessibilityChildrenVector& results)
 {
     // A table's children includes elements whose own children are also the table's children (due to the way the Mac exposes tables).
@@ -2335,7 +2382,7 @@ String AccessibilityObject::roleDescription() const
     return stripLeadingAndTrailingHTMLSpaces(getAttribute(aria_roledescriptionAttr));
 }
     
-static bool nodeHasPresentationRole(Node* node)
+bool nodeHasPresentationRole(Node* node)
 {
     return nodeHasRole(node, "presentation") || nodeHasRole(node, "none");
 }
index cc7f501..d3c1a17 100644 (file)
@@ -480,7 +480,9 @@ enum AccessibilityMathScriptObjectType { Subscript, Superscript };
 enum AccessibilityMathMultiscriptObjectType { PreSubscript, PreSuperscript, PostSubscript, PostSuperscript };
 
 enum AccessibilityARIACurrentState { ARIACurrentFalse, ARIACurrentTrue, ARIACurrentPage, ARIACurrentStep, ARIACurrentLocation, ARIACurrentDate, ARIACurrentTime };
-
+    
+bool nodeHasPresentationRole(Node*);
+    
 class AccessibilityObject : public RefCounted<AccessibilityObject> {
 protected:
     AccessibilityObject();
@@ -850,8 +852,8 @@ public:
     virtual void updateAccessibilityRole() { }
     const AccessibilityChildrenVector& children(bool updateChildrenIfNeeded = true);
     virtual void addChildren() { }
-    virtual void addChild(AccessibilityObject*) { }
-    virtual void insertChild(AccessibilityObject*, unsigned) { }
+    virtual void addChild(AccessibilityObject*);
+    virtual void insertChild(AccessibilityObject*, unsigned);
 
     virtual bool shouldIgnoreAttributeRole() const { return false; }
     
@@ -1130,7 +1132,9 @@ protected:
     AccessibilityRole m_role;
     AccessibilityObjectInclusion m_lastKnownIsIgnoredValue;
     AccessibilityIsIgnoredFromParentData m_isIgnoredFromParentData;
-    
+    bool m_childrenDirty;
+    bool m_subtreeDirty;
+
     void setIsIgnoredFromParentData(AccessibilityIsIgnoredFromParentData& data) { m_isIgnoredFromParentData = data; }
 
     virtual bool computeAccessibilityIsIgnored() const { return true; }
index 4f0dd0e..47b2128 100644 (file)
@@ -1151,7 +1151,18 @@ AccessibilityObjectInclusion AccessibilityRenderObject::defaultObjectInclusion()
 
     return AccessibilityObject::defaultObjectInclusion();
 }
-
+    
+static bool webAreaIsPresentational(RenderObject* renderer)
+{
+    if (!is<RenderView>(*renderer))
+        return false;
+    
+    if (auto ownerElement = renderer->document().ownerElement())
+        return nodeHasPresentationRole(ownerElement);
+    
+    return false;
+}
+    
 bool AccessibilityRenderObject::computeAccessibilityIsIgnored() const
 {
 #ifndef NDEBUG
@@ -1180,6 +1191,10 @@ bool AccessibilityRenderObject::computeAccessibilityIsIgnored() const
     if (roleValue() == PresentationalRole || inheritsPresentationalRole())
         return true;
     
+    // WebAreas should be ignored if their iframe container is marked as presentational.
+    if (webAreaIsPresentational(m_renderer))
+        return true;
+        
     // An ARIA tree can only have tree items and static text as children.
     if (!isAllowedChildOfTree())
         return true;
@@ -3113,9 +3128,7 @@ void AccessibilityRenderObject::addAttachmentChildren()
     if (!widget || !widget->isFrameView())
         return;
     
-    AccessibilityObject* axWidget = axObjectCache()->getOrCreate(widget);
-    if (!axWidget->accessibilityIsIgnored())
-        m_children.append(axWidget);
+    addChild(axObjectCache()->getOrCreate(widget));
 }
 
 #if PLATFORM(COCOA)
index ed5aa58..9bbb091 100644 (file)
@@ -180,11 +180,8 @@ void AccessibilityScrollView::addChildren()
     ASSERT(!m_haveChildren);
     m_haveChildren = true;
     
-    AccessibilityObject* webArea = webAreaObject();
-    if (webArea && !webArea->accessibilityIsIgnored())
-        m_children.append(webArea);
-    
-    updateScrollbars();
+    addChild(webAreaObject());
+    updateScrollbars();    
 }
 
 AccessibilityObject* AccessibilityScrollView::webAreaObject() const