Regression: AX: Don't expose role or notifications for invalid menus
authorcfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Mar 2014 21:26:25 +0000 (21:26 +0000)
committercfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 24 Mar 2014 21:26:25 +0000 (21:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=129814

Reviewed by Mario Sanchez Prada.

Source/WebCore:

If a role=menu has no menuitem children, it should not be a menu.
This was a bit tricky to implement since we need to update the role after the children are created,
but it means we have to be a bit more aggressive about when to updateChildren, so that the role
is known to be valid.

Test: platform/mac/accessibility/invalid-menu-role-does-not-send-notification.html

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::notificationPostTimerFired):
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::updateRoleAfterChildrenCreation):
(WebCore::AccessibilityRenderObject::addChildren):
* accessibility/AccessibilityRenderObject.h:
* accessibility/mac/WebAccessibilityObjectWrapperBase.mm:
(-[WebAccessibilityObjectWrapperBase updateObjectBackingStore]):

LayoutTests:

* accessibility/table-with-aria-role.html:
     Modify test so that tests only what we want (that the role is correct).
     No longer use role=menu on the table, since the table won't have menuitems and won't be a valid menu.
* platform/mac/accessibility/invalid-menu-role-does-not-send-notification-expected.txt: Added.
* platform/mac/accessibility/invalid-menu-role-does-not-send-notification.html: Added.
* platform/mac/accessibility/table-with-aria-role-expected.txt:

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

LayoutTests/ChangeLog
LayoutTests/accessibility/table-with-aria-role-expected.txt [new file with mode: 0644]
LayoutTests/accessibility/table-with-aria-role.html
LayoutTests/platform/mac/accessibility/invalid-menu-role-does-not-send-notification-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/accessibility/invalid-menu-role-does-not-send-notification.html [new file with mode: 0644]
LayoutTests/platform/mac/accessibility/table-with-aria-role-expected.txt [deleted file]
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AXObjectCache.cpp
Source/WebCore/accessibility/AccessibilityObject.cpp
Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Source/WebCore/accessibility/AccessibilityRenderObject.h

index bf22812..5ec363a 100644 (file)
@@ -1,3 +1,17 @@
+2014-03-18  Chris Fleizach  <cfleizach@apple.com>
+
+        Regression: AX: Don't expose role or notifications for invalid menus
+        https://bugs.webkit.org/show_bug.cgi?id=129814
+
+        Reviewed by Mario Sanchez Prada.
+
+        * accessibility/table-with-aria-role.html:
+             Modify test so that tests only what we want (that the role is correct).
+             No longer use role=menu on the table, since the table won't have menuitems and won't be a valid menu.
+        * platform/mac/accessibility/invalid-menu-role-does-not-send-notification-expected.txt: Added.
+        * platform/mac/accessibility/invalid-menu-role-does-not-send-notification.html: Added.
+        * platform/mac/accessibility/table-with-aria-role-expected.txt:
+
 2014-03-24  Chris Fleizach  <cfleizach@apple.com>
 
         <hr> should expose AXRole/AXSubrole, etc
diff --git a/LayoutTests/accessibility/table-with-aria-role-expected.txt b/LayoutTests/accessibility/table-with-aria-role-expected.txt
new file mode 100644 (file)
index 0000000..ec613ea
--- /dev/null
@@ -0,0 +1,6 @@
+Table1 should not have a table role because it's a button: AXRole: AXButton
+Table2 should not have a table role because it's a group: AXRole: AXGroup
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
index ebbce6a..bde2ebe 100644 (file)
@@ -1,28 +1,33 @@
 <html>
+<script src="../resources/js-test-pre.js"></script>
 <script>
     if (window.testRunner)
         testRunner.dumpAsText();
 </script>
 <body id="body">
 
-    <table border=1 role="button">
+<div id="content">
+    <table border=1 role="button" id='table1'>
        <tr><td>test</td><td>test</td><td>test</td></tr>
        <tr><td>test</td><td>test</td><td>test</td></tr>
     </table>
 
-    <table border=1 role="menu">
+    <table border=1 role="group" id='table2'>
        <tr><td>test</td><td>test</td><td>test</td></tr>
        <tr><td>test</td><td>test</td><td>test</td></tr>
     </table>
