Nullptr crash in WebCore::findPlaceForCounter with pseudo element that has display...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Feb 2020 21:40:58 +0000 (21:40 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Feb 2020 21:40:58 +0000 (21:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=207241

When the pseudo element's host element does not initiate a renderer
(e.g. display: contents) we need to look further in the DOM tree
for a previous-sibling-or-parent-element candidate.

Patch by Jack Lee <shihchieh_lee@apple.com> on 2020-02-06
Reviewed by Zalan Bujtas.

Source/WebCore:

Test: fast/css/counters/findPlaceForCounter-pseudo-element-display-content-host-crash.html

* rendering/RenderCounter.cpp:
(WebCore::previousSiblingOrParentElement):

LayoutTests:

* fast/css/counters/findPlaceForCounter-pseudo-element-display-content-host-crash-expected.txt: Added.
* fast/css/counters/findPlaceForCounter-pseudo-element-display-content-host-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/css/counters/findPlaceForCounter-pseudo-element-display-content-host-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/counters/findPlaceForCounter-pseudo-element-display-content-host-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderCounter.cpp

index 36c2e48..9c1b8c1 100644 (file)
@@ -1,3 +1,17 @@
+2020-02-06  Jack Lee  <shihchieh_lee@apple.com>
+
+        Nullptr crash in WebCore::findPlaceForCounter with pseudo element that has display:contents host.
+        https://bugs.webkit.org/show_bug.cgi?id=207241
+
+        When the pseudo element's host element does not initiate a renderer
+        (e.g. display: contents) we need to look further in the DOM tree 
+        for a previous-sibling-or-parent-element candidate.
+
+        Reviewed by Zalan Bujtas.
+
+        * fast/css/counters/findPlaceForCounter-pseudo-element-display-content-host-crash-expected.txt: Added.
+        * fast/css/counters/findPlaceForCounter-pseudo-element-display-content-host-crash.html: Added.
+
 2020-02-06  Sukolsak Sakshuwong  <sukolsak@gmail.com> and Alexey Shvayka  <shvaikalesh@gmail.com>
 
         JavaScript string corruption using RegExp with unicode character
diff --git a/LayoutTests/fast/css/counters/findPlaceForCounter-pseudo-element-display-content-host-crash-expected.txt b/LayoutTests/fast/css/counters/findPlaceForCounter-pseudo-element-display-content-host-crash-expected.txt
new file mode 100644 (file)
index 0000000..406ddba
--- /dev/null
@@ -0,0 +1 @@
+Tests CSS counter of a pseudo element that has display: contents host. The test passes if WebKit doesn't crash or hit an assertion.
diff --git a/LayoutTests/fast/css/counters/findPlaceForCounter-pseudo-element-display-content-host-crash.html b/LayoutTests/fast/css/counters/findPlaceForCounter-pseudo-element-display-content-host-crash.html
new file mode 100644 (file)
index 0000000..5f75842
--- /dev/null
@@ -0,0 +1,18 @@
+<style>
+html, body {
+  counter-reset: counter;
+}
+
+#outer {
+  display: contents;
+}
+
+#outer::before {
+  content: "text";
+}
+</style><span id=outer><span id=inner>Tests CSS counter of a pseudo element that has display: contents host. The test passes if WebKit doesn't crash or hit an assertion.</span></span><script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+    document.body.offsetHeight;
+    inner.style.counterIncrement = "counter";
+</script>
index 0ea5c05..366b3b3 100644 (file)
@@ -1,3 +1,19 @@
+2020-02-06  Jack Lee  <shihchieh_lee@apple.com>
+
+        Nullptr crash in WebCore::findPlaceForCounter with pseudo element that has display:contents host.
+        https://bugs.webkit.org/show_bug.cgi?id=207241
+
+        When the pseudo element's host element does not initiate a renderer
+        (e.g. display: contents) we need to look further in the DOM tree 
+        for a previous-sibling-or-parent-element candidate.
+
+        Reviewed by Zalan Bujtas.
+
+        Test: fast/css/counters/findPlaceForCounter-pseudo-element-display-content-host-crash.html
+
+        * rendering/RenderCounter.cpp:
+        (WebCore::previousSiblingOrParentElement):
+
 2020-02-06  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r255910, r255970, and r255972.
index fe46bd1..d3eb463 100644 (file)
@@ -75,23 +75,28 @@ static inline Element* parentOrPseudoHostElement(const RenderElement& renderer)
     return renderer.element() ? renderer.element()->parentElement() : nullptr;
 }
 
-static Element* previousSiblingOrParentElement(const Element* element)
+static Element* previousSiblingOrParentElement(const Element& element)
 {
-    auto* previous = ElementTraversal::pseudoAwarePreviousSibling(*element);
-    while (previous && !previous->renderer())
-        previous = ElementTraversal::pseudoAwarePreviousSibling(*previous);
-
-    if (previous)
-        return previous;
+    if (auto* previous = ElementTraversal::pseudoAwarePreviousSibling(element)) {
+        while (previous && !previous->renderer())
+            previous = ElementTraversal::pseudoAwarePreviousSibling(*previous);
 
-    auto* renderer = element->renderer();
-    if (renderer && renderer->isPseudoElement())
-        return renderer->generatingElement();
+        if (previous)
+            return previous;
+    }
 
-    previous = element->parentElement();
-    if (previous && !previous->renderer())
-        previous = previousSiblingOrParentElement(previous);
-    return previous;
+    if (is<PseudoElement>(element)) {
+        auto* hostElement = downcast<PseudoElement>(element).hostElement();
+        ASSERT(hostElement);
+        if (hostElement->renderer())
+            return hostElement;
+        return previousSiblingOrParentElement(*hostElement);
+    }
+    
+    auto* parent = element.parentElement();
+    if (parent && !parent->renderer())
+        parent = previousSiblingOrParentElement(*parent);
+    return parent;
 }
 
 // This function processes the renderer tree in the order of the DOM tree
@@ -100,7 +105,7 @@ static RenderElement* previousSiblingOrParent(const RenderElement& renderer)
 {
     ASSERT(renderer.element());
 
-    auto* previous = previousSiblingOrParentElement(renderer.element());
+    auto* previous = previousSiblingOrParentElement(*renderer.element());
     return previous ? previous->renderer() : nullptr;
 }