AX: [ATK] Attempting to clear selection on ARIA listboxes results in crash

https://bugs.webkit.org/show_bug.cgi?id=164331

Reviewed by Chris Fleizach.

Source/WebCore:

The ATK code is using is<AccessibilityListBox>() to identify native listboxes.
But is<AccessibilityListBox>() returns the value of isListBox() which returns
true both for AccessibilityListBox instances as well as for AccessibilityObject
instances which have an AccessibilityRole value of ListBoxRole. Because only
native listboxes should be AccessibilityListBoxes, add isNativeListBox() so
that we can distinguish native and ARIA listboxes.

Tests: accessibility/aria-listbox-clear-selection-crash.html
       accessibility/listbox-clear-selection.html

* accessibility/AccessibilityListBox.h:
* accessibility/AccessibilityObject.h:
(WebCore::AccessibilityObject::isNativeListBox):
(WebCore::AccessibilityObject::isListBox):

Tools:

Add AccessibilityUIElement::clearSelectedChildren() to DRT and WKTR.
Implement it for ATK.

* DumpRenderTree/AccessibilityUIElement.h:
* DumpRenderTree/ios/AccessibilityUIElementIOS.mm:
(AccessibilityUIElement::clearSelectedChildren):
* DumpRenderTree/mac/AccessibilityUIElementMac.mm:
(AccessibilityUIElement::clearSelectedChildren):
* DumpRenderTree/win/AccessibilityUIElementWin.cpp:
(AccessibilityUIElement::clearSelectedChildren):
* WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h:
* WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl:
* WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:
(WTR::AccessibilityUIElement::clearSelectedChildren):
* WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:
(WTR::AccessibilityUIElement::clearSelectedChildren):
* WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:
(WTR::AccessibilityUIElement::clearSelectedChildren):

LayoutTests:

Add tests to verify clearing selection works for native listboxes, and does not
crash for ARIA listboxes. These tests are being skipped for the Mac and Win ports
which do not yet implement AccessibilityUIElement::clearSelectedChildren().

* accessibility/aria-listbox-clear-selection-crash-expected.txt: Added.
* accessibility/aria-listbox-clear-selection-crash.html: Added.
* accessibility/listbox-clear-selection-expected.txt: Added.
* accessibility/listbox-clear-selection.html: Added.
* platform/mac/TestExpectations:
* platform/win/TestExpectations:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index cae8d47..554c509 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,21 @@
+2016-11-04  Joanmarie Diggs  <jdiggs@igalia.com>
+
+        AX: [ATK] Attempting to clear selection on ARIA listboxes results in crash
+        https://bugs.webkit.org/show_bug.cgi?id=164331
+
+        Reviewed by Chris Fleizach.
+
+        Add tests to verify clearing selection works for native listboxes, and does not
+        crash for ARIA listboxes. These tests are being skipped for the Mac and Win ports
+        which do not yet implement AccessibilityUIElement::clearSelectedChildren().
+
+        * accessibility/aria-listbox-clear-selection-crash-expected.txt: Added.
+        * accessibility/aria-listbox-clear-selection-crash.html: Added.
+        * accessibility/listbox-clear-selection-expected.txt: Added.
+        * accessibility/listbox-clear-selection.html: Added.
+        * platform/mac/TestExpectations:
+        * platform/win/TestExpectations:
+
 2016-11-04  Brady Eidson  <beidson@apple.com>
 
         IndexedDB 2.0: Throw the correct exceptions during IDBObjectStore/IDBIndex renaming.

diff --git a/LayoutTests/accessibility/aria-listbox-clear-selection-crash-expected.txt b/LayoutTests/accessibility/aria-listbox-clear-selection-crash-expected.txt
new file mode 100644 (file)
index 0000000..3e250ec
--- /dev/null
+++ b/LayoutTests/accessibility/aria-listbox-clear-selection-crash-expected.txt
@@ -0,0 +1,11 @@
+Option 1
+Option 2
+Tests that attempting to clear the selection in an ARIA listbox doesn't cause a crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS listbox.selectedChildrenCount is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

diff --git a/LayoutTests/accessibility/aria-listbox-clear-selection-crash.html b/LayoutTests/accessibility/aria-listbox-clear-selection-crash.html
new file mode 100644 (file)
index 0000000..a3f12e4
--- /dev/null
+++ b/LayoutTests/accessibility/aria-listbox-clear-selection-crash.html
@@ -0,0 +1,23 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body id="body">
+<ul id="listbox" role="listbox">
+    <li role="option">Option 1</li>
+    <li role="option">Option 2</li>
+</ul>
+<div id="description"></div>
+<div id="console"></div>
+<script>
+    description("Tests that attempting to clear the selection in an ARIA listbox doesn't cause a crash.");
+    if (window.accessibilityController) {
+        var listbox = accessibilityController.accessibleElementById("listbox");
+        listbox.clearSelectedChildren();
+        shouldBe("listbox.selectedChildrenCount", "0");
+    }
+</script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>

diff --git a/LayoutTests/accessibility/listbox-clear-selection-expected.txt b/LayoutTests/accessibility/listbox-clear-selection-expected.txt
new file mode 100644 (file)
index 0000000..55ff722
--- /dev/null
+++ b/LayoutTests/accessibility/listbox-clear-selection-expected.txt
@@ -0,0 +1,13 @@
+
+Tests clearing the selection of a multi-select select element.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS listbox.selectedChildrenCount is 0
+PASS listbox.selectedChildrenCount is 1
+PASS listbox.selectedChildrenCount is 2
+PASS listbox.selectedChildrenCount is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

diff --git a/LayoutTests/accessibility/listbox-clear-selection.html b/LayoutTests/accessibility/listbox-clear-selection.html
new file mode 100644 (file)
index 0000000..1afec37
--- /dev/null
+++ b/LayoutTests/accessibility/listbox-clear-selection.html
@@ -0,0 +1,31 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body id="body">
+<select id="listbox" multiple="multiple">
+    <option>Option 1</option>
+    <option>Option 2</option>
+</select>
+<div id="description"></div>
+<div id="console"></div>
+<script>
+    description("Tests clearing the selection of a multi-select select element.");
+    if (window.accessibilityController) {
+        var listbox = accessibilityController.accessibleElementById("listbox");
+        shouldBe("listbox.selectedChildrenCount", "0");
+
+        listbox.setSelectedChildAtIndex(0);
+        shouldBe("listbox.selectedChildrenCount", "1");
+
+        listbox.setSelectedChildAtIndex(1);
+        shouldBe("listbox.selectedChildrenCount", "2");
+
+        listbox.clearSelectedChildren();
+        shouldBe("listbox.selectedChildrenCount", "0");
+    }
+</script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>

diff --git a/LayoutTests/platform/mac/TestExpectations b/LayoutTests/platform/mac/TestExpectations
index 02cdd9f..af896fd 100644 (file)
--- a/LayoutTests/platform/mac/TestExpectations
+++ b/LayoutTests/platform/mac/TestExpectations
@@ -59,6 +59,10 @@ accessibility/children-changed-sends-notification.html
 accessibility/menu-list-sends-change-notification.html
 accessibility/multiselect-list-reports-active-option.html
 
+# Need to implement AccessibilityUIElement::clearSelectedChildren()
+accessibility/aria-listbox-clear-selection-crash.html
+accessibility/listbox-clear-selection.html
+
 # Need to implement this in plugins code in WebKit
 plugins/reloadplugins-and-pages.html
 

diff --git a/LayoutTests/platform/win/TestExpectations b/LayoutTests/platform/win/TestExpectations
index 379cb71..d52b0e3 100644 (file)
--- a/LayoutTests/platform/win/TestExpectations
+++ b/LayoutTests/platform/win/TestExpectations
@@ -222,6 +222,10 @@ http/tests/misc/image-checks-for-accept.html [ Skip ]
 # TODO Need to add functionality to DumpRenderTree to handle enable/disable Spatial Navigation
 fast/spatial-navigation/ [ Skip ]
 
+# Need to implement AccessibilityUIElement::clearSelectedChildren()
+accessibility/aria-listbox-clear-selection-crash.html [ Skip ]
+accessibility/listbox-clear-selection.html [ Skip ]
+
 # TODO Investigate why these mouse scroll tests are failing.
 fast/events/platform-wheelevent-in-scrolling-div.html [ Failure ]
 fast/events/continuous-platform-wheelevent-in-scrolling-div.html [ Failure ]

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index add97c5..6be93e1 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2016-11-04  Joanmarie Diggs  <jdiggs@igalia.com>
+
+        AX: [ATK] Attempting to clear selection on ARIA listboxes results in crash
+        https://bugs.webkit.org/show_bug.cgi?id=164331
+
+        Reviewed by Chris Fleizach.
+
+        The ATK code is using is<AccessibilityListBox>() to identify native listboxes.
+        But is<AccessibilityListBox>() returns the value of isListBox() which returns
+        true both for AccessibilityListBox instances as well as for AccessibilityObject
+        instances which have an AccessibilityRole value of ListBoxRole. Because only
+        native listboxes should be AccessibilityListBoxes, add isNativeListBox() so
+        that we can distinguish native and ARIA listboxes.
+
+        Tests: accessibility/aria-listbox-clear-selection-crash.html
+               accessibility/listbox-clear-selection.html
+
+        * accessibility/AccessibilityListBox.h:
+        * accessibility/AccessibilityObject.h:
+        (WebCore::AccessibilityObject::isNativeListBox):
+        (WebCore::AccessibilityObject::isListBox):
+
 2016-11-04  Brady Eidson  <beidson@apple.com>
 
         IndexedDB 2.0: Throw the correct exceptions during IDBObjectStore/IDBIndex renaming.

diff --git a/Source/WebCore/accessibility/AccessibilityListBox.h b/Source/WebCore/accessibility/AccessibilityListBox.h
index 2e5f023..ab91d93 100644 (file)
--- a/Source/WebCore/accessibility/AccessibilityListBox.h
+++ b/Source/WebCore/accessibility/AccessibilityListBox.h
@@ -49,11 +49,11 @@ public:
 private:
     explicit AccessibilityListBox(RenderObject*);
 
-    bool isListBox() const override { return true; }
+    bool isNativeListBox() const override { return true; }
     AccessibilityObject* listBoxOptionAccessibilityObject(HTMLElement*) const;
     AccessibilityObject* elementAccessibilityHitTest(const IntPoint&) const override;
 };
     
 } // namespace WebCore
 
-SPECIALIZE_TYPE_TRAITS_ACCESSIBILITY(AccessibilityListBox, isListBox())
+SPECIALIZE_TYPE_TRAITS_ACCESSIBILITY(AccessibilityListBox, isNativeListBox())

diff --git a/Source/WebCore/accessibility/AccessibilityObject.h b/Source/WebCore/accessibility/AccessibilityObject.h
index 371cd23..daea0f5 100644 (file)
--- a/Source/WebCore/accessibility/AccessibilityObject.h
+++ b/Source/WebCore/accessibility/AccessibilityObject.h
@@ -494,7 +494,8 @@ public:
     bool isWebArea() const { return roleValue() == WebAreaRole; }
     virtual bool isCheckbox() const { return roleValue() == CheckBoxRole; }
     virtual bool isRadioButton() const { return roleValue() == RadioButtonRole; }
-    virtual bool isListBox() const { return roleValue() == ListBoxRole; }
+    virtual bool isNativeListBox() const { return false; }
+    bool isListBox() const { return roleValue() == ListBoxRole; }
     virtual bool isListBoxOption() const { return false; }
     virtual bool isAttachment() const { return false; }
     virtual bool isMediaTimeline() const { return false; }

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index b0af40c..2af956e 100644 (file)
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,29 @@
+2016-11-04  Joanmarie Diggs  <jdiggs@igalia.com>
+
+        AX: [ATK] Attempting to clear selection on ARIA listboxes results in crash
+        https://bugs.webkit.org/show_bug.cgi?id=164331
+
+        Reviewed by Chris Fleizach.
+
+        Add AccessibilityUIElement::clearSelectedChildren() to DRT and WKTR.
+        Implement it for ATK.
+
+        * DumpRenderTree/AccessibilityUIElement.h:
+        * DumpRenderTree/ios/AccessibilityUIElementIOS.mm:
+        (AccessibilityUIElement::clearSelectedChildren):
+        * DumpRenderTree/mac/AccessibilityUIElementMac.mm:
+        (AccessibilityUIElement::clearSelectedChildren):
+        * DumpRenderTree/win/AccessibilityUIElementWin.cpp:
+        (AccessibilityUIElement::clearSelectedChildren):
+        * WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h:
+        * WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl:
+        * WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:
+        (WTR::AccessibilityUIElement::clearSelectedChildren):
+        * WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:
+        (WTR::AccessibilityUIElement::clearSelectedChildren):
+        * WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:
+        (WTR::AccessibilityUIElement::clearSelectedChildren):
+
 2016-11-04  Per Arne Vollan  <pvollan@apple.com>
 
         [Win] Page visibility tests are timing out.

diff --git a/Tools/DumpRenderTree/AccessibilityUIElement.h b/Tools/DumpRenderTree/AccessibilityUIElement.h
index 4abf9e9..1c716ce 100644 (file)
--- a/Tools/DumpRenderTree/AccessibilityUIElement.h
+++ b/Tools/DumpRenderTree/AccessibilityUIElement.h
@@ -152,6 +152,7 @@ public:
     AccessibilityUIElement selectedChildAtIndex(unsigned) const;
     void setSelectedChildAtIndex(unsigned) const;
     void removeSelectionAtIndex(unsigned) const;
+    void clearSelectedChildren() const;
     
     bool isExpanded() const;
     bool isChecked() const;

diff --git a/Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm b/Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm
index 954221e..4ad6e52 100644 (file)
--- a/Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm
+++ b/Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm
@@ -986,6 +986,11 @@ JSStringRef AccessibilityUIElement::accessibilityValue() const
     return JSStringCreateWithCharacters(0, 0);
 }
 
+void AccessibilityUIElement::clearSelectedChildren() const
+{
+    // FIXME: implement
+}
+
 JSStringRef AccessibilityUIElement::documentEncoding()
 {
     return JSStringCreateWithCharacters(0, 0);

diff --git a/Tools/DumpRenderTree/mac/AccessibilityUIElementMac.mm b/Tools/DumpRenderTree/mac/AccessibilityUIElementMac.mm
index 7a56ca7..511b8f3 100644 (file)
--- a/Tools/DumpRenderTree/mac/AccessibilityUIElementMac.mm
+++ b/Tools/DumpRenderTree/mac/AccessibilityUIElementMac.mm
@@ -1440,6 +1440,11 @@ void AccessibilityUIElement::removeSelectionAtIndex(unsigned index) const
     END_AX_OBJC_EXCEPTIONS
 }
 
+void AccessibilityUIElement::clearSelectedChildren() const
+{
+    // FIXME: implement
+}
+
 JSStringRef AccessibilityUIElement::accessibilityValue() const
 {
     // FIXME: implement

diff --git a/Tools/DumpRenderTree/win/AccessibilityUIElementWin.cpp b/Tools/DumpRenderTree/win/AccessibilityUIElementWin.cpp
index 9a7f857..218d7ee 100644 (file)
--- a/Tools/DumpRenderTree/win/AccessibilityUIElementWin.cpp
+++ b/Tools/DumpRenderTree/win/AccessibilityUIElementWin.cpp
@@ -802,6 +802,10 @@ JSStringRef AccessibilityUIElement::accessibilityValue() const
     return JSStringCreateWithBSTR(valueBSTR);
 }
 
+void AccessibilityUIElement::clearSelectedChildren() const
+{
+    // FIXME: implement
+}
 
 JSStringRef AccessibilityUIElement::documentEncoding()
 {

diff --git a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h b/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h
index 86ceddb..5fa4e0d 100644 (file)
--- a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h
+++ b/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h
@@ -143,6 +143,7 @@ public:
     void setSelectedChild(AccessibilityUIElement*) const;
     void setSelectedChildAtIndex(unsigned) const;
     void removeSelectionAtIndex(unsigned) const;
+    void clearSelectedChildren() const;
     unsigned selectedChildrenCount() const;
     RefPtr<AccessibilityUIElement> selectedChildAtIndex(unsigned) const;
     

diff --git a/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl b/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl
index 60be603..fcac2a4 100644 (file)
--- a/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl
+++ b/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl
@@ -39,6 +39,7 @@ interface AccessibilityUIElement {
     void setSelectedChild(AccessibilityUIElement element);
     void setSelectedChildAtIndex(unsigned long index);
     void removeSelectionAtIndex(unsigned long index);
+    void clearSelectedChildren();
     AccessibilityUIElement titleUIElement();
     AccessibilityUIElement parentElement();
     

diff --git a/Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp b/Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp
index 6406976..1dc709c 100644 (file)
--- a/Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp
+++ b/Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp
@@ -1796,6 +1796,14 @@ void AccessibilityUIElement::removeSelectionAtIndex(unsigned index) const
     atk_selection_remove_selection(ATK_SELECTION(m_element.get()), index);
 }
 
+void AccessibilityUIElement::clearSelectedChildren() const
+{
+    if (!ATK_IS_SELECTION(m_element.get()))
+        return;
+
+    atk_selection_clear_selection(ATK_SELECTION(m_element.get()));
+}
+
 JSRetainPtr<JSStringRef> AccessibilityUIElement::accessibilityValue() const
 {
     // FIXME: implement

diff --git a/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm b/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm
index f47c7c5..9c2caa4 100644 (file)
--- a/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm
+++ b/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm
@@ -832,6 +832,11 @@ void AccessibilityUIElement::removeSelectionAtIndex(unsigned index) const
 {
 }
 
+void AccessibilityUIElement::clearSelectedChildren() const
+{
+    // FIXME: implement
+}
+
 JSRetainPtr<JSStringRef> AccessibilityUIElement::accessibilityValue() const
 {
     // FIXME: implement

diff --git a/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm b/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm
index 0c182e0..cd04f8c 100644 (file)
--- a/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm
+++ b/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm
@@ -1492,6 +1492,11 @@ void AccessibilityUIElement::removeSelectionAtIndex(unsigned index) const
     END_AX_OBJC_EXCEPTIONS
 }
 
+void AccessibilityUIElement::clearSelectedChildren() const
+{
+    // FIXME: implement
+}
+
 JSRetainPtr<JSStringRef> AccessibilityUIElement::accessibilityValue() const
 {
     // FIXME: implement