A selector should not match anything if there is a subselector after a non-scrollbar...
authorbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Dec 2014 23:56:20 +0000 (23:56 +0000)
committerbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Dec 2014 23:56:20 +0000 (23:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=139336
Source/WebCore:

rdar://problem/19051623

Patch by Benjamin Poulain <bpoulain@apple.com> on 2014-12-08
Reviewed by Andreas Kling.

Tests: fast/css/duplicated-after-pseudo-element.html
       fast/css/duplicated-before-pseudo-element.html
       fast/css/simple-selector-after-pseudo-element.html

* cssjit/SelectorCompiler.cpp:
(WebCore::SelectorCompiler::constructFragments):
The code filtering out simple selectors was only considering
the relation CSSSelector::SubSelector. That comes from SelectorChecker where
the relation considered is the one from the previous selector.

In this case, the relation is the extracted from the current simple selector,
which is the relation with the following selector.

When a single simple selector was following a pseudo element, the relation evaluated
to descendant/adjacent/direct-adjacent and we were skipping the early return.
That simple selector was evaluated as a regular filter on the element.

In the CSS JIT, we can just remove that test altogether. Fragments are built one after
the other. By definition, the evaluated simple selector belong to the current fragment.

LayoutTests:

Patch by Benjamin Poulain <bpoulain@apple.com> on 2014-12-08
Reviewed by Andreas Kling.

* fast/css/duplicated-after-pseudo-element-expected.html: Added.
* fast/css/duplicated-after-pseudo-element.html: Added.
* fast/css/duplicated-before-pseudo-element-expected.html: Added.
* fast/css/duplicated-before-pseudo-element.html: Added.
* fast/css/simple-selector-after-pseudo-element-expected.html: Added.
* fast/css/simple-selector-after-pseudo-element.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/css/duplicated-after-pseudo-element-expected.html [new file with mode: 0644]
LayoutTests/fast/css/duplicated-after-pseudo-element.html [new file with mode: 0644]
LayoutTests/fast/css/duplicated-before-pseudo-element-expected.html [new file with mode: 0644]
LayoutTests/fast/css/duplicated-before-pseudo-element.html [new file with mode: 0644]
LayoutTests/fast/css/simple-selector-after-pseudo-element-expected.html [new file with mode: 0644]
LayoutTests/fast/css/simple-selector-after-pseudo-element.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/cssjit/SelectorCompiler.cpp

index 278b769..e9a8140 100644 (file)
@@ -1,3 +1,17 @@
+2014-12-08  Benjamin Poulain  <bpoulain@apple.com>
+
+        A selector should not match anything if there is a subselector after a non-scrollbar pseudo element
+        https://bugs.webkit.org/show_bug.cgi?id=139336
+
+        Reviewed by Andreas Kling.
+
+        * fast/css/duplicated-after-pseudo-element-expected.html: Added.
+        * fast/css/duplicated-after-pseudo-element.html: Added.
+        * fast/css/duplicated-before-pseudo-element-expected.html: Added.
+        * fast/css/duplicated-before-pseudo-element.html: Added.
+        * fast/css/simple-selector-after-pseudo-element-expected.html: Added.
+        * fast/css/simple-selector-after-pseudo-element.html: Added.
+
 2014-12-08  Benjamin Poulain  <benjamin@webkit.org>
 
         Fix the parsing of advanced :lang() after r176902
diff --git a/LayoutTests/fast/css/duplicated-after-pseudo-element-expected.html b/LayoutTests/fast/css/duplicated-after-pseudo-element-expected.html
new file mode 100644 (file)
index 0000000..a96fcb1
--- /dev/null
@@ -0,0 +1,16 @@
+<!doctype html>
+<html>
+<head>
+    <style>
+        target::after {
+            content:"WebKit!";
+            color: white;
+            background-color: green;
+        }
+    </style>
+</head>
+<body>
+    <p>Verify that selectors with duplicated ::after pseudo elements never match. If the test pass, you should see a box with a green background an the test "WebKit!" written in white.</p>
+    <target></target>
+</body>
+</html>
diff --git a/LayoutTests/fast/css/duplicated-after-pseudo-element.html b/LayoutTests/fast/css/duplicated-after-pseudo-element.html
new file mode 100644 (file)
index 0000000..48499cf
--- /dev/null
@@ -0,0 +1,20 @@
+<!doctype html>
+<html>
+<head>
+    <style>
+        target::after {
+            content:"WebKit!";
+            color: white;
+            background-color: green;
+        }
+        target::after::after, ::after::after, target::after::after::after, ::after::after::after {
+            background-color: red;
+            color: black;
+        }
+    </style>
+</head>
+<body>
+    <p>Verify that selectors with duplicated ::after pseudo elements never match. If the test pass, you should see a box with a green background an the test "WebKit!" written in white.</p>
+    <target></target>
+</body>
+</html>
diff --git a/LayoutTests/fast/css/duplicated-before-pseudo-element-expected.html b/LayoutTests/fast/css/duplicated-before-pseudo-element-expected.html
new file mode 100644 (file)
index 0000000..b1c9639
--- /dev/null
@@ -0,0 +1,16 @@
+<!doctype html>
+<html>
+<head>
+    <style>
+        target::before {
+            content:"WebKit!";
+            color: white;
+            background-color: green;
+        }
+    </style>
+</head>
+<body>
+    <p>Verify that selectors with duplicated ::before pseudo elements never match. If the test pass, you should see a box with a green background an the test "WebKit!" written in white.</p>
+    <target></target>
+</body>
+</html>
diff --git a/LayoutTests/fast/css/duplicated-before-pseudo-element.html b/LayoutTests/fast/css/duplicated-before-pseudo-element.html
new file mode 100644 (file)
index 0000000..3c49364
--- /dev/null
@@ -0,0 +1,20 @@
+<!doctype html>
+<html>
+<head>
+    <style>
+        target::before {
+            content:"WebKit!";
+            color: white;
+            background-color: green;
+        }
+        target::before::before, ::before::before, target::before::before::before, ::before::before::before {
+            background-color: red;
+            color: black;
+        }
+    </style>
+</head>
+<body>
+    <p>Verify that selectors with duplicated ::before pseudo elements never match. If the test pass, you should see a box with a green background an the test "WebKit!" written in white.</p>
+    <target></target>
+</body>
+</html>
diff --git a/LayoutTests/fast/css/simple-selector-after-pseudo-element-expected.html b/LayoutTests/fast/css/simple-selector-after-pseudo-element-expected.html
new file mode 100644 (file)
index 0000000..d754b50
--- /dev/null
@@ -0,0 +1,20 @@
+<!doctype html>
+<html>
+<head>
+    <style>
+        .target {
+            display: block;
+        }
+        target1::before, target2 {
+            content:"WebKit!";
+            color: white;
+            background-color: green;
+        }
+    </style>
+</head>
+<body>
+    <p>Verify that selectors with duplicated ::before pseudo elements never match. If the test pass, you should see a box with a green background an the test "WebKit!" written in white.</p>
+    <target1 class="target" id="target"></target1>
+    <target2 class="target" id="target"></target2>
+</body>
+</html>
diff --git a/LayoutTests/fast/css/simple-selector-after-pseudo-element.html b/LayoutTests/fast/css/simple-selector-after-pseudo-element.html
new file mode 100644 (file)
index 0000000..174cf75
--- /dev/null
@@ -0,0 +1,31 @@
+<!doctype html>
+<html>
+<head>
+    <style>
+        .target {
+            display: block;
+        }
+        target1::before, target2 {
+            content:"WebKit!";
+            color: white;
+            background-color: green;
+        }
+        .target::before:nth-child(n),
+        .target::before.target,
+        .target::before#target,
+        .target::before::after,
+        .target::after:nth-child(n),
+        .target::after.target,
+        .target::after#target,
+        .target::after::before {
+            background-color: red;
+            color: black;
+        }
+    </style>
+</head>
+<body>
+    <p>Verify that selectors with duplicated ::before pseudo elements never match. If the test pass, you should see a box with a green background an the test "WebKit!" written in white.</p>
+    <target1 class="target" id="target"></target1>
+    <target2 class="target" id="target"></target2>
+</body>
+</html>
index 040d16d..5c23f2c 100644 (file)
@@ -1,3 +1,31 @@
+2014-12-08  Benjamin Poulain  <bpoulain@apple.com>
+
+        A selector should not match anything if there is a subselector after a non-scrollbar pseudo element
+        https://bugs.webkit.org/show_bug.cgi?id=139336
+        rdar://problem/19051623
+
+        Reviewed by Andreas Kling.
+
+        Tests: fast/css/duplicated-after-pseudo-element.html
+               fast/css/duplicated-before-pseudo-element.html
+               fast/css/simple-selector-after-pseudo-element.html
+
+        * cssjit/SelectorCompiler.cpp:
+        (WebCore::SelectorCompiler::constructFragments):
+        The code filtering out simple selectors was only considering
+        the relation CSSSelector::SubSelector. That comes from SelectorChecker where
+        the relation considered is the one from the previous selector.
+
+        In this case, the relation is the extracted from the current simple selector,
+        which is the relation with the following selector.
+
+        When a single simple selector was following a pseudo element, the relation evaluated
+        to descendant/adjacent/direct-adjacent and we were skipping the early return.
+        That simple selector was evaluated as a regular filter on the element.
+
+        In the CSS JIT, we can just remove that test altogether. Fragments are built one after
+        the other. By definition, the evaluated simple selector belong to the current fragment.
+
 2014-12-08  Benjamin Poulain  <benjamin@webkit.org>
 
         Fix the parsing of advanced :lang() after r176902
index 87012b7..24f55cd 100644 (file)
@@ -866,16 +866,11 @@ static FunctionType constructFragments(const CSSSelector* rootSelector, Selector
     for (const CSSSelector* selector = rootSelector; selector; selector = selector->tagHistory()) {
         specificity = CSSSelector::addSpecificities(specificity, selector->simpleSelectorSpecificity());
 
-        CSSSelector::Relation relation = selector->relation();
-
         // A selector is invalid if something follows a pseudo-element.
         // We make an exception for scrollbar pseudo elements and allow a set of pseudo classes (but nothing else)
         // to follow the pseudo elements.
-        if (relation == CSSSelector::SubSelector
-            && fragment.pseudoElementSelector
-            && !isScrollbarPseudoElement(fragment.pseudoElementSelector->pseudoElementType())) {
+        if (fragment.pseudoElementSelector && !isScrollbarPseudoElement(fragment.pseudoElementSelector->pseudoElementType()))
             return FunctionType::CannotMatchAnything;
-        }
 
         switch (selector->match()) {
         case CSSSelector::Tag:
@@ -933,6 +928,7 @@ static FunctionType constructFragments(const CSSSelector* rootSelector, Selector
             case CSSSelector::PseudoElementScrollbarThumb:
             case CSSSelector::PseudoElementScrollbarTrack:
             case CSSSelector::PseudoElementScrollbarTrackPiece:
+                ASSERT(!fragment.pseudoElementSelector);
                 fragment.pseudoElementSelector = selector;
                 break;
             case CSSSelector::PseudoElementUnknown:
@@ -979,6 +975,7 @@ static FunctionType constructFragments(const CSSSelector* rootSelector, Selector
             return FunctionType::CannotMatchAnything;
         }
 
+        CSSSelector::Relation relation = selector->relation();
         if (relation == CSSSelector::SubSelector)
             continue;