+</div>
 
     <div id="result"></div>
     
     <script>
         if (window.accessibilityController) {
-            var body = document.getElementById("body");
-            body.focus();
-            result.innerText += accessibilityController.focusedElement.attributesOfChildren();
+            debug("Table1 should not have a table role because it's a button: " + accessibilityController.accessibleElementById('table1').role);
+            debug("Table2 should not have a table role because it's a group: " + accessibilityController.accessibleElementById('table2').role);
+
+            document.getElementById("content").style.display = "none";
         }
     </script>
+<script src="../resources/js-test-post.js"></script>
 </body>
 </html>
diff --git a/LayoutTests/platform/mac/accessibility/invalid-menu-role-does-not-send-notification-expected.txt b/LayoutTests/platform/mac/accessibility/invalid-menu-role-does-not-send-notification-expected.txt
new file mode 100644 (file)
index 0000000..727bcfb
--- /dev/null
@@ -0,0 +1,10 @@
+This tests that the AXMenuOpened notification does not get fired for a menu that is invalid (does not have a menu item child).
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+The item should not have a menu role: AXRole: AXGroup
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/platform/mac/accessibility/invalid-menu-role-does-not-send-notification.html b/LayoutTests/platform/mac/accessibility/invalid-menu-role-does-not-send-notification.html
new file mode 100644 (file)
index 0000000..b4bec0c
--- /dev/null
@@ -0,0 +1,58 @@
+<!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="menu1" role="menu" hidden>
+<a href="#">item</a>
+</div>
+</div>
+
+
+<p id="description"></p>
+<div id="console"></div>
+
+<script>
+
+    description("This tests that the AXMenuOpened notification does not get fired for a menu that is invalid (does not have a menu item child).");
+
+    function showMenu(menu) {
+        document.getElementById("menu1").removeAttribute("hidden");
+        debug("The item should not have a menu role: " + accessibilityController.accessibleElementById('menu1').role);
+    }
+
+    var notification = 0;
+    var element = 0;
+    function ariaCallback(element, notification) {
+        if (notification == "AXMenuOpened") {
+           debug("FAIL Received menu opened notification: " + notification);
+        }
+    }
+
+    function endTest() {
+       document.getElementById("content").style.display = 'none';
+       accessibilityController.removeNotificationListener();
+       finishJSTest();
+    }
+
+    window.jsTestIsAsync = true;
+    if (window.accessibilityController) {
+        var addedNotification = accessibilityController.addNotificationListener(ariaCallback);
+
+        // Make sure AX is enabled by accessing root element.
+        accessibilityController.rootElement;
+
+        setTimeout("showMenu();", 1);
+        // We should not actually get this notification, so end the test after a short time when we 'might' have gotten the notification.
+        setTimeout("endTest();", 100);
+    }
+    successfullyParsed = true;
+
+</script>
+
+<script src="../../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/platform/mac/accessibility/table-with-aria-role-expected.txt b/LayoutTests/platform/mac/accessibility/table-with-aria-role-expected.txt
deleted file mode 100644 (file)
index 8bb2a06..0000000
+++ /dev/null
@@ -1,45 +0,0 @@
-test   test    test
-test   test    test
-test   test    test
-test   test    test
-AXRole: AXButton
-AXSubrole: (null)
-AXRoleDescription: button
-AXChildren: <array of size 0>
-AXHelp: 
-AXParent: <AXButton: 'test test test test test test'>
-AXSize: NSSize: {85, 52}
-AXTitle: test test test test test test
-AXDescription: 
-AXFocused: 0
-AXEnabled: 1
-AXWindow: <AXButton: 'test test test test test test'>
-AXSelectedTextMarkerRange: (null)
-AXStartTextMarker: <AXButton: 'test test test test test test'>
-AXEndTextMarker: <AXButton: 'test test test test test test'>
-AXVisited: 0
-AXLinkedUIElements: (null)
-AXSelected: 0
-AXBlockQuoteLevel: 0
-AXTopLevelUIElement: <AXButton: 'test test test test test test'>
-AXLanguage: 
-AXDOMIdentifier: 
-AXDOMClassList: <array of size 0>
-AXTitleUIElement: (null)
-AXAccessKey: (null)
-AXElementBusy: 0
-
-------------
-AXRole: AXMenu
-AXRoleDescription: menu
-AXChildren: <array of size 6>
-AXParent: <AXMenu>
-AXEnabled: 1
-AXSize: NSSize: {85, 52}
-AXSelectedChildren: (null)
-AXVisibleChildren: (null)
-AXTitleUIElement: (null)
-AXElementBusy: 0
-
-------------
-
index c7d1fb8..570722c 100644 (file)
@@ -1,3 +1,26 @@
+2014-03-24  Chris Fleizach  <cfleizach@apple.com>
+
+        Regression: AX: Don't expose role or notifications for invalid menus
+        https://bugs.webkit.org/show_bug.cgi?id=129814
+
+        Reviewed by Mario Sanchez Prada.
+
+        If a role=menu has no menuitem children, it should not be a menu.
+        This was a bit tricky to implement since we need to update the role after the children are created,
+        but it means we have to be a bit more aggressive about when to updateChildren, so that the role
+        is known to be valid.
+
+        Test: platform/mac/accessibility/invalid-menu-role-does-not-send-notification.html
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::notificationPostTimerFired):
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::updateRoleAfterChildrenCreation):
+        (WebCore::AccessibilityRenderObject::addChildren):
+        * accessibility/AccessibilityRenderObject.h:
+        * accessibility/mac/WebAccessibilityObjectWrapperBase.mm:
+        (-[WebAccessibilityObjectWrapperBase updateObjectBackingStore]):
+
 2014-03-24  Andy Estes  <aestes@apple.com>
 
         [iOS] Download support by CFURLDownloadRef under USE(CFNETWORK).
index 9f39535..3435156 100644 (file)
@@ -684,6 +684,15 @@ void AXObjectCache::notificationPostTimerFired(Timer<AXObjectCache>&)
 #endif
 
         AXNotification notification = note.second;
+        
+        // Ensure that this menu really is a menu. We do this check here so that we don't have to create
+        // the axChildren when the menu is marked as opening.
+        if (notification == AXMenuOpened) {
+            obj->updateChildrenIfNecessary();
+            if (obj->roleValue() != MenuRole)
+                continue;
+        }
+        
         postPlatformNotification(obj, notification);
 
         if (notification == AXChildrenChanged && obj->parentObjectIfExists() && obj->lastKnownIsIgnoredValue() != obj->accessibilityIsIgnored())
index df9e7b8..7cc9c18 100644 (file)
@@ -1354,6 +1354,8 @@ void AccessibilityObject::updateBackingStore()
     // Updating the layout may delete this object.
     if (Document* document = this->document())
         document->updateLayoutIgnorePendingStylesheets();
+    
+    updateChildrenIfNecessary();
 }
 #endif
 
index de1f961..5e85c6d 100644 (file)
@@ -2995,6 +2995,24 @@ void AccessibilityRenderObject::addHiddenChildren()
     }
 }
     
+void AccessibilityRenderObject::updateRoleAfterChildrenCreation()
+{
+    // If a menu does not have valid menuitem children, it should not be exposed as a menu.
+    if (roleValue() == MenuRole) {
+        // Elements marked as menus must have at least one menu item child.
+        size_t menuItemCount = 0;
+        for (const auto& child : children()) {
+            if (child->isMenuItem()) {
+                menuItemCount++;
+                break;
+            }
+        }
+
+        if (!menuItemCount)
+            m_role = GroupRole;
+    }
+}
+    
 void AccessibilityRenderObject::addChildren()
 {
     // If the need to add more children in addition to existing children arises, 
@@ -3019,6 +3037,8 @@ void AccessibilityRenderObject::addChildren()
 #if PLATFORM(COCOA)
     updateAttachmentViewParents();
 #endif
+    
+    updateRoleAfterChildrenCreation();
 }
 
 bool AccessibilityRenderObject::canHaveChildren() const
index 20f56f7..da8f814 100644 (file)
@@ -269,7 +269,8 @@ private:
 #endif
     virtual String expandedTextValue() const;
     virtual bool supportsExpandedTextValue() const;
-
+    void updateRoleAfterChildrenCreation();
+    
     void ariaSelectedRows(AccessibilityChildrenVector&);
     
     bool elementAttributeValue(const QualifiedName&) const;