Attachment 103381 Details for Bug 65174 – patch

[patch] patch

patch.txt (text/plain), 7.88 KB, created by chris fleizach on 2011-08-09 11:56:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: chris fleizach

Created: 2011-08-09 11:56:42 PDT

Size: 7.88 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 92693)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,22 @@
>+2011-08-09  Chris Fleizach  <cfleizach@apple.com>
>+
>+        Accessibility stack exhaustion using role attribute
>+        https://bugs.webkit.org/show_bug.cgi?id=65174
>+
>+        Make sure we do not call accessibilityIsIgnored() during the AccessibilityObject
>+        creation flow. That can lead to loops.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Tests: accessibility/crash-determining-aria-role-when-label-present.html
>+
>+        * accessibility/AccessibilityObject.cpp:
>+        (WebCore::createARIARoleMap):
>+        * accessibility/AccessibilityRenderObject.cpp:
>+        (WebCore::AccessibilityRenderObject::remapAriaRoleDueToParent):
>+        (WebCore::AccessibilityRenderObject::determineAriaRoleAttribute):
>+        * accessibility/AccessibilityRenderObject.h:
>+
> 2011-08-09  Steve Block  <steveblock@google.com>
> 
>         JavaString is not needed by V8
>Index: Source/WebCore/accessibility/AccessibilityRenderObject.h
>===================================================================
>--- Source/WebCore/accessibility/AccessibilityRenderObject.h	(revision 92688)
>+++ Source/WebCore/accessibility/AccessibilityRenderObject.h	(working copy)
>@@ -284,7 +284,8 @@
>     Element* menuItemElementForMenu() const;
>     AccessibilityRole determineAccessibilityRole();
>     AccessibilityRole determineAriaRoleAttribute() const;
>-
>+    AccessibilityRole remapAriaRoleDueToParent(AccessibilityRole) const;
>+    
>     bool isTabItemSelected() const;
>     void alterSliderValue(bool increase);
>     void changeValueByStep(bool increase);
>Index: Source/WebCore/accessibility/AccessibilityRenderObject.cpp
>===================================================================
>--- Source/WebCore/accessibility/AccessibilityRenderObject.cpp	(revision 92688)
>+++ Source/WebCore/accessibility/AccessibilityRenderObject.cpp	(working copy)
>@@ -2985,6 +2985,35 @@
>     return 0;
> }
> 
>+AccessibilityRole AccessibilityRenderObject::remapAriaRoleDueToParent(AccessibilityRole role) const
>+{
>+    // Some objects change their role based on their parent.
>+    // However, asking for the unignoredParent calls accessibilityIsIgnored(), which can trigger a loop. While
>+    // inside the the call stack of creating an element, we need to avoid accessibilityIsIgnored().
>+    // https://bugs.webkit.org/show_bug.cgi?id=65174
>+
>+    if (role != ListBoxOptionRole && role != MenuItemRole)
>+        return role;
>+    
>+    AccessibilityObject* parent;
>+    for (parent = parentObject(); parent && !parent->accessibilityIsIgnored(); parent = parent->parentObject()) {
>+        AccessibilityRole parentAriaRole = parent->ariaRoleAttribute();
>+
>+        // Selects and listboxes both have options as child roles, but they map to different roles within WebCore.
>+        if (role == ListBoxOptionRole && parentAriaRole == MenuRole)
>+            return MenuItemRole;
>+        // An aria "menuitem" may map to MenuButton or MenuItem depending on its parent.
>+        else if (role == MenuItemRole && parentAriaRole == GroupRole)
>+            return MenuButtonRole;
>+        
>+        // If the parent had a different role, then we don't need to continue searching up the chain.
>+        if (parentAriaRole)
>+            break;
>+    }
>+    
>+    return role;
>+}
>+    
> AccessibilityRole AccessibilityRenderObject::determineAriaRoleAttribute() const
> {
>     const AtomicString& ariaRole = getAttribute(roleAttr);
>@@ -2998,30 +3027,11 @@
> 
>     if (role == TextAreaRole && !ariaIsMultiline())
>         role = TextFieldRole;
>+
>+    role = remapAriaRoleDueToParent(role);
>     
>     if (role)
>         return role;
>-
>-    AccessibilityObject* parentObject = parentObjectUnignored();
>-    if (!parentObject)
>-        return UnknownRole;
>-
>-    AccessibilityRole parentAriaRole = parentObject->ariaRoleAttribute();
>-
>-    // selects and listboxes both have options as child roles, but they map to different roles within WebCore
>-    if (equalIgnoringCase(ariaRole, "option")) {
>-        if (parentAriaRole == MenuRole)
>-            return MenuItemRole;
>-        if (parentAriaRole == ListBoxRole)
>-            return ListBoxOptionRole;
>-    }
>-    // an aria "menuitem" may map to MenuButton or MenuItem depending on its parent
>-    if (equalIgnoringCase(ariaRole, "menuitem")) {
>-        if (parentAriaRole == GroupRole)
>-            return MenuButtonRole;
>-        if (parentAriaRole == MenuRole || parentAriaRole == MenuBarRole)
>-            return MenuItemRole;
>-    }
>     
>     return UnknownRole;
> }
>Index: Source/WebCore/accessibility/AccessibilityObject.cpp
>===================================================================
>--- Source/WebCore/accessibility/AccessibilityObject.cpp	(revision 92688)
>+++ Source/WebCore/accessibility/AccessibilityObject.cpp	(working copy)
>@@ -1196,7 +1196,7 @@
>         { "math", DocumentMathRole },
>         { "menu", MenuRole },
>         { "menubar", MenuBarRole },
>-        // "menuitem" isn't here because it may map to different roles depending on the parent element's role
>+        { "menuitem", MenuItemRole },
>         { "menuitemcheckbox", MenuItemRole },
>         { "menuitemradio", MenuItemRole },
>         { "note", DocumentNoteRole },
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 92693)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2011-08-09  Chris Fleizach  <cfleizach@apple.com>
>+
>+        Accessibility stack exhaustion using role attribute
>+        https://bugs.webkit.org/show_bug.cgi?id=65174
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * accessibility/crash-determining-aria-role-when-label-present-expected.txt: Added.
>+        * accessibility/crash-determining-aria-role-when-label-present.html: Added.
>+
> 2011-08-09  Abhishek Arya  <inferno@chromium.org>
> 
>         Tests that we do not crash when trying to dirty line boxes on
>Index: LayoutTests/accessibility/crash-determining-aria-role-when-label-present-expected.txt
>===================================================================
>--- LayoutTests/accessibility/crash-determining-aria-role-when-label-present-expected.txt	(revision 0)
>+++ LayoutTests/accessibility/crash-determining-aria-role-when-label-present-expected.txt	(revision 0)
>@@ -0,0 +1,11 @@
>+
>+This tests a crashing scenario where an element with a role attribute is a child of a label that also has a corresponding control.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS input.childrenCount is 0
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>Index: LayoutTests/accessibility/crash-determining-aria-role-when-label-present.html
>===================================================================
>--- LayoutTests/accessibility/crash-determining-aria-role-when-label-present.html	(revision 0)
>+++ LayoutTests/accessibility/crash-determining-aria-role-when-label-present.html	(revision 0)
>@@ -0,0 +1,35 @@
>+<!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">
>+
>+<label><q role=x><input id="test">
>+
>+<p id="description"></p>
>+<div id="console"></div>
>+
>+<script>
>+
>+    description("This tests a crashing scenario where an element with a role attribute is a child of a label that also has a corresponding control.");
>+
>+    if (window.accessibilityController) {
>+          document.getElementById("test").focus();
>+         
>+          // This line should not crash.
>+          var input = accessibilityController.focusedElement;
>+
>+          shouldBe("input.childrenCount", "0");
>+    }
>+
>+    successfullyParsed = true;
>+</script>
>+
>+<script src="../fast/js/resources/js-test-post.js"></script>
>+</body>
>+</html>

Flags:

darin: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 65174: 101987 | 103381