Title: [94864] trunk
Revision
94864
Author
[email protected]
Date
2011-09-09 11:06:22 -0700 (Fri, 09 Sep 2011)

Log Message

Source/WebCore: Assert being hit in AccessibilityRenderObject::addChildren()
https://bugs.webkit.org/show_bug.cgi?id=61805

Patch by Dominic Mazzoni <[email protected]> on 2011-09-09
Reviewed by Chris Fleizach.

Fix nextSibling and previousSibling to handle adjacent continuations
properly, otherwise nodes end up appearing in the accessibility
tree twice (or a debug assertion could be raised).

Test: accessibility/adjacent-continuations-cause-assertion-failure.html

* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::previousSibling):
(WebCore::AccessibilityRenderObject::nextSibling):

LayoutTests: Add a test to catch a case where adjacent continuations were
causing nodes to get added to the accessibility tree twice, leading
to an assertion failure or a crash. The test expectations are
currently Mac-specific, so added the test to the Skipped file for
gtk and win.
https://bugs.webkit.org/show_bug.cgi?id=61805

Patch by Dominic Mazzoni <[email protected]> on 2011-09-09
Reviewed by Chris Fleizach.

* accessibility/adjacent-continuations-cause-assertion-failure.html: Added.
* platform/mac/accessibility/adjacent-continuations-cause-assertion-failure-expected.txt: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (94863 => 94864)


--- trunk/LayoutTests/ChangeLog	2011-09-09 18:00:12 UTC (rev 94863)
+++ trunk/LayoutTests/ChangeLog	2011-09-09 18:06:22 UTC (rev 94864)
@@ -1,3 +1,17 @@
+2011-09-09  Dominic Mazzoni  <[email protected]>
+
+        Add a test to catch a case where adjacent continuations were
+        causing nodes to get added to the accessibility tree twice, leading
+        to an assertion failure or a crash. The test expectations are
+        currently Mac-specific, so added the test to the Skipped file for
+        gtk and win.
+        https://bugs.webkit.org/show_bug.cgi?id=61805
+
+        Reviewed by Chris Fleizach.
+
+        * accessibility/adjacent-continuations-cause-assertion-failure.html: Added.
+        * platform/mac/accessibility/adjacent-continuations-cause-assertion-failure-expected.txt: Added.
+
 2011-09-08  Kentaro Hara  <[email protected]>
 
         Implement a WebKitAnimationEvent constructor.

Added: trunk/LayoutTests/accessibility/adjacent-continuations-cause-assertion-failure.html (0 => 94864)


--- trunk/LayoutTests/accessibility/adjacent-continuations-cause-assertion-failure.html	                        (rev 0)
+++ trunk/LayoutTests/accessibility/adjacent-continuations-cause-assertion-failure.html	2011-09-09 18:06:22 UTC (rev 94864)
@@ -0,0 +1,55 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href=""
+<script>
+    var successfullyParsed = false;
+
+    function buildAccessibilityTree(accessibilityObject, indent) {
+        var str = "";
+        for (var i = 0; i < indent; i++)
+            str += "    ";
+        str += accessibilityObject.role;
+        str += " " + accessibilityObject.stringValue;
+        str += "\n";
+        document.getElementById("tree").innerText += str;
+
+        if (accessibilityObject.stringValue.indexOf('End of test') >= 0)
+            return false;
+
+        var count = accessibilityObject.childrenCount;
+        for (var i = 0; i < count; ++i) {
+            if (!buildAccessibilityTree(accessibilityObject.childAtIndex(i), indent + 1))
+                return false;
+        }
+
+        return true;
+    }
+</script>
+<script src=""
+</head>
+<body>
+
+<span><div></div></span><span>x<div>y</div>z</span>
+
+<div>End of test</div>
+
+<p id="description"></p>
+<pre id="tree"></pre>
+<div id="console"></div>
+
+<script>
+    description("Make sure that a debug assert is not triggered when constructing the accessibility tree for this page.");
+
+    if (window.accessibilityController) {
+        // Build the accessibility tree up until 'End of test' is encountered.
+        document.body.focus();
+        buildAccessibilityTree(accessibilityController.focusedElement, 0);
+    }
+
+    successfullyParsed = true;
+</script>
+
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/platform/mac/accessibility/adjacent-continuations-cause-assertion-failure-expected.txt (0 => 94864)


--- trunk/LayoutTests/platform/mac/accessibility/adjacent-continuations-cause-assertion-failure-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/accessibility/adjacent-continuations-cause-assertion-failure-expected.txt	2011-09-09 18:06:22 UTC (rev 94864)
@@ -0,0 +1,20 @@
+x
+y
+z
+End of test
+Make sure that a debug assert is not triggered when constructing the accessibility tree for this page.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+AXRole: AXWebArea AXValue: 
+    AXRole: AXStaticText AXValue: x
+    AXRole: AXGroup AXValue: 
+        AXRole: AXStaticText AXValue: y
+    AXRole: AXStaticText AXValue: z
+    AXRole: AXGroup AXValue: 
+        AXRole: AXStaticText AXValue: End of test
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Modified: trunk/Source/WebCore/ChangeLog (94863 => 94864)


--- trunk/Source/WebCore/ChangeLog	2011-09-09 18:00:12 UTC (rev 94863)
+++ trunk/Source/WebCore/ChangeLog	2011-09-09 18:06:22 UTC (rev 94864)
@@ -1,3 +1,20 @@
+2011-09-09  Dominic Mazzoni  <[email protected]>
+
+        Assert being hit in AccessibilityRenderObject::addChildren()
+        https://bugs.webkit.org/show_bug.cgi?id=61805
+
+        Reviewed by Chris Fleizach.
+
+        Fix nextSibling and previousSibling to handle adjacent continuations
+        properly, otherwise nodes end up appearing in the accessibility
+        tree twice (or a debug assertion could be raised).
+
+        Test: accessibility/adjacent-continuations-cause-assertion-failure.html
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::previousSibling):
+        (WebCore::AccessibilityRenderObject::nextSibling):
+
 2011-09-08  Kentaro Hara  <[email protected]>
 
         Implement a WebKitAnimationEvent constructor.

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (94863 => 94864)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2011-09-09 18:00:12 UTC (rev 94863)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2011-09-09 18:06:22 UTC (rev 94864)
@@ -305,8 +305,12 @@
 
     // Case 2: Anonymous block parent of the end of a continuation - skip all the way to before
     // the parent of the start, since everything in between will be linked up via the continuation.
-    else if (m_renderer->isAnonymousBlock() && firstChildIsInlineContinuation(m_renderer))
-        previousSibling = startOfContinuations(m_renderer->firstChild())->parent()->previousSibling();
+    else if (m_renderer->isAnonymousBlock() && firstChildIsInlineContinuation(m_renderer)) {
+        RenderObject* firstParent = startOfContinuations(m_renderer->firstChild())->parent();
+        while (firstChildIsInlineContinuation(firstParent))
+            firstParent = startOfContinuations(firstParent->firstChild())->parent();
+        previousSibling = firstParent->previousSibling();
+    }
 
     // Case 3: The node has an actual previous sibling
     else if (RenderObject* ps = m_renderer->previousSibling())
@@ -343,8 +347,12 @@
 
     // Case 2: Anonymous block parent of the start of a continuation - skip all the way to
     // after the parent of the end, since everything in between will be linked up via the continuation.
-    else if (m_renderer->isAnonymousBlock() && lastChildHasContinuation(m_renderer))
-        nextSibling = endOfContinuations(m_renderer->lastChild())->parent()->nextSibling();
+    else if (m_renderer->isAnonymousBlock() && lastChildHasContinuation(m_renderer)) {
+        RenderObject* lastParent = endOfContinuations(m_renderer->lastChild())->parent();
+        while (lastChildHasContinuation(lastParent))
+            lastParent = endOfContinuations(lastParent->lastChild())->parent();
+        nextSibling = lastParent->nextSibling();
+    }
 
     // Case 3: node has an actual next sibling
     else if (RenderObject* ns = m_renderer->nextSibling())
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to