AX: crash when accessing selectedTab in a tab list
authorcfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Nov 2011 19:08:03 +0000 (19:08 +0000)
committercfleizach@apple.com <cfleizach@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Nov 2011 19:08:03 +0000 (19:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=70938

Reviewed by Beth Dakin.

Source/WebCore:

There were a few methods accessing m_children directly without first validating that those elements
needed to be updated (because the layout changed). Changing those to call children() ensures
that they will have the correct children.

Test: platform/mac/accessibility/selected-tab-crash.html

* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::isChecked):
(WebCore::AccessibilityRenderObject::selectedRadioButton):
(WebCore::AccessibilityRenderObject::selectedTabItem):
(WebCore::AccessibilityRenderObject::ariaListboxVisibleChildren):
(WebCore::AccessibilityRenderObject::tabChildren):

Tools:

Add the ability to retrieve an element through an arbitrary attribute.

* DumpRenderTree/AccessibilityUIElement.cpp:
(uiElementAttributeValueCallback):
(AccessibilityUIElement::uiElementAttributeValue):
(AccessibilityUIElement::getJSClass):
* DumpRenderTree/AccessibilityUIElement.h:
* DumpRenderTree/mac/AccessibilityUIElementMac.mm:
(AccessibilityUIElement::uiElementAttributeValue):

LayoutTests:

* platform/mac/accessibility/selected-tab-crash-expected.txt: Added.
* platform/mac/accessibility/selected-tab-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/mac/accessibility/selected-tab-crash-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/accessibility/selected-tab-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Tools/ChangeLog
Tools/DumpRenderTree/AccessibilityUIElement.cpp
Tools/DumpRenderTree/AccessibilityUIElement.h
Tools/DumpRenderTree/mac/AccessibilityUIElementMac.mm

index 34b43a7772279755d6ae2f5e0347c3bc506a386d..8a5a2fdb18b87bffd3a6bc37f9712fec2a1fd55e 100755 (executable)
@@ -1,3 +1,13 @@
+2011-11-09 Chris Fleizach  <cfleizach@apple.com>
+
+        AX: crash when accessing selectedTab in a tab list
+        https://bugs.webkit.org/show_bug.cgi?id=70938
+
+        Reviewed by Beth Dakin.
+
+        * platform/mac/accessibility/selected-tab-crash-expected.txt: Added.
+        * platform/mac/accessibility/selected-tab-crash.html: Added.
+
 2011-11-09  Ojan Vafai  <ojan@chromium.org>
 
         Remove failures fixed with r99630.
diff --git a/LayoutTests/platform/mac/accessibility/selected-tab-crash-expected.txt b/LayoutTests/platform/mac/accessibility/selected-tab-crash-expected.txt
new file mode 100644 (file)
index 0000000..e2eee6c
--- /dev/null
@@ -0,0 +1,11 @@
+tab 2
+tab 3
+This tests that if a tablists children are updated, we will not crash accessing an old object.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/platform/mac/accessibility/selected-tab-crash.html b/LayoutTests/platform/mac/accessibility/selected-tab-crash.html
new file mode 100644 (file)
index 0000000..b3e29e1
--- /dev/null
@@ -0,0 +1,48 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../../fast/js/resources/js-test-style.css">
+<script>
+var successfullyParsed = false;
+</script>
+<script src="../../../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body id="body">
+
+<div role="tablist" tabindex="0" id="tablist">
+<div role="tab" aria-checked="true" id="selectedtab">tab 1</div>
+<div role="tab">tab 2</div>
+<div role="tab">tab 3</div>
+</div>
+
+<p id="description"></p>
+<div id="console"></div>
+
+<script>
+
+    description("This tests that if a tablists children are updated, we will not crash accessing an old object.");
+
+    if (window.accessibilityController) {
+
+        document.getElementById("tablist").focus();
+        var tablist = accessibilityController.focusedElement;
+
+        // Iterate all the children so we have a cache of them.
+        tablist.attributesOfChildren();
+
+        // Retrieve and verify we have the right selected child.
+        var selectedTab = tablist.uiElementAttributeValue("AXValue");
+
+        // Delete the selected child.
+        document.getElementById("tablist").removeChild(document.getElementById("selectedtab"));
+
+        // Retrieve the tab. We should not crash here!
+        var selectedTab = tablist.uiElementAttributeValue("AXValue");
+    }
+
+    successfullyParsed = true;
+</script>
+
+<script src="../../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index d11856f5969d1a75e0e4155e40262a787d1ef86a..1f696c0f9b197249757fdcbd86044af8d6f0a205 100755 (executable)
@@ -1,3 +1,23 @@
+2011-11-09  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: crash when accessing selectedTab in a tab list
+        https://bugs.webkit.org/show_bug.cgi?id=70938
+
+        Reviewed by Beth Dakin.
+
+        There were a few methods accessing m_children directly without first validating that those elements
+        needed to be updated (because the layout changed). Changing those to call children() ensures
+        that they will have the correct children.
+
+        Test: platform/mac/accessibility/selected-tab-crash.html
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::isChecked):
+        (WebCore::AccessibilityRenderObject::selectedRadioButton):
+        (WebCore::AccessibilityRenderObject::selectedTabItem):
+        (WebCore::AccessibilityRenderObject::ariaListboxVisibleChildren):
+        (WebCore::AccessibilityRenderObject::tabChildren):
+
 2011-11-09  Andreas Kling  <kling@webkit.org>
 
         Shrink HTMLCollection.
index b402ead76cb9033d77b6d3d6b6fd3eed10bccf25..4a8994ca9e72ce16f3a3c5b8836ad7abc9821bec 100644 (file)
@@ -628,11 +628,13 @@ bool AccessibilityRenderObject::isNativeCheckboxOrRadio() const
 bool AccessibilityRenderObject::isChecked() const
 {
     ASSERT(m_renderer);
-    if (!m_renderer->node())
+    
+    Node* node = this->node();
+    if (!node)
         return false;
 
     // First test for native checkedness semantics
-    HTMLInputElement* inputElement = m_renderer->node()->toInputElement();
+    HTMLInputElement* inputElement = node->toInputElement();
     if (inputElement)
         return inputElement->shouldAppearChecked();
 
@@ -775,10 +777,12 @@ AccessibilityObject* AccessibilityRenderObject::selectedRadioButton()
     if (!isRadioGroup())
         return 0;
     
+    AccessibilityObject::AccessibilityChildrenVector children = this->children();
+
     // Find the child radio button that is selected (ie. the intValue == 1).
-    int count = m_children.size();
-    for (int i = 0; i < count; ++i) {
-        AccessibilityObject* object = m_children[i].get();
+    size_t size = children.size();
+    for (size_t i = 0; i < size; ++i) {
+        AccessibilityObject* object = children[i].get();
         if (object->roleValue() == RadioButtonRole && object->checkboxOrRadioValue() == ButtonStateOn)
             return object;
     }
@@ -794,9 +798,11 @@ AccessibilityObject* AccessibilityRenderObject::selectedTabItem()
     AccessibilityObject::AccessibilityChildrenVector tabs;
     tabChildren(tabs);
     
-    int count = tabs.size();
-    for (int i = 0; i < count; ++i) {
-        AccessibilityObject* object = m_children[i].get();
+    AccessibilityObject::AccessibilityChildrenVector children = this->children();
+    
+    size_t size = tabs.size();
+    for (size_t i = 0; i < size; ++i) {
+        AccessibilityObject* object = children[i].get();
         if (object->isTabItem() && object->isChecked())
             return object;
     }
@@ -3661,10 +3667,11 @@ void AccessibilityRenderObject::ariaListboxVisibleChildren(AccessibilityChildren
     if (!hasChildren())
         addChildren();
     
-    unsigned length = m_children.size();
-    for (unsigned i = 0; i < length; i++) {
-        if (!m_children[i]->isOffScreen())
-            result.append(m_children[i]);
+    AccessibilityObject::AccessibilityChildrenVector children = this->children();
+    size_t size = children.size();
+    for (size_t i = 0; i < size; i++) {
+        if (!children[i]->isOffScreen())
+            result.append(children[i]);
     }
 }
 
@@ -3684,10 +3691,11 @@ void AccessibilityRenderObject::tabChildren(AccessibilityChildrenVector& result)
 {
     ASSERT(roleValue() == TabListRole);
     
-    unsigned length = m_children.size();
-    for (unsigned i = 0; i < length; ++i) {
-        if (m_children[i]->isTabItem())
-            result.append(m_children[i]);
+    AccessibilityObject::AccessibilityChildrenVector children = this->children();
+    size_t size = children.size();
+    for (size_t i = 0; i < size; ++i) {
+        if (children[i]->isTabItem())
+            result.append(children[i]);
     }
 }
     
index c54dec0294bf461b5ce21aca26fde65f48c284eb..5c419368292dbbebbaaae746b3827980894464ad 100644 (file)
@@ -1,3 +1,20 @@
+2011-11-09  Chris Fleizach  <cfleizach@apple.com>
+
+        AX: crash when accessing selectedTab in a tab list
+        https://bugs.webkit.org/show_bug.cgi?id=70938
+
+        Reviewed by Beth Dakin.
+
+        Add the ability to retrieve an element through an arbitrary attribute.
+
+        * DumpRenderTree/AccessibilityUIElement.cpp:
+        (uiElementAttributeValueCallback):
+        (AccessibilityUIElement::uiElementAttributeValue):
+        (AccessibilityUIElement::getJSClass):
+        * DumpRenderTree/AccessibilityUIElement.h:
+        * DumpRenderTree/mac/AccessibilityUIElementMac.mm:
+        (AccessibilityUIElement::uiElementAttributeValue):
+
 2011-11-09  Andy Wingo  <wingo@igalia.com>
 
         Add webkitdirs.pm:getArchitecture implementation for GTK
index 03b0fab1cf5bb881d33e089358c3062bff987952..d22a355bccccdbd5ee7370e1f0abc803354833d1 100644 (file)
@@ -374,6 +374,15 @@ static JSValueRef stringAttributeValueCallback(JSContextRef context, JSObjectRef
     return result;
 }
 
+static JSValueRef uiElementAttributeValueCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
+{
+    JSStringRef attribute = 0;
+    if (argumentCount == 1)
+        attribute = JSValueToStringCopy(context, arguments[0], exception);
+    
+    return AccessibilityUIElement::makeJSAccessibilityUIElement(context, toAXElement(thisObject)->uiElementAttributeValue(attribute));
+}
+
 static JSValueRef numberAttributeValueCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
 {
     JSStringRef attribute = 0;
@@ -859,6 +868,7 @@ JSStringRef AccessibilityUIElement::rangeForLine(int line) { return 0; }
 void AccessibilityUIElement::setSelectedChild(AccessibilityUIElement*) const { }
 unsigned AccessibilityUIElement::selectedChildrenCount() const { return 0; }
 AccessibilityUIElement AccessibilityUIElement::selectedChildAtIndex(unsigned) const { return 0; }
+AccessibilityUIElement AccessibilityUIElement::uiElementAttributeValue(JSStringRef) const { return 0; }
 #endif
 
 #if !PLATFORM(WIN)
@@ -933,6 +943,9 @@ static void finalize(JSObjectRef thisObject)
 
 JSObjectRef AccessibilityUIElement::makeJSAccessibilityUIElement(JSContextRef context, const AccessibilityUIElement& element)
 {
+    if (!element)
+        return 0;
+    
     return JSObjectMake(context, AccessibilityUIElement::getJSClass(), new AccessibilityUIElement(element));
 }
 
@@ -1020,6 +1033,7 @@ JSClassRef AccessibilityUIElement::getJSClass()
         { "titleUIElement", titleUIElementCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "setSelectedTextRange", setSelectedTextRangeCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "stringAttributeValue", stringAttributeValueCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
+        { "uiElementAttributeValue", uiElementAttributeValueCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "numberAttributeValue", numberAttributeValueCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "boolAttributeValue", boolAttributeValueCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "isAttributeSupported", isAttributeSupportedCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
index 5cc7a171e1425bb2500355e3f2399297b6596c19..e8e813e6b412d247bce88f22595375a241017c6b 100644 (file)
@@ -105,6 +105,7 @@ public:
     // Attributes - platform-independent implementations
     JSStringRef stringAttributeValue(JSStringRef attribute);
     double numberAttributeValue(JSStringRef attribute);
+    AccessibilityUIElement uiElementAttributeValue(JSStringRef attribute);    
     bool boolAttributeValue(JSStringRef attribute);
     bool isAttributeSupported(JSStringRef attribute);
     bool isAttributeSettable(JSStringRef attribute);
index d76da7a96e74cd14c97b7e7523c83a4b5136c047..f24b420bd9511ba2e6dd62646886a46c5477344b 100644 (file)
@@ -480,6 +480,17 @@ JSStringRef AccessibilityUIElement::stringAttributeValue(JSStringRef attribute)
     return 0;
 }
 
+AccessibilityUIElement AccessibilityUIElement::uiElementAttributeValue(JSStringRef attribute)
+{
+    BEGIN_AX_OBJC_EXCEPTIONS
+    id uiElement = [m_element accessibilityAttributeValue:[NSString stringWithJSStringRef:attribute]];
+    return AccessibilityUIElement(uiElement);
+    END_AX_OBJC_EXCEPTIONS
+    
+    return 0;
+}
+
+
 double AccessibilityUIElement::numberAttributeValue(JSStringRef attribute)
 {
     BEGIN_AX_OBJC_EXCEPTIONS