Attachment 361484 Details for Bug 194423 – Patch

[patch] Patch

bug-194423-20190207183527.patch (text/plain), 7.94 KB, created by Devin Rousso on 2019-02-07 18:35:28 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Devin Rousso

Created: 2019-02-07 18:35:28 PST

Size: 7.94 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 6922c0089356d3fc32c051f8ea44db9e1a064fe3..e1e6f3a6788588aaff147ee0e7b07cb9277ac7a3 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-02-07  Devin Rousso  <drousso@apple.com>
>+
>+        PseudoElement created for any ::before/::after selector regardless of whether a content property exists
>+        https://bugs.webkit.org/show_bug.cgi?id=194423
>+        <rdar://problem/46787260>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test: inspector/css/pseudo-creation.html
>+
>+        * style/StyleTreeResolver.cpp:
>+        (WebCore::Style::TreeResolver::resolvePseudoStyle):
>+        We should only be creating `PseudoElement`s if we actually have a `content` proprety in the
>+        `PseudoElement`'s style. Otherwise, we end up creating `PseudoElement`s for every CSS rule
>+        that has a `::before`/`::after`, only to immediately destroy them as there is nothing to show.
>+
> 2019-02-07  Youenn Fablet  <youenn@apple.com>
> 
>         Make to clear sources from UserMediaCaptureManagerProxy and UserMediaCaptureManager when no longer needed
>diff --git a/Source/WebCore/style/StyleTreeResolver.cpp b/Source/WebCore/style/StyleTreeResolver.cpp
>index 3d57b267af76e81697ac16bc42bbaa67a97c8a42..bb676cb0350e8158bbb9b2056ee6f30b93f26658 100644
>--- a/Source/WebCore/style/StyleTreeResolver.cpp
>+++ b/Source/WebCore/style/StyleTreeResolver.cpp
>@@ -43,6 +43,7 @@
> #include "Page.h"
> #include "PlatformStrategies.h"
> #include "RenderElement.h"
>+#include "RenderStyle.h"
> #include "RenderView.h"
> #include "RuntimeEnabledFeatures.h"
> #include "Settings.h"
>@@ -252,7 +253,7 @@ ElementUpdate TreeResolver::resolvePseudoStyle(Element& element, const ElementUp
>         return { };
> 
>     auto pseudoStyle = scope().styleResolver.pseudoStyleForElement(element, { pseudoId }, *elementUpdate.style, &scope().selectorFilter);
>-    if (!pseudoStyle)
>+    if (!pseudoElementRendererIsNeeded(pseudoStyle.get()))
>         return { };
> 
>     PseudoElement* pseudoElement = pseudoId == PseudoId::Before ? element.beforePseudoElement() : element.afterPseudoElement();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index afd05f7fd002652ae992b208a46acdfc65e451ed..8f3bc90e1477a476801593d61f09b1a123e030f7 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-02-07  Devin Rousso  <drousso@apple.com>
>+
>+        PseudoElement created for any ::before/::after selector regardless of whether a content property exists
>+        https://bugs.webkit.org/show_bug.cgi?id=194423
>+        <rdar://problem/46787260>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * inspector/css/pseudo-creation.html: Added.
>+        * inspector/css/pseudo-creation-expected.txt: Added.
>+
> 2019-02-07  Alex Christensen  <achristensen@webkit.org>
> 
>         Deprecate WKBundlePageSetDefersLoading
>diff --git a/LayoutTests/inspector/css/pseudo-creation-expected.txt b/LayoutTests/inspector/css/pseudo-creation-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..5b8323349a128ba663bdc878b4a62ed6423d8172
>--- /dev/null
>+++ b/LayoutTests/inspector/css/pseudo-creation-expected.txt
>@@ -0,0 +1,22 @@
>+Testing when CSS.events.pseudoElementCreated and CSS.events.pseudoElementDestroyed are fired.
>+
>+Requesting document...
>+
>+Calling "createElementWithClass("test-pseudo-without-content")"...
>+Checking for nodes with class ".test-pseudo-without-content"...
>+PASS: There should be 1 node with the class ".test-pseudo-without-content".
>+
>+Calling "removeElementWithClass("test-pseudo-without-content")"...
>+Checking for nodes with class ".test-pseudo-without-content"...
>+PASS: There should be 0 node with the class ".test-pseudo-without-content".
>+
>+Calling "createElementWithClass("test-pseudo-with-content")"...
>+Checking for nodes with class ".test-pseudo-with-content"...
>+PASS: Created ::before pseudo element
>+PASS: There should be 1 node with the class ".test-pseudo-with-content".
>+
>+Calling "removeElementWithClass("test-pseudo-with-content")"...
>+Checking for nodes with class ".test-pseudo-with-content"...
>+PASS: Removed ::before pseudo element
>+PASS: There should be 0 node with the class ".test-pseudo-with-content".
>+
>diff --git a/LayoutTests/inspector/css/pseudo-creation.html b/LayoutTests/inspector/css/pseudo-creation.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..d4632a3a90fad18c4280c0c962cced52dad981e6
>--- /dev/null
>+++ b/LayoutTests/inspector/css/pseudo-creation.html
>@@ -0,0 +1,104 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src="../../http/tests/inspector/resources/protocol-test.js"></script>
>+<style>
>+.test-pseudo-without-content::before {
>+    color: red;
>+}
>+.test-pseudo-with-content::before {
>+    content: "TEST";
>+}
>+</style>
>+<script>
>+
>+function createElementWithClass(className) {
>+    let element = document.body.appendChild(document.createElement("div"));
>+    element.classList.add(className);
>+}
>+
>+function removeElementWithClass(className) {
>+    let element = document.querySelector(`.${className}`);
>+    element.classList.remove(className);
>+
>+    // Don't remove the element, as that removes the entire subtree rather than just the pseudo element.
>+}
>+
>+function test() {
>+    ProtocolTest.debug();
>+
>+    let documentNode = null;
>+    let pseudoElement = null;
>+
>+    function handlePromiseReject(error) {
>+        console.log(error);
>+        ProtocolTest.log(error);
>+        ProtocolTest.completeTest();
>+    }
>+
>+    function evaluateWithLog(expression) {
>+        ProtocolTest.log("");
>+        ProtocolTest.log(`Calling "${expression}"...`);
>+        return ProtocolTest.evaluateInPage(expression)
>+        .catch(handlePromiseReject);
>+    }
>+
>+    function checkElementsWithClass(className, expectedCount) {
>+        ProtocolTest.log(`Checking for nodes with class ".${className}"...`);
>+        return InspectorProtocol.awaitCommand({
>+            method: "DOM.querySelectorAll",
>+            params: {
>+                nodeId: documentNode.nodeId,
>+                selector: `.${className}`,
>+            },
>+        })
>+        .then((result) => {
>+            ProtocolTest.expectEqual(result.nodeIds.length, expectedCount, `There should be ${expectedCount} node with the class ".${className}".`);
>+        })
>+        .catch(handlePromiseReject);
>+    }
>+
>+    function createElementWithClass(className) {
>+        return evaluateWithLog(`createElementWithClass("${className}")`)
>+        .then(() => checkElementsWithClass(className, 1))
>+        .catch(handlePromiseReject);
>+    }
>+
>+    function removeElementWithClass(className) {
>+        return evaluateWithLog(`removeElementWithClass("${className}")`)
>+        .then(() => checkElementsWithClass(className, 0))
>+        .catch(handlePromiseReject);
>+    }
>+
>+    InspectorProtocol.eventHandler["DOM.pseudoElementAdded"] = (response) => {
>+        pseudoElement = response.params.pseudoElement;
>+
>+        ProtocolTest.pass(`Created ::${pseudoElement.pseudoType} pseudo element`);
>+    };
>+
>+    InspectorProtocol.eventHandler["DOM.pseudoElementRemoved"] = (response) => {
>+        ProtocolTest.expectEqual(response.params.pseudoElementId, pseudoElement.nodeId, `Removed ::${pseudoElement.pseudoType} pseudo element`);
>+    };
>+
>+    ProtocolTest.log("Requesting document...");
>+    InspectorProtocol.sendCommand("DOM.getDocument", {}, (response) => {
>+        InspectorProtocol.checkForError(response);
>+
>+        documentNode = response.result.root;
>+
>+        Promise.resolve()
>+        .then(() => createElementWithClass("test-pseudo-without-content"))
>+        .then(() => removeElementWithClass("test-pseudo-without-content"))
>+        .then(() => createElementWithClass("test-pseudo-with-content"))
>+        .then(() => removeElementWithClass("test-pseudo-with-content"))
>+        .then(() => ProtocolTest.completeTest())
>+        .catch(handlePromiseReject);
>+    });
>+}
>+
>+</script>
>+</head>
>+<body onload="runTest()">
>+    <p>Testing when CSS.events.pseudoElementCreated and CSS.events.pseudoElementDestroyed are fired.</p>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 194423: 361484