Title: [129746] trunk
Revision
129746
Author
apav...@chromium.org
Date
2012-09-27 02:55:52 -0700 (Thu, 27 Sep 2012)

Log Message

CollectingRules and QueryingRules modes of SelectorChecker miss some complex selectors with pseudo elements
https://bugs.webkit.org/show_bug.cgi?id=78595

Reviewed by Antti Koivisto.

Source/WebCore:

Do not use the same dynamicPseudo reference when recursively invoking checkSelector() for non-SubSelector selectors.

Test: fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex.html

* css/SelectorChecker.cpp:
(WebCore::SelectorChecker::checkSelector): Use new NOPSEUDO dynamic pseudoId values
for each non-SubSelector selector further in the tag history.

LayoutTests:

The original issue mentioned failures on selectors like ".foo.moo > .bar:before", while things worked fine for ".foo > .bar:before".
The test checks 2 and 3 selector levels for all conventional non-SubSelector relations (Child, Descendant, DirectAdjacent, IndirectAdjacent),
each selector in all levels but the last one containing a SubSelector relation.

* fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex.html: Added.
* fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex-expected.txt: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (129745 => 129746)


--- trunk/LayoutTests/ChangeLog	2012-09-27 09:44:05 UTC (rev 129745)
+++ trunk/LayoutTests/ChangeLog	2012-09-27 09:55:52 UTC (rev 129746)
@@ -1,3 +1,17 @@
+2012-09-25  Alexander Pavlov  <apav...@chromium.org>
+
+        CollectingRules and QueryingRules modes of SelectorChecker miss some complex selectors with pseudo elements
+        https://bugs.webkit.org/show_bug.cgi?id=78595
+
+        Reviewed by Antti Koivisto.
+
+        The original issue mentioned failures on selectors like ".foo.moo > .bar:before", while things worked fine for ".foo > .bar:before".
+        The test checks 2 and 3 selector levels for all conventional non-SubSelector relations (Child, Descendant, DirectAdjacent, IndirectAdjacent),
+        each selector in all levels but the last one containing a SubSelector relation.
+
+        * fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex.html: Added.
+        * fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex-expected.txt: Added.
+
 2012-09-27  Vsevolod Vlasov  <vse...@chromium.org>
 
         Unreviewed inspector test unskipped on qt, expectations updated.

Added: trunk/LayoutTests/fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex-expected.txt (0 => 129746)


--- trunk/LayoutTests/fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex-expected.txt	2012-09-27 09:55:52 UTC (rev 129746)
@@ -0,0 +1,12 @@
+Test for WebKit bug 78595: CollectingRules and QueryingRules modes of SelectorChecker miss some complex selectors with pseudo elements
+
+
+
+PASS Expected 'rgb(165, 42, 42)' for color in the matched CSS rule for element with id firstChild and pseudo-element :before and got 'rgb(165, 42, 42)'
+PASS Expected 'rgb(0, 255, 255)' for color in the matched CSS rule for element with id secondChild and pseudo-element :before and got 'rgb(0, 255, 255)'
+PASS Expected 'red' for color in the matched CSS rule for element with id thirdChild and pseudo-element :before and got 'red'
+PASS Expected 'rgb(255, 0, 255)' for color in the matched CSS rule for element with id fourthChild and pseudo-element :before and got 'rgb(255, 0, 255)'
+PASS Expected 'green' for color in the matched CSS rule for element with id firstParent and pseudo-element :before and got 'green'
+PASS Expected 'lime' for color in the matched CSS rule for element with id secondParent and pseudo-element :before and got 'lime'
+PASS Expected 'blue' for color in the matched CSS rule for element with id thirdParent and pseudo-element :before and got 'blue'
+PASS Expected 'rgb(220, 20, 60)' for color in the matched CSS rule for element with id fourthParent and pseudo-element :before and got 'rgb(220, 20, 60)'
Property changes on: trunk/LayoutTests/fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex-expected.txt
___________________________________________________________________

Added: svn:eol-style

Added: trunk/LayoutTests/fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex.html (0 => 129746)


--- trunk/LayoutTests/fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex.html	2012-09-27 09:55:52 UTC (rev 129746)
@@ -0,0 +1,125 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>Test for WebKit bug 78595: CollectingRules and QueryingRules modes of SelectorChecker miss some complex selectors with pseudo elements</title>
+    <style type="text/css">
+      .parent.secondParent .firstChild:before {
+          color: brown;
+          content: "brown";
+      }
+
+      .topParent.topmost .parent.secondParent .secondChild:before {
+          color: cyan;
+          content: "cyan";
+      }
+
+      .parent.secondParent > .thirdChild:before {
+          color: red;
+          content: "red";
+      }
+
+      .topParent.topmost > .parent.secondParent > .fourthChild:before {
+          color: magenta;
+          content: "magenta";
+      }
+
+      .parent.predecessor + .parent.firstParent:before {
+          color: green;
+          content: "green";
+      }
+
+      .parent.firstParent ~ .secondParent:before {
+          color: lime;
+          content: "lime";
+      }
+
+      .parent.firstParent + .parent.secondParent + .thirdParent:before {
+          color: blue;
+          content: "blue";
+      }
+
+      .parent.predecessor ~ .parent.secondParent ~ .fourthParent:before {
+          color: crimson;
+          content: "crimson";
+      }
+
+      .pass {
+          color: green;
+      }
+
+      .fail {
+          color: red;
+      }
+
+    </style>
+    <script type="text/_javascript_">
+      if (window.testRunner)
+          testRunner.dumpAsText();
+
+      var tests = [
+        { 'elementId' : 'firstChild', 'expectedValue' : 'rgb(165, 42, 42)' },
+        { 'elementId' : 'secondChild', 'expectedValue' : 'rgb(0, 255, 255)' },
+        { 'elementId' : 'thirdChild', 'expectedValue' : 'red' },
+        { 'elementId' : 'fourthChild', 'expectedValue' : 'rgb(255, 0, 255)' },
+        { 'elementId' : 'firstParent', 'expectedValue' : 'green' },
+        { 'elementId' : 'secondParent', 'expectedValue' : 'lime' },
+        { 'elementId' : 'thirdParent', 'expectedValue' : 'blue' },
+        { 'elementId' : 'fourthParent', 'expectedValue' : 'rgb(220, 20, 60)' },
+      ];
+
+      function runTests()
+      {
+        var resultsElement = document.getElementById('results');
+
+        tests.forEach(function(curTest) {
+          var msg = document.createElement('div');
+          var element = document.querySelector("#" + curTest.elementId);
+
+          var mainMessage;
+          var matchedRules;
+          if (!element)
+              mainMessage = "Element with id #" + curTest.elementId + " not found";
+          else {
+              matchedRules = window.getMatchedCSSRules(element, "before");
+              if (!matchedRules || matchedRules.length !== 1)
+                  mainMessage = " Matching rules for #" + curTest.elementId + ":before do not contain a single rule";
+          }
+          if (mainMessage) {
+              msg.innerHTML = "<span class='fail'>FAIL</span>" + mainMessage;
+              resultsElement.appendChild(msg);
+              return;
+          }
+          var value = matchedRules[0].style.color;
+          mainMessage = " Expected '" + curTest.expectedValue + "' for color in the matched CSS rule for element with id " +
+              curTest.elementId + " and pseudo-element :before";
+          if (value == curTest.expectedValue)
+              msg.innerHTML = "<span class='pass'>PASS</span>" + mainMessage + " and got '" + value + "'";
+          else
+              msg.innerHTML = "<span class='fail'>FAIL</span>" + mainMessage + " but instead got '" + value + "'";
+          resultsElement.appendChild(msg);
+        });
+
+        if (window.testRunner)
+            testRunner.notifyDone();
+      }
+    </script>
+  </head>
+  <body _onload_="runTests();">
+    <h3>Test for <a href="" bug 78595</a>: CollectingRules and QueryingRules modes of SelectorChecker miss some complex selectors with pseudo elements</h3>
+    <br />
+    <div class="topParent topmost">
+      <div class="parent predecessor"></div>
+      <div class="parent firstParent" id="firstParent"></div>
+      <div class="parent secondParent" id="secondParent">
+        <div class="firstChild" id="firstChild"></div>
+        <div class="secondChild" id="secondChild"></div>
+        <div class="thirdChild" id="thirdChild"></div>
+        <div class="fourthChild" id="fourthChild"></div>
+      </div>
+      <div class="parent thirdParent" id="thirdParent"></div>
+      <div class="parent fourthParent" id="fourthParent"></div>
+    </div>
+    <br />
+    <div id="results"></div>
+  </body>
+</html>
Property changes on: trunk/LayoutTests/fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex.html
___________________________________________________________________

Added: svn:eol-style

Modified: trunk/Source/WebCore/ChangeLog (129745 => 129746)


--- trunk/Source/WebCore/ChangeLog	2012-09-27 09:44:05 UTC (rev 129745)
+++ trunk/Source/WebCore/ChangeLog	2012-09-27 09:55:52 UTC (rev 129746)
@@ -1,3 +1,18 @@
+2012-09-25  Alexander Pavlov  <apav...@chromium.org>
+
+        CollectingRules and QueryingRules modes of SelectorChecker miss some complex selectors with pseudo elements
+        https://bugs.webkit.org/show_bug.cgi?id=78595
+
+        Reviewed by Antti Koivisto.
+
+        Do not use the same dynamicPseudo reference when recursively invoking checkSelector() for non-SubSelector selectors.
+
+        Test: fast/dom/Window/getMatchedCSSRules-with-pseudo-elements-complex.html
+
+        * css/SelectorChecker.cpp:
+        (WebCore::SelectorChecker::checkSelector): Use new NOPSEUDO dynamic pseudoId values
+        for each non-SubSelector selector further in the tag history.
+
 2012-09-27  Christophe Dumez  <christophe.du...@intel.com>
 
         [EFL] No way to exit video fullscreen mode once entered

Modified: trunk/Source/WebCore/css/SelectorChecker.cpp (129745 => 129746)


--- trunk/Source/WebCore/css/SelectorChecker.cpp	2012-09-27 09:44:05 UTC (rev 129745)
+++ trunk/Source/WebCore/css/SelectorChecker.cpp	2012-09-27 09:55:52 UTC (rev 129746)
@@ -468,8 +468,13 @@
         // Disable :visited matching when we see the first link or try to match anything else than an ancestors.
         if (!context.isSubSelector && (context.element->isLink() || (relation != CSSSelector::Descendant && relation != CSSSelector::Child)))
             nextContext.visitedMatchType = VisitedMatchDisabled;
+
+        nextContext.pseudoStyle = NOPSEUDO;
     }
 
+    PseudoId nonSubSelectorPseudo(NOPSEUDO);
+    PseudoId& currentDynamicPseudo = relation == CSSSelector::SubSelector ? dynamicPseudo : nonSubSelectorPseudo;
+
     switch (relation) {
     case CSSSelector::Descendant:
         nextContext.element = context.element->parentElement();
@@ -477,7 +482,7 @@
         nextContext.elementStyle = 0;
         nextContext.elementParentStyle = 0;
         for (; nextContext.element; nextContext.element = nextContext.element->parentElement()) {
-            SelectorMatch match = checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
+            SelectorMatch match = checkSelector(nextContext, currentDynamicPseudo, hasUnknownPseudoElements);
             if (match == SelectorMatches || match == SelectorFailsCompletely)
                 return match;
             if (nextContext.element == nextContext.scope)
@@ -492,7 +497,7 @@
         nextContext.isSubSelector = false;
         nextContext.elementStyle = 0;
         nextContext.elementParentStyle = 0;
-        return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
+        return checkSelector(nextContext, currentDynamicPseudo, hasUnknownPseudoElements);
 
     case CSSSelector::DirectAdjacent:
         if (m_mode == ResolvingStyle && context.element->parentElement()) {
@@ -506,7 +511,7 @@
         nextContext.isSubSelector = false;
         nextContext.elementStyle = 0;
         nextContext.elementParentStyle = 0;
-        return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
+        return checkSelector(nextContext, currentDynamicPseudo, hasUnknownPseudoElements);
 
     case CSSSelector::IndirectAdjacent:
         if (m_mode == ResolvingStyle && context.element->parentElement()) {
@@ -519,7 +524,7 @@
         nextContext.elementStyle = 0;
         nextContext.elementParentStyle = 0;
         for (; nextContext.element; nextContext.element = nextContext.element->previousElementSibling()) {
-            SelectorMatch match = checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
+            SelectorMatch match = checkSelector(nextContext, currentDynamicPseudo, hasUnknownPseudoElements);
             if (match == SelectorMatches || match == SelectorFailsAllSiblings || match == SelectorFailsCompletely)
                 return match;
         };
@@ -533,7 +538,7 @@
              && !((RenderScrollbar::scrollbarForStyleResolve() || dynamicPseudo == SCROLLBAR_CORNER || dynamicPseudo == RESIZER) && nextContext.selector->m_match == CSSSelector::PseudoClass))
             return SelectorFailsCompletely;
         nextContext.isSubSelector = true;
-        return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
+        return checkSelector(nextContext, currentDynamicPseudo, hasUnknownPseudoElements);
 
     case CSSSelector::ShadowDescendant:
         {
@@ -547,7 +552,7 @@
             nextContext.isSubSelector = false;
             nextContext.elementStyle = 0;
             nextContext.elementParentStyle = 0;
-            return checkSelector(nextContext, dynamicPseudo, hasUnknownPseudoElements);
+            return checkSelector(nextContext, currentDynamicPseudo, hasUnknownPseudoElements);
         }
     }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to