Attachment 264660 Details for Bug 150670 – Patch

[patch] Patch

bug-150670-20151102234159.patch (text/plain), 5.67 KB, created by Joanmarie Diggs on 2015-11-02 20:44:10 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Joanmarie Diggs

Created: 2015-11-02 20:44:10 PST

Size: 5.67 KB

patch

obsolete

>Subversion Revision: 191887
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f802f9a83c1f2d8ca810b1a5f3580cecb404430e..e5480c3cd92605072d532678694231d0817c0f35 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2015-11-02  Joanmarie Diggs  <jdiggs@igalia.com>
>+
>+        [AX] WebProcess from WebKitGtk+ 2.10.0 compiled in Debug mode hits ASSERT on textUnderElement
>+        https://bugs.webkit.org/show_bug.cgi?id=150670
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        RenderText::styleDidChange emits platform notifications which can cause
>+        a new accessible object to be created. In the case of GTK and EFL, the
>+        creation of an accessible object for a list item with pseudo element list
>+        marker content causes textUnderElement to be called as part of the layout-
>+        list heuristic. But textUnderElement ASSERTs that the document has no
>+        children in need of a style recalculation. Using stringValue instead to
>+        determine if a list item marker with pseudo element content contains
>+        rendered text avoids triggering the assertion in question.
>+
>+        Test: accessibility/gtk/list-item-with-pseudo-element-crash.html
>+
>+        * accessibility/AccessibilityList.cpp:
>+        (WebCore::AccessibilityList::childHasPseudoVisibleListItemMarkers):
>+
> 2015-11-02  Youenn Fablet  <youenn.fablet@crf.canon.fr>
> 
>         Rename JSDOMWrapper.impl to JSDOMWrapper.wrapped
>diff --git a/Source/WebCore/accessibility/AccessibilityList.cpp b/Source/WebCore/accessibility/AccessibilityList.cpp
>index c78d364867ee84b66ba87bbf3b498e5186c9b21f..3eddfb49cc9e8669c7b4e95eb29eb75a819ccb60 100644
>--- a/Source/WebCore/accessibility/AccessibilityList.cpp
>+++ b/Source/WebCore/accessibility/AccessibilityList.cpp
>@@ -118,13 +118,21 @@ bool AccessibilityList::childHasPseudoVisibleListItemMarkers(RenderObject* listI
>     }
>     
>     // Platforms which expose rendered text content through the parent element will treat
>-    // those renderers as "ignored" objects.
>+    // those renderers as "ignored" objects, so we need to examine the accessible rendered
>+    // text. AccessibilityNodeObject::textUnderElement() currently asserts that the document
>+    // cannot need a layout update or have children in need of a style recalculation. The
>+    // latter condition can occur if the style of a list item with a "before" pseudo element
>+    // is modified (see RenderText::styleDidChange). We don't actually care here what the text
>+    // under the pseudo element is; just that such text is present. So we'll work around it by
>+    // calling stringValue() on the children of the pseudo element's RenderObject.
> #if PLATFORM(GTK) || PLATFORM(EFL)
>-    String text = axObj->textUnderElement();
>-    return !text.isEmpty() && !text.containsOnlyWhitespace();
>-#else
>-    return false;
>+    for (RenderObject* child = axObj->renderer()->firstChildSlow(); child; child = child->nextSibling()) {
>+        String text = axObjectCache()->getOrCreate(child)->stringValue();
>+        if (!text.isEmpty() && !text.containsOnlyWhitespace())
>+            return true;
>+    }
> #endif
>+    return false;
> }
>     
> AccessibilityRole AccessibilityList::determineAccessibilityRole()
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index a267db6614ddcb9c064228afed127f65357ffe17..4e238c54f0fa760428e8ca4ddd6109baa9d7edaf 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2015-11-02  Joanmarie Diggs  <jdiggs@igalia.com>
>+
>+        [AX] WebProcess from WebKitGtk+ 2.10.0 compiled in Debug mode hits ASSERT on textUnderElement
>+        https://bugs.webkit.org/show_bug.cgi?id=150670
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * accessibility/gtk/list-item-with-pseudo-element-crash-expected.txt: Added.
>+        * accessibility/gtk/list-item-with-pseudo-element-crash.html: Added.
>+
> 2015-10-29  Sergio Villar Senin  <svillar@igalia.com>
> 
>         [CSS Grid Layout] min-content row does not always shrink
>diff --git a/LayoutTests/accessibility/gtk/list-item-with-pseudo-element-crash-expected.txt b/LayoutTests/accessibility/gtk/list-item-with-pseudo-element-crash-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..4adff0659774a608440b32a83faf3575da0d484e
>--- /dev/null
>+++ b/LayoutTests/accessibility/gtk/list-item-with-pseudo-element-crash-expected.txt
>@@ -0,0 +1,11 @@
>+Foo
>+BAR
>+This verifies that list items with pseudo elements and styles do not trigger an assertion.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/accessibility/gtk/list-item-with-pseudo-element-crash.html b/LayoutTests/accessibility/gtk/list-item-with-pseudo-element-crash.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..233b4f14c75028bcdc3e39f957abd076c92f3470
>--- /dev/null
>+++ b/LayoutTests/accessibility/gtk/list-item-with-pseudo-element-crash.html
>@@ -0,0 +1,26 @@
>+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
>+<html>
>+<head>
>+<script src="../../resources/js-test-pre.js"></script>
>+<style>
>+.item:before {content:""; }
>+</style>
>+</head>
>+<body>
>+<ul id="list" style="display:none;">
>+<li id="item1" class="item" style="float:left;">Foo</li>
>+<li id="item2" class="item" style="text-transform:uppercase;">Bar</li>
>+</ul>
>+<p id="description"></p>
>+<div id="console"></div>
>+<script>
>+description("This verifies that list items with pseudo elements and styles do not trigger an assertion.");
>+
>+if (window.testRunner && window.accessibilityController) {
>+    var list = document.getElementById("list");
>+    list.style.display = "";
>+}
>+</script>
>+<script src="../../resources/js-test-post.js"></script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 150670: 264315 | 264346 | 264387 | 264603 | 264660 | 264790 | 264796