<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[176984] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/176984">176984</a></dd>
<dt>Author</dt> <dd>benjamin@webkit.org</dd>
<dt>Date</dt> <dd>2014-12-08 15:56:20 -0800 (Mon, 08 Dec 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>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
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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorecssjitSelectorCompilercpp">trunk/Source/WebCore/cssjit/SelectorCompiler.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastcssduplicatedafterpseudoelementexpectedhtml">trunk/LayoutTests/fast/css/duplicated-after-pseudo-element-expected.html</a></li>
<li><a href="#trunkLayoutTestsfastcssduplicatedafterpseudoelementhtml">trunk/LayoutTests/fast/css/duplicated-after-pseudo-element.html</a></li>
<li><a href="#trunkLayoutTestsfastcssduplicatedbeforepseudoelementexpectedhtml">trunk/LayoutTests/fast/css/duplicated-before-pseudo-element-expected.html</a></li>
<li><a href="#trunkLayoutTestsfastcssduplicatedbeforepseudoelementhtml">trunk/LayoutTests/fast/css/duplicated-before-pseudo-element.html</a></li>
<li><a href="#trunkLayoutTestsfastcsssimpleselectorafterpseudoelementexpectedhtml">trunk/LayoutTests/fast/css/simple-selector-after-pseudo-element-expected.html</a></li>
<li><a href="#trunkLayoutTestsfastcsssimpleselectorafterpseudoelementhtml">trunk/LayoutTests/fast/css/simple-selector-after-pseudo-element.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (176983 => 176984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2014-12-08 23:55:04 UTC (rev 176983)
+++ trunk/LayoutTests/ChangeLog 2014-12-08 23:56:20 UTC (rev 176984)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+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.
+
</ins><span class="cx"> 2014-12-08 Benjamin Poulain <benjamin@webkit.org>
</span><span class="cx">
</span><span class="cx"> Fix the parsing of advanced :lang() after r176902
</span></span></pre></div>
<a id="trunkLayoutTestsfastcssduplicatedafterpseudoelementexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/duplicated-after-pseudo-element-expected.html (0 => 176984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/duplicated-after-pseudo-element-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css/duplicated-after-pseudo-element-expected.html 2014-12-08 23:56:20 UTC (rev 176984)
</span><span class="lines">@@ -0,0 +1,16 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssduplicatedafterpseudoelementhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/duplicated-after-pseudo-element.html (0 => 176984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/duplicated-after-pseudo-element.html (rev 0)
+++ trunk/LayoutTests/fast/css/duplicated-after-pseudo-element.html 2014-12-08 23:56:20 UTC (rev 176984)
</span><span class="lines">@@ -0,0 +1,20 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssduplicatedbeforepseudoelementexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/duplicated-before-pseudo-element-expected.html (0 => 176984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/duplicated-before-pseudo-element-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css/duplicated-before-pseudo-element-expected.html 2014-12-08 23:56:20 UTC (rev 176984)
</span><span class="lines">@@ -0,0 +1,16 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssduplicatedbeforepseudoelementhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/duplicated-before-pseudo-element.html (0 => 176984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/duplicated-before-pseudo-element.html (rev 0)
+++ trunk/LayoutTests/fast/css/duplicated-before-pseudo-element.html 2014-12-08 23:56:20 UTC (rev 176984)
</span><span class="lines">@@ -0,0 +1,20 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcsssimpleselectorafterpseudoelementexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/simple-selector-after-pseudo-element-expected.html (0 => 176984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/simple-selector-after-pseudo-element-expected.html (rev 0)
+++ trunk/LayoutTests/fast/css/simple-selector-after-pseudo-element-expected.html 2014-12-08 23:56:20 UTC (rev 176984)
</span><span class="lines">@@ -0,0 +1,20 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcsssimpleselectorafterpseudoelementhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/simple-selector-after-pseudo-element.html (0 => 176984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/simple-selector-after-pseudo-element.html (rev 0)
+++ trunk/LayoutTests/fast/css/simple-selector-after-pseudo-element.html 2014-12-08 23:56:20 UTC (rev 176984)
</span><span class="lines">@@ -0,0 +1,31 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (176983 => 176984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-12-08 23:55:04 UTC (rev 176983)
+++ trunk/Source/WebCore/ChangeLog 2014-12-08 23:56:20 UTC (rev 176984)
</span><span class="lines">@@ -1,3 +1,31 @@
</span><ins>+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.
+
</ins><span class="cx"> 2014-12-08 Benjamin Poulain <benjamin@webkit.org>
</span><span class="cx">
</span><span class="cx"> Fix the parsing of advanced :lang() after r176902
</span></span></pre></div>
<a id="trunkSourceWebCorecssjitSelectorCompilercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/cssjit/SelectorCompiler.cpp (176983 => 176984)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/cssjit/SelectorCompiler.cpp 2014-12-08 23:55:04 UTC (rev 176983)
+++ trunk/Source/WebCore/cssjit/SelectorCompiler.cpp 2014-12-08 23:56:20 UTC (rev 176984)
</span><span class="lines">@@ -866,16 +866,11 @@
</span><span class="cx"> for (const CSSSelector* selector = rootSelector; selector; selector = selector->tagHistory()) {
</span><span class="cx"> specificity = CSSSelector::addSpecificities(specificity, selector->simpleSelectorSpecificity());
</span><span class="cx">
</span><del>- CSSSelector::Relation relation = selector->relation();
-
</del><span class="cx"> // A selector is invalid if something follows a pseudo-element.
</span><span class="cx"> // We make an exception for scrollbar pseudo elements and allow a set of pseudo classes (but nothing else)
</span><span class="cx"> // to follow the pseudo elements.
</span><del>- if (relation == CSSSelector::SubSelector
- && fragment.pseudoElementSelector
- && !isScrollbarPseudoElement(fragment.pseudoElementSelector->pseudoElementType())) {
</del><ins>+ if (fragment.pseudoElementSelector && !isScrollbarPseudoElement(fragment.pseudoElementSelector->pseudoElementType()))
</ins><span class="cx"> return FunctionType::CannotMatchAnything;
</span><del>- }
</del><span class="cx">
</span><span class="cx"> switch (selector->match()) {
</span><span class="cx"> case CSSSelector::Tag:
</span><span class="lines">@@ -933,6 +928,7 @@
</span><span class="cx"> case CSSSelector::PseudoElementScrollbarThumb:
</span><span class="cx"> case CSSSelector::PseudoElementScrollbarTrack:
</span><span class="cx"> case CSSSelector::PseudoElementScrollbarTrackPiece:
</span><ins>+ ASSERT(!fragment.pseudoElementSelector);
</ins><span class="cx"> fragment.pseudoElementSelector = selector;
</span><span class="cx"> break;
</span><span class="cx"> case CSSSelector::PseudoElementUnknown:
</span><span class="lines">@@ -979,6 +975,7 @@
</span><span class="cx"> return FunctionType::CannotMatchAnything;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ CSSSelector::Relation relation = selector->relation();
</ins><span class="cx"> if (relation == CSSSelector::SubSelector)
</span><span class="cx"> continue;
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>