CSS: Refactor :visited handling in SelectorChecker
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 30 Aug 2014 13:42:51 +0000 (13:42 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 30 Aug 2014 13:42:51 +0000 (13:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135639

Reviewed by Benjamin Poulain.

Source/WebCore:

:visited is specially handled in the SelectorChecker and style resolution
because of security issues. That is nested links and adjacent combinators.
Since we propagate linkState from the ancestors,

    1. linkStates of ancestors from the target node are only used to calculate
    the linkState of the current node.
    This is why adjacent combinators disable :visited.

    2. linkState is overrides by the closest link element in the ancestors.
    This is why :visited is effective on the closest link element.

In this patch, we fix 3 things.

    1. Simplify SelectorChecker. Move m_mode to CheckingContext and it makes
    CheckingContext more similar to SelectorCompiler::CheckingContext in CSS JIT.
    And hide visitedMatchType parameter from the caller of SelectorChecker.

    2. Disable :visited inside :-webkit-any. Currently, :-webkit-any provides MatchAll
    link match type. So considering visited match type in the matching phase of :visited
    provides inconsistency. In this patch, :-webkit-any(:visited) never matches.
    And :-webkit-any(:link) acts like a :-webkit-any(:any-link). This behavior represents
    that visited match type is always considered as disabled inside :-webkit-any.
    This behavior may be changed when Selector Level4 is implemented.

    3. Fix the issue when traversing the descendant element,
    first encountered link check is missing.

Tests: fast/history/link-inside-any.html
       fast/history/link-inside-not.html
       fast/history/nested-visited-test-override.html
       fast/history/visited-inside-any.html
       fast/history/visited-inside-not.html

* css/ElementRuleCollector.cpp:
(WebCore::ElementRuleCollector::ruleMatches):
* css/SelectorChecker.cpp:
(WebCore::SelectorChecker::SelectorChecker):
(WebCore::SelectorChecker::match):
(WebCore::hasScrollbarPseudoElement):
(WebCore::checkingContextForParent):
(WebCore::SelectorChecker::matchRecursively):
(WebCore::SelectorChecker::checkOne):
* css/SelectorChecker.h:
(WebCore::SelectorChecker::SelectorCheckingContext::SelectorCheckingContext):
* css/StyleResolver.h:
(WebCore::checkRegionSelector):
* dom/SelectorQuery.cpp:
(WebCore::SelectorDataList::selectorMatches):

LayoutTests:

* fast/history/link-inside-any-expected.txt: Added.
* fast/history/link-inside-any.html: Added.
* fast/history/link-inside-not-expected.txt: Added.
* fast/history/link-inside-not.html: Added.
* fast/history/nested-visited-test-override-expected.txt: Added.
* fast/history/nested-visited-test-override.html: Added.
* fast/history/visited-inside-any-expected.txt: Added.
* fast/history/visited-inside-any.html: Added.
* fast/history/visited-inside-not-expected.txt: Added.
* fast/history/visited-inside-not.html: Added.

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

17 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/history/link-inside-any-expected.txt [new file with mode: 0644]
LayoutTests/fast/history/link-inside-any.html [new file with mode: 0644]
LayoutTests/fast/history/link-inside-not-expected.txt [new file with mode: 0644]
LayoutTests/fast/history/link-inside-not.html [new file with mode: 0644]
LayoutTests/fast/history/nested-visited-test-override-expected.txt [new file with mode: 0644]
LayoutTests/fast/history/nested-visited-test-override.html [new file with mode: 0644]
LayoutTests/fast/history/visited-inside-any-expected.txt [new file with mode: 0644]
LayoutTests/fast/history/visited-inside-any.html [new file with mode: 0644]
LayoutTests/fast/history/visited-inside-not-expected.txt [new file with mode: 0644]
LayoutTests/fast/history/visited-inside-not.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/ElementRuleCollector.cpp
Source/WebCore/css/SelectorChecker.cpp
Source/WebCore/css/SelectorChecker.h
Source/WebCore/css/StyleResolver.h
Source/WebCore/dom/SelectorQuery.cpp

index 4b1d7d5..5e3c089 100644 (file)
@@ -1,3 +1,21 @@
+2014-08-30  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        CSS: Refactor :visited handling in SelectorChecker
+        https://bugs.webkit.org/show_bug.cgi?id=135639
+
+        Reviewed by Benjamin Poulain.
+
+        * fast/history/link-inside-any-expected.txt: Added.
+        * fast/history/link-inside-any.html: Added.
+        * fast/history/link-inside-not-expected.txt: Added.
+        * fast/history/link-inside-not.html: Added.
+        * fast/history/nested-visited-test-override-expected.txt: Added.
+        * fast/history/nested-visited-test-override.html: Added.
+        * fast/history/visited-inside-any-expected.txt: Added.
+        * fast/history/visited-inside-any.html: Added.
+        * fast/history/visited-inside-not-expected.txt: Added.
+        * fast/history/visited-inside-not.html: Added.
+
 2014-08-28  Saam Barati  <sbarati@apple.com>
 
         Web Inspector: Write tests for ScriptSyntaxTree and fix bugs in the data structure
diff --git a/LayoutTests/fast/history/link-inside-any-expected.txt b/LayoutTests/fast/history/link-inside-any-expected.txt
new file mode 100644 (file)
index 0000000..0c63a38
--- /dev/null
@@ -0,0 +1,18 @@
+:link inside :-webkit-any should behave like :any-link.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS oneStyle.color became "rgb(0, 128, 0)"
+PASS twoStyle.color became "rgb(0, 128, 0)"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+One and Two links should be green:
+
+One
+
+and Two
+Three
+
+
diff --git a/LayoutTests/fast/history/link-inside-any.html b/LayoutTests/fast/history/link-inside-any.html
new file mode 100644 (file)
index 0000000..ce1c716
--- /dev/null
@@ -0,0 +1,64 @@
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+description(":link inside :-webkit-any should behave like :any-link.");
+jsTestIsAsync = true;
+
+if (window.testRunner)
+    window.testRunner.keepWebHistory();
+
+var count = 0;
+function finish()
+{
+    if (++count === 2)
+        finishJSTest();
+}
+
+function compareStyles()
+{
+    if (window.internals) {
+        oneStyle = internals.computedStyleIncludingVisitedInfo(document.getElementById("one").firstChild);
+        twoStyle = internals.computedStyleIncludingVisitedInfo(document.getElementById("two"));
+        shouldBecomeEqualToString("oneStyle.color", "rgb(0, 128, 0)", finish);
+        shouldBecomeEqualToString("twoStyle.color", "rgb(0, 128, 0)", finish);
+    }
+}
+</script>
+<style>
+a { color: red; }
+#area1 #one :-webkit-any(:link) {
+    color: green;
+}
+#area1 #one + :-webkit-any(:link) + a {
+    color: green;
+}
+
+#area2 p {
+    color: green;
+}
+/* :-webkit-any(:link) should not matched against not link element. */
+#area2 :-webkit-any(:link) + p {
+    color: red;
+}
+</style>
+</head>
+<body onload="compareStyles()">
+<iframe src="resources/dummy.html" style="display:none"></iframe>
+
+<p>One and Two links should be green:</p>
+<div id="area1">
+    <p id="one"><a href="resources/dummy.html">One</a></p>
+    <a href="resources/dummy.html">and</a>
+    <a id="two"href="resources/dummy.html">Two</a>
+</div>
+
+<div id="area2">
+    <span></span>
+    <p id="three">Three</p>
+</div>
+
+<div id=console></div>
+</body>
+<script src="../../resources/js-test-post.js"></script>
+</html>
diff --git a/LayoutTests/fast/history/link-inside-not-expected.txt b/LayoutTests/fast/history/link-inside-not-expected.txt
new file mode 100644 (file)
index 0000000..ab2a3c3
--- /dev/null
@@ -0,0 +1,23 @@
+:link inside :not should be always matched when visited match type is enabled.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS oneStyle.color became "rgb(0, 128, 0)"
+PASS twoStyle.color became "rgb(0, 128, 0)"
+PASS threeStyle.color became "rgb(0, 128, 0)"
+PASS fourStyle.color became "rgb(0, 128, 0)"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+color of One, Two, Three, and Four should be green:
+
+One
+
+Two
+
+Three
+
+Four
+
+
diff --git a/LayoutTests/fast/history/link-inside-not.html b/LayoutTests/fast/history/link-inside-not.html
new file mode 100644 (file)
index 0000000..e37e5eb
--- /dev/null
@@ -0,0 +1,81 @@
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+description(":link inside :not should be always matched when visited match type is enabled.");
+jsTestIsAsync = true;
+
+if (window.testRunner)
+    window.testRunner.keepWebHistory();
+
+var count = 0;
+function finish() {
+    if (++count === 4) {
+        finishJSTest();
+    }
+}
+
+function compareStyles()
+{
+    if (window.internals) {
+        oneStyle = internals.computedStyleIncludingVisitedInfo(document.getElementById("one"));
+        twoStyle = internals.computedStyleIncludingVisitedInfo(document.getElementById("two"));
+        threeStyle = internals.computedStyleIncludingVisitedInfo(document.getElementById("three"));
+        fourStyle = internals.computedStyleIncludingVisitedInfo(document.getElementById("four"));
+        shouldBecomeEqualToString("oneStyle.color", "rgb(0, 128, 0)", finish);
+        shouldBecomeEqualToString("twoStyle.color", "rgb(0, 128, 0)", finish);
+        shouldBecomeEqualToString("threeStyle.color", "rgb(0, 128, 0)", finish);
+        shouldBecomeEqualToString("fourStyle.color", "rgb(0, 128, 0)", finish);
+    }
+}
+</script>
+<style>
+a { color: red; }
+#area1 a:not(:link) {
+    color: green;
+}
+
+#area2 :not(:link) {
+    color: green;
+}
+
+/* In this case, :not(:link)'s visited match type is disabled. So isLink() check is used instead. */
+#area3 span {
+    color: green;
+}
+#area3 :not(:link) + span {
+    color: red;
+}
+
+#area4 :not(:link) + span {
+    color: green;
+}
+</style>
+</head>
+<body onload="compareStyles()">
+<iframe src="resources/dummy.html" style="display:none"></iframe>
+
+<p>color of One, Two, Three, and Four should be green:</p>
+<p>
+
+<p id="area1">
+    <a id="one" href="resources/dummy.html">One</a>
+</p>
+
+<p><a href="resources/dummy.html"><span id="area2"><span id="two">Two</span></span></a></p>
+
+<p id="area3">
+    <a href="resources/dummy.html"></a>
+    <span id="three">Three</span>
+</p>
+
+<p id="area4">
+    <span></span>
+    <span id="four">Four</span>
+</p>
+</p>
+
+<div id=console></div>
+</body>
+<script src="../../resources/js-test-post.js"></script>
+</html>
diff --git a/LayoutTests/fast/history/nested-visited-test-override-expected.txt b/LayoutTests/fast/history/nested-visited-test-override-expected.txt
new file mode 100644 (file)
index 0000000..9efe8f5
--- /dev/null
@@ -0,0 +1,14 @@
+:visited is only effective when the target is first encountered link element.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS style.color became "rgb(0, 128, 0)"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+This link should be green:
+
+One
+
+
diff --git a/LayoutTests/fast/history/nested-visited-test-override.html b/LayoutTests/fast/history/nested-visited-test-override.html
new file mode 100644 (file)
index 0000000..caabe24
--- /dev/null
@@ -0,0 +1,43 @@
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+description(":visited is only effective when the target is first encountered link element.");
+
+jsTestIsAsync = true;
+
+if (window.testRunner)
+    window.testRunner.keepWebHistory();
+
+function compareStyles()
+{
+
+    var anchor = document.createElement("a");
+    anchor.href="resources/dummy.html";
+    anchor.setAttribute("id", "one");
+    anchor.innerHTML = "<span>One</span>";
+    document.getElementById("enclosure").appendChild(anchor);
+
+    if (window.internals) {
+        style = internals.computedStyleIncludingVisitedInfo(document.getElementById("one").firstChild);
+        shouldBecomeEqualToString("style.color", "rgb(0, 128, 0)", finishJSTest);
+    }
+}
+</script>
+<style>
+.ok span { color: green; }
+.ok:visited span { color: red; }
+</style>
+</head>
+<body onload="compareStyles()">
+<iframe src="resources/dummy.html" style="display:none"></iframe>
+
+<p>This link should be green:</p>
+<p>
+<a id="enclosure" class="ok" href="http://madeup.site.com"></a>
+</p>
+
+<div id=console></div>
+</body>
+<script src="../../resources/js-test-post.js"></script>
+</html>
diff --git a/LayoutTests/fast/history/visited-inside-any-expected.txt b/LayoutTests/fast/history/visited-inside-any-expected.txt
new file mode 100644 (file)
index 0000000..f72dbfd
--- /dev/null
@@ -0,0 +1,16 @@
+:visited inside :-webkit-any should not be effective.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS oneStyle.color became "rgb(0, 128, 0)"
+PASS twoStyle.color became "rgb(0, 128, 0)"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+One and Two links should be green:
+
+One
+
+and Two
+
diff --git a/LayoutTests/fast/history/visited-inside-any.html b/LayoutTests/fast/history/visited-inside-any.html
new file mode 100644 (file)
index 0000000..573ab71
--- /dev/null
@@ -0,0 +1,51 @@
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+description(":visited inside :-webkit-any should not be effective.");
+jsTestIsAsync = true;
+
+if (window.testRunner)
+    window.testRunner.keepWebHistory();
+
+var count = 0;
+function finish()
+{
+    if (++count === 2)
+        finishJSTest();
+}
+
+function compareStyles()
+{
+    if (window.internals) {
+        oneStyle = internals.computedStyleIncludingVisitedInfo(document.getElementById("one").firstChild);
+        twoStyle = internals.computedStyleIncludingVisitedInfo(document.getElementById("two"));
+        shouldBecomeEqualToString("oneStyle.color", "rgb(0, 128, 0)", finish);
+        shouldBecomeEqualToString("twoStyle.color", "rgb(0, 128, 0)", finish);
+    }
+}
+</script>
+<style>
+a { color: green; }
+#one :-webkit-any(:visited) {
+    color: red;
+}
+#one + :-webkit-any(:visited) + a {
+    color: red;
+}
+</style>
+</head>
+<body onload="compareStyles()">
+<iframe src="resources/dummy.html" style="display:none"></iframe>
+
+<p>One and Two links should be green:</p>
+<div>
+<p id="one"><a href="resources/dummy.html">One</a></p>
+<a href="resources/dummy.html">and</a>
+<a id="two"href="resources/dummy.html">Two</a>
+</div>
+
+<div id=console></div>
+</body>
+<script src="../../resources/js-test-post.js"></script>
+</html>
diff --git a/LayoutTests/fast/history/visited-inside-not-expected.txt b/LayoutTests/fast/history/visited-inside-not-expected.txt
new file mode 100644 (file)
index 0000000..be8449b
--- /dev/null
@@ -0,0 +1,17 @@
+:visited inside :not should be always matched.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS oneStyle.color became "rgb(0, 128, 0)"
+PASS twoStyle.color became "rgb(0, 128, 0)"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+color of One and Two should be green:
+
+One
+
+Two
+
+
diff --git a/LayoutTests/fast/history/visited-inside-not.html b/LayoutTests/fast/history/visited-inside-not.html
new file mode 100644 (file)
index 0000000..1352913
--- /dev/null
@@ -0,0 +1,50 @@
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+description(":visited inside :not should be always matched.");
+jsTestIsAsync = true;
+
+if (window.testRunner)
+    window.testRunner.keepWebHistory();
+
+var count = 0;
+function finish() {
+    if (++count === 2) {
+        finishJSTest();
+    }
+}
+
+function compareStyles()
+{
+    if (window.internals) {
+        oneStyle = internals.computedStyleIncludingVisitedInfo(document.getElementById("one"));
+        twoStyle = internals.computedStyleIncludingVisitedInfo(document.getElementById("two"));
+        shouldBecomeEqualToString("oneStyle.color", "rgb(0, 128, 0)", finish);
+        shouldBecomeEqualToString("twoStyle.color", "rgb(0, 128, 0)", finish);
+    }
+}
+</script>
+<style>
+a { color: red; }
+#area1 a:not(:visited) {
+    color: green;
+}
+#area2 :not(:visited) {
+    color: green;
+}
+</style>
+</head>
+<body onload="compareStyles()">
+<iframe src="resources/dummy.html" style="display:none"></iframe>
+
+<p>color of One and Two should be green:</p>
+<div>
+<p id="area1"><a id="one" href="http://madeup.site.com">One</a></p>
+<p><a href="http://madeup.site.com"><span id="area2"><span id="two">Two</span></span></a></p>
+</div>
+
+<div id=console></div>
+</body>
+<script src="../../resources/js-test-post.js"></script>
+</html>
index 4e6b93a..c015ba4 100644 (file)
@@ -1,3 +1,59 @@
+2014-08-30  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        CSS: Refactor :visited handling in SelectorChecker
+        https://bugs.webkit.org/show_bug.cgi?id=135639
+
+        Reviewed by Benjamin Poulain.
+
+        :visited is specially handled in the SelectorChecker and style resolution
+        because of security issues. That is nested links and adjacent combinators.
+        Since we propagate linkState from the ancestors,
+
+            1. linkStates of ancestors from the target node are only used to calculate
+            the linkState of the current node.
+            This is why adjacent combinators disable :visited.
+
+            2. linkState is overrides by the closest link element in the ancestors.
+            This is why :visited is effective on the closest link element.
+
+        In this patch, we fix 3 things.
+
+            1. Simplify SelectorChecker. Move m_mode to CheckingContext and it makes
+            CheckingContext more similar to SelectorCompiler::CheckingContext in CSS JIT.
+            And hide visitedMatchType parameter from the caller of SelectorChecker.
+
+            2. Disable :visited inside :-webkit-any. Currently, :-webkit-any provides MatchAll
+            link match type. So considering visited match type in the matching phase of :visited
+            provides inconsistency. In this patch, :-webkit-any(:visited) never matches.
+            And :-webkit-any(:link) acts like a :-webkit-any(:any-link). This behavior represents
+            that visited match type is always considered as disabled inside :-webkit-any.
+            This behavior may be changed when Selector Level4 is implemented.
+
+            3. Fix the issue when traversing the descendant element,
+            first encountered link check is missing.
+
+        Tests: fast/history/link-inside-any.html
+               fast/history/link-inside-not.html
+               fast/history/nested-visited-test-override.html
+               fast/history/visited-inside-any.html
+               fast/history/visited-inside-not.html
+
+        * css/ElementRuleCollector.cpp:
+        (WebCore::ElementRuleCollector::ruleMatches):
+        * css/SelectorChecker.cpp:
+        (WebCore::SelectorChecker::SelectorChecker):
+        (WebCore::SelectorChecker::match):
+        (WebCore::hasScrollbarPseudoElement):
+        (WebCore::checkingContextForParent):
+        (WebCore::SelectorChecker::matchRecursively):
+        (WebCore::SelectorChecker::checkOne):
+        * css/SelectorChecker.h:
+        (WebCore::SelectorChecker::SelectorCheckingContext::SelectorCheckingContext):
+        * css/StyleResolver.h:
+        (WebCore::checkRegionSelector):
+        * dom/SelectorQuery.cpp:
+        (WebCore::SelectorDataList::selectorMatches):
+
 2014-08-29  Zalan Bujtas  <zalan@apple.com>
 
         Subpixel layout: Remove LayoutUnit's kEffectiveFixedPointDenominator.
index 95ea6ff..dad4a22 100644 (file)
@@ -320,8 +320,8 @@ inline bool ElementRuleCollector::ruleMatches(const RuleData& ruleData)
 #endif // ENABLE(CSS_SELECTOR_JIT)
 
     // Slow path.
-    SelectorChecker selectorChecker(m_element.document(), m_mode);
-    SelectorChecker::SelectorCheckingContext context(ruleData.selector(), &m_element, SelectorChecker::VisitedMatchEnabled);
+    SelectorChecker selectorChecker(m_element.document());
+    SelectorChecker::SelectorCheckingContext context(ruleData.selector(), &m_element, m_mode);
     context.elementStyle = m_style;
     context.pseudoId = m_pseudoStyleRequest.pseudoId;
     context.scrollbar = m_pseudoStyleRequest.scrollbar;
index d6f10b6..3fc9dc9 100644 (file)
@@ -128,10 +128,9 @@ static inline int countElementsOfTypeAfter(const Element* element, const Qualifi
     return count;
 }
 
-SelectorChecker::SelectorChecker(Document& document, Mode mode)
+SelectorChecker::SelectorChecker(Document& document)
     : m_strictParsing(!document.inQuirksMode())
     , m_documentIsHTML(document.isHTMLDocument())
-    , m_mode(mode)
 {
 }
 
@@ -143,12 +142,12 @@ bool SelectorChecker::match(const SelectorCheckingContext& context) const
     if (context.pseudoId != NOPSEUDO && context.pseudoId != pseudoId)
         return false;
     if (context.pseudoId == NOPSEUDO && pseudoId != NOPSEUDO) {
-        if (m_mode == Mode::ResolvingStyle && pseudoId < FIRST_INTERNAL_PSEUDOID)
+        if (context.resolvingMode == Mode::ResolvingStyle && pseudoId < FIRST_INTERNAL_PSEUDOID)
             context.elementStyle->setHasPseudoStyle(pseudoId);
 
         // When ignoring virtual pseudo elements, the context's pseudo should also be NOPSEUDO but that does
         // not cause a failure.
-        return m_mode == Mode::CollectingRulesIgnoringVirtualPseudoElements;
+        return context.resolvingMode == Mode::CollectingRulesIgnoringVirtualPseudoElements;
     }
     return true;
 }
@@ -164,12 +163,22 @@ inline static bool hasScrollbarPseudoElement(const SelectorChecker::SelectorChec
         ASSERT_UNUSED(context, context.scrollbar);
         return true;
     }
-    
+
     // RESIZER does not always have a scrollbar but it is a scrollbar-like pseudo element
     // because it can have more than one pseudo element.
     return dynamicPseudo == RESIZER;
 }
-    
+
+static SelectorChecker::SelectorCheckingContext checkingContextForParent(const SelectorChecker::SelectorCheckingContext& context)
+{
+    SelectorChecker::SelectorCheckingContext updatedContext(context);
+    // Disable :visited matching when we see the first link.
+    if (context.element->isLink())
+        updatedContext.visitedMatchType = SelectorChecker::VisitedMatchDisabled;
+    updatedContext.element = context.element->parentElement();
+    return updatedContext;
+}
+
 // Recursive check of selectors and combinators
 // It can return 4 different values:
 // * SelectorMatches          - the selector matches the element e
@@ -198,7 +207,7 @@ SelectorChecker::Match SelectorChecker::matchRecursively(const SelectorCheckingC
             } else
                 return SelectorFailsLocally;
         } else {
-            if ((!context.elementStyle && m_mode == Mode::ResolvingStyle) || m_mode == Mode::QueryingRules)
+            if ((!context.elementStyle && context.resolvingMode == Mode::ResolvingStyle) || context.resolvingMode == Mode::QueryingRules)
                 return SelectorFailsLocally;
 
             PseudoId pseudoId = CSSSelector::pseudoId(context.selector->pseudoElementType());
@@ -224,8 +233,8 @@ SelectorChecker::Match SelectorChecker::matchRecursively(const SelectorCheckingC
         if (context.pseudoId != NOPSEUDO && context.pseudoId != dynamicPseudo)
             return SelectorFailsCompletely;
 
-        // Disable :visited matching when we see the first link or try to match anything else than an ancestors.
-        if (context.element->isLink() || (relation != CSSSelector::Descendant && relation != CSSSelector::Child))
+        // Disable :visited matching when we try to match anything else than an ancestors.
+        if (relation != CSSSelector::Descendant && relation != CSSSelector::Child)
             nextContext.visitedMatchType = VisitedMatchDisabled;
 
         nextContext.pseudoId = NOPSEUDO;
@@ -233,10 +242,10 @@ SelectorChecker::Match SelectorChecker::matchRecursively(const SelectorCheckingC
 
     switch (relation) {
     case CSSSelector::Descendant:
-        nextContext.element = context.element->parentElement();
+        nextContext = checkingContextForParent(nextContext);
         nextContext.firstSelectorOfTheFragment = nextContext.selector;
         nextContext.elementStyle = 0;
-        for (; nextContext.element; nextContext.element = nextContext.element->parentElement()) {
+        for (; nextContext.element; nextContext = checkingContextForParent(nextContext)) {
             Match match = this->matchRecursively(nextContext, ignoreDynamicPseudo);
             if (match == SelectorMatches || match == SelectorFailsCompletely)
                 return match;
@@ -245,7 +254,7 @@ SelectorChecker::Match SelectorChecker::matchRecursively(const SelectorCheckingC
 
     case CSSSelector::Child:
         {
-            nextContext.element = context.element->parentElement();
+            nextContext = checkingContextForParent(nextContext);
             if (!nextContext.element)
                 return SelectorFailsCompletely;
             nextContext.firstSelectorOfTheFragment = nextContext.selector;
@@ -257,7 +266,7 @@ SelectorChecker::Match SelectorChecker::matchRecursively(const SelectorCheckingC
         }
 
     case CSSSelector::DirectAdjacent:
-        if (m_mode == Mode::ResolvingStyle) {
+        if (context.resolvingMode == Mode::ResolvingStyle) {
             if (Element* parentElement = context.element->parentElement())
                 parentElement->setChildrenAffectedByDirectAdjacentRules();
         }
@@ -269,7 +278,7 @@ SelectorChecker::Match SelectorChecker::matchRecursively(const SelectorCheckingC
         return matchRecursively(nextContext, ignoreDynamicPseudo);
 
     case CSSSelector::IndirectAdjacent:
-        if (m_mode == Mode::ResolvingStyle) {
+        if (context.resolvingMode == Mode::ResolvingStyle) {
             if (Element* parentElement = context.element->parentElement())
                 parentElement->setChildrenAffectedByForwardPositionalRules();
         }
@@ -289,7 +298,7 @@ SelectorChecker::Match SelectorChecker::matchRecursively(const SelectorCheckingC
         // to follow the pseudo elements.
         nextContext.hasScrollbarPseudo = hasScrollbarPseudoElement(context, dynamicPseudo);
         nextContext.hasSelectionPseudo = dynamicPseudo == SELECTION;
-        if ((context.elementStyle || m_mode == Mode::CollectingRules) && dynamicPseudo != NOPSEUDO
+        if ((context.elementStyle || context.resolvingMode == Mode::CollectingRules) && dynamicPseudo != NOPSEUDO
             && !nextContext.hasSelectionPseudo
             && !(nextContext.hasScrollbarPseudo && nextContext.selector->m_match == CSSSelector::PseudoClass))
             return SelectorFailsCompletely;
@@ -522,7 +531,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
                         }
                     }
                 }
-                if (m_mode == Mode::ResolvingStyle) {
+                if (context.resolvingMode == Mode::ResolvingStyle) {
                     element->setStyleAffectedByEmpty();
                     if (context.elementStyle)
                         context.elementStyle->setEmptyState(result);
@@ -533,7 +542,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
             // first-child matches the first child that is an element
             if (Element* parentElement = element->parentElement()) {
                 bool result = isFirstChildElement(element);
-                if (m_mode == Mode::ResolvingStyle) {
+                if (context.resolvingMode == Mode::ResolvingStyle) {
                     RenderStyle* childStyle = context.elementStyle ? context.elementStyle : element->renderStyle();
                     parentElement->setChildrenAffectedByFirstChildRules();
                     if (result && childStyle)
@@ -546,7 +555,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
             // first-of-type matches the first element of its type
             if (Element* parentElement = element->parentElement()) {
                 bool result = isFirstOfType(element, element->tagQName());
-                if (m_mode == Mode::ResolvingStyle)
+                if (context.resolvingMode == Mode::ResolvingStyle)
                     parentElement->setChildrenAffectedByForwardPositionalRules();
                 return result;
             }
@@ -555,7 +564,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
             // last-child matches the last child that is an element
             if (Element* parentElement = element->parentElement()) {
                 bool result = parentElement->isFinishedParsingChildren() && isLastChildElement(element);
-                if (m_mode == Mode::ResolvingStyle) {
+                if (context.resolvingMode == Mode::ResolvingStyle) {
                     RenderStyle* childStyle = context.elementStyle ? context.elementStyle : element->renderStyle();
                     parentElement->setChildrenAffectedByLastChildRules();
                     if (result && childStyle)
@@ -567,7 +576,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
         case CSSSelector::PseudoClassLastOfType:
             // last-of-type matches the last element of its type
             if (Element* parentElement = element->parentElement()) {
-                if (m_mode == Mode::ResolvingStyle)
+                if (context.resolvingMode == Mode::ResolvingStyle)
                     parentElement->setChildrenAffectedByBackwardPositionalRules();
                 if (!parentElement->isFinishedParsingChildren())
                     return false;
@@ -578,7 +587,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
             if (Element* parentElement = element->parentElement()) {
                 bool firstChild = isFirstChildElement(element);
                 bool onlyChild = firstChild && parentElement->isFinishedParsingChildren() && isLastChildElement(element);
-                if (m_mode == Mode::ResolvingStyle) {
+                if (context.resolvingMode == Mode::ResolvingStyle) {
                     RenderStyle* childStyle = context.elementStyle ? context.elementStyle : element->renderStyle();
                     parentElement->setChildrenAffectedByFirstChildRules();
                     parentElement->setChildrenAffectedByLastChildRules();
@@ -593,7 +602,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
         case CSSSelector::PseudoClassOnlyOfType:
             // FIXME: This selector is very slow.
             if (Element* parentElement = element->parentElement()) {
-                if (m_mode == Mode::ResolvingStyle) {
+                if (context.resolvingMode == Mode::ResolvingStyle) {
                     parentElement->setChildrenAffectedByForwardPositionalRules();
                     parentElement->setChildrenAffectedByBackwardPositionalRules();
                 }
@@ -605,7 +614,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
 #if ENABLE(CSS_SELECTORS_LEVEL4)
         case CSSSelector::PseudoClassPlaceholderShown:
             if (isHTMLTextFormControlElement(*element)) {
-                if (m_mode == Mode::ResolvingStyle) {
+                if (context.resolvingMode == Mode::ResolvingStyle) {
                     if (RenderStyle* style = context.elementStyle ? context.elementStyle : element->renderStyle())
                         style->setUnique();
                 }
@@ -618,7 +627,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
                 break;
             if (Element* parentElement = element->parentElement()) {
                 int count = 1 + countElementsBefore(element);
-                if (m_mode == Mode::ResolvingStyle) {
+                if (context.resolvingMode == Mode::ResolvingStyle) {
                     RenderStyle* childStyle = context.elementStyle ? context.elementStyle : element->renderStyle();
                     element->setChildIndex(count);
                     if (childStyle)
@@ -635,7 +644,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
                 break;
             if (Element* parentElement = element->parentElement()) {
                 int count = 1 + countElementsOfTypeBefore(element, element->tagQName());
-                if (m_mode == Mode::ResolvingStyle)
+                if (context.resolvingMode == Mode::ResolvingStyle)
                     parentElement->setChildrenAffectedByForwardPositionalRules();
 
                 if (selector->matchNth(count))
@@ -646,7 +655,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
             if (!selector->parseNth())
                 break;
             if (Element* parentElement = element->parentElement()) {
-                if (m_mode == Mode::ResolvingStyle)
+                if (context.resolvingMode == Mode::ResolvingStyle)
                     parentElement->setChildrenAffectedByBackwardPositionalRules();
                 if (!parentElement->isFinishedParsingChildren())
                     return false;
@@ -659,7 +668,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
             if (!selector->parseNth())
                 break;
             if (Element* parentElement = element->parentElement()) {
-                if (m_mode == Mode::ResolvingStyle)
+                if (context.resolvingMode == Mode::ResolvingStyle)
                     parentElement->setChildrenAffectedByBackwardPositionalRules();
                 if (!parentElement->isFinishedParsingChildren())
                     return false;
@@ -693,9 +702,12 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
             return element->isLink();
         case CSSSelector::PseudoClassVisited:
             // ...except if :visited matching is disabled for ancestor/sibling matching.
+            // Inside functional pseudo class except for :not, :visited never matches.
+            if (context.inFunctionalPseudoClass)
+                return false;
             return element->isLink() && context.visitedMatchType == VisitedMatchEnabled;
         case CSSSelector::PseudoClassDrag:
-            if (m_mode == Mode::ResolvingStyle) {
+            if (context.resolvingMode == Mode::ResolvingStyle) {
                 if (context.elementStyle)
                     context.elementStyle->setAffectedByDrag();
                 else
@@ -708,7 +720,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
             return matchesFocusPseudoClass(element);
         case CSSSelector::PseudoClassHover:
             if (m_strictParsing || element->isLink() || canMatchHoverOrActiveInQuirksMode(context)) {
-                if (m_mode == Mode::ResolvingStyle) {
+                if (context.resolvingMode == Mode::ResolvingStyle) {
                     if (context.elementStyle)
                         context.elementStyle->setAffectedByHover();
                     else
@@ -720,7 +732,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context) const
             break;
         case CSSSelector::PseudoClassActive:
             if (m_strictParsing || element->isLink() || canMatchHoverOrActiveInQuirksMode(context)) {
-                if (m_mode == Mode::ResolvingStyle) {
+                if (context.resolvingMode == Mode::ResolvingStyle) {
                     if (context.elementStyle)
                         context.elementStyle->setAffectedByActive();
                     else
index 23fcd8e..f4393f3 100644 (file)
@@ -51,15 +51,16 @@ public:
         ResolvingStyle = 0, CollectingRules, CollectingRulesIgnoringVirtualPseudoElements, QueryingRules
     };
 
-    SelectorChecker(Document&, Mode);
+    SelectorChecker(Document&);
 
     struct SelectorCheckingContext {
         // Initial selector constructor
-        SelectorCheckingContext(const CSSSelector* selector, Element* element, VisitedMatchType visitedMatchType)
+        SelectorCheckingContext(const CSSSelector* selector, Element* element, Mode resolvingMode)
             : selector(selector)
+            , resolvingMode(resolvingMode)
             , element(element)
             , scope(nullptr)
-            , visitedMatchType(visitedMatchType)
+            , visitedMatchType(resolvingMode == Mode::QueryingRules ? VisitedMatchDisabled : VisitedMatchEnabled)
             , pseudoId(NOPSEUDO)
             , elementStyle(nullptr)
             , scrollbar(nullptr)
@@ -71,6 +72,7 @@ public:
         { }
 
         const CSSSelector* selector;
+        Mode resolvingMode;
         Element* element;
         const ContainerNode* scope;
         VisitedMatchType visitedMatchType;
@@ -102,7 +104,6 @@ private:
 
     bool m_strictParsing;
     bool m_documentIsHTML;
-    Mode m_mode;
 };
 
 inline bool SelectorChecker::isCommonPseudoClassSelector(const CSSSelector* selector)
index 1a7afe5..6ea6706 100644 (file)
@@ -572,9 +572,9 @@ inline bool checkRegionSelector(const CSSSelector* regionSelector, Element* regi
     if (!regionSelector || !regionElement)
         return false;
 
-    SelectorChecker selectorChecker(regionElement->document(), SelectorChecker::Mode::QueryingRules);
+    SelectorChecker selectorChecker(regionElement->document());
     for (const CSSSelector* s = regionSelector; s; s = CSSSelectorList::next(s)) {
-        SelectorChecker::SelectorCheckingContext selectorCheckingContext(s, regionElement, SelectorChecker::VisitedMatchDisabled);
+        SelectorChecker::SelectorCheckingContext selectorCheckingContext(s, regionElement, SelectorChecker::Mode::QueryingRules);
         if (selectorChecker.match(selectorCheckingContext))
             return true;
     }
index 10ef02f..5c3f9bc 100644 (file)
@@ -121,8 +121,8 @@ inline bool SelectorDataList::selectorMatches(const SelectorData& selectorData,
         return selectorCheckerFastPath.matches();
     }
 
-    SelectorChecker selectorChecker(element.document(), SelectorChecker::Mode::QueryingRules);
-    SelectorChecker::SelectorCheckingContext selectorCheckingContext(selectorData.selector, &element, SelectorChecker::VisitedMatchDisabled);
+    SelectorChecker selectorChecker(element.document());
+    SelectorChecker::SelectorCheckingContext selectorCheckingContext(selectorData.selector, &element, SelectorChecker::Mode::QueryingRules);
     selectorCheckingContext.scope = rootNode.isDocumentNode() ? nullptr : &rootNode;
     return selectorChecker.match(selectorCheckingContext);
 }