Bug 525952 part 1. Make sure pseudo-element selectors never land inside SelectorMatches (because tree pseudo-elements do weird stuff with pseudo-classes). r=dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 11 Dec 2009 02:37:41 -0500
changeset 35560 626e14ae4183e9ef681af16b00d9577794fd6a98
parent 35559 9c8bd114fd20ae1ad6966602f92cea46c26d65d8
child 35561 5bcdfa447ae5683c8c89b045f755987577325648
push id10635
push userbzbarsky@mozilla.com
push dateFri, 11 Dec 2009 07:38:06 +0000
treeherdermozilla-central@9af2a428dcb1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs525952
milestone1.9.3a1pre
Bug 525952 part 1. Make sure pseudo-element selectors never land inside SelectorMatches (because tree pseudo-elements do weird stuff with pseudo-classes). r=dbaron
content/base/src/nsGenericElement.cpp
layout/style/nsCSSRuleProcessor.cpp
layout/style/nsCSSRuleProcessor.h
layout/style/test/Makefile.in
layout/style/test/test_bug525952.html
--- a/content/base/src/nsGenericElement.cpp
+++ b/content/base/src/nsGenericElement.cpp
@@ -5135,38 +5135,55 @@ nsGenericElement::PostHandleEventForLink
 
 void
 nsGenericElement::GetLinkTarget(nsAString& aTarget)
 {
   aTarget.Truncate();
 }
 
 // NOTE: The aPresContext pointer is NOT addrefed.
+// *aSelectorList might be null even if NS_OK is returned; this
+// happens when all the selectors were pseudo-element selectors.
 static nsresult
 ParseSelectorList(nsINode* aNode,
                   const nsAString& aSelectorString,
                   nsCSSSelectorList** aSelectorList,
                   nsPresContext** aPresContext)
 {
   NS_ENSURE_ARG(aNode);
 
   nsIDocument* doc = aNode->GetOwnerDoc();
   NS_ENSURE_STATE(doc);
 
   nsCOMPtr<nsICSSParser> parser;
   nsresult rv = doc->CSSLoader()->GetParserFor(nsnull, getter_AddRefs(parser));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  nsCSSSelectorList* selectorList;
   rv = parser->ParseSelectorString(aSelectorString,
                                    doc->GetDocumentURI(),
                                    0, // XXXbz get the right line number!
-                                   aSelectorList);
+                                   &selectorList);
   doc->CSSLoader()->RecycleParser(parser);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  // Filter out pseudo-element selectors from selectorList
+  nsCSSSelectorList** slot = &selectorList;
+  do {
+    nsCSSSelectorList* cur = *slot;
+    if (cur->mSelectors->IsPseudoElement()) {
+      *slot = cur->mNext;
+      cur->mNext = nsnull;
+      delete cur;
+    } else {
+      slot = &cur->mNext;
+    }
+  } while (*slot);
+  *aSelectorList = selectorList;
+
   // It's not strictly necessary to have a prescontext here, but it's
   // a bit of an optimization for various stuff.
   *aPresContext = nsnull;
   nsIPresShell* shell = doc->GetPrimaryShell();
   if (shell) {
     *aPresContext = shell->GetPresContext();
   }
 
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -1273,16 +1273,18 @@ static PRBool AttrMatchesValue(const nsA
 //    because of aStateMask
 static PRBool SelectorMatches(RuleProcessorData &data,
                               nsCSSSelector* aSelector,
                               PRInt32 aStateMask, // states NOT to test
                               PRBool aForStyling,
                               PRBool* const aDependence = nsnull) 
 
 {
+  NS_PRECONDITION(!aSelector->IsPseudoElement(),
+                  "Pseudo-element snuck into SelectorMatches?");
   // namespace/tag match
   // optimization : bail out early if we can
   if ((kNameSpaceID_Unknown != aSelector->mNameSpace &&
        data.mNameSpaceID != aSelector->mNameSpace))
     return PR_FALSE;
 
   if (aSelector->mLowercaseTag && 
       (data.mIsHTML ? aSelector->mLowercaseTag : aSelector->mCasedTag) !=
@@ -2416,16 +2418,23 @@ AddRule(RuleValue* aRuleInfo, RuleCascad
   }
 
   nsTArray<nsCSSSelector*>* stateArray = &cascade->mStateSelectors;
   nsTArray<nsCSSSelector*>* classArray = &cascade->mClassSelectors;
   nsTArray<nsCSSSelector*>* idArray = &cascade->mIDSelectors;
   
   for (nsCSSSelector* selector = aRuleInfo->mSelector;
            selector; selector = selector->mNext) {
+    if (selector->IsPseudoElement()) {
+      NS_ASSERTION(!selector->mNegations, "Shouldn't have negations");
+      // Make sure these selectors don't end up in the hashtables we use to
+      // match against actual elements, no matter what.  Normally they wouldn't
+      // anyway, but trees overload mPseudoClassList with weird stuff.
+      continue;
+    }
     // It's worth noting that this loop over negations isn't quite
     // optimal for two reasons.  One, we could add something to one of
     // these lists twice, which means we'll check it twice, but I don't
     // think that's worth worrying about.   (We do the same for multiple
     // attribute selectors on the same attribute.)  Two, we don't really
     // need to check negations past the first in the current
     // implementation (and they're rare as well), but that might change
     // in the future if :not() is extended. 
@@ -2731,16 +2740,17 @@ nsCSSRuleProcessor::RefreshRuleCascade(n
 
 /* static */ PRBool
 nsCSSRuleProcessor::SelectorListMatches(RuleProcessorData& aData,
                                         nsCSSSelectorList* aSelectorList)
 {
   while (aSelectorList) {
     nsCSSSelector* sel = aSelectorList->mSelectors;
     NS_ASSERTION(sel, "Should have *some* selectors");
+    NS_ASSERTION(!sel->IsPseudoElement(), "Shouldn't have been called");
     if (SelectorMatches(aData, sel, 0, PR_FALSE)) {
       nsCSSSelector* next = sel->mNext;
       if (!next || SelectorMatchesTree(aData, next, PR_FALSE)) {
         return PR_TRUE;
       }
     }
 
     aSelectorList = aSelectorList->mNext;
--- a/layout/style/nsCSSRuleProcessor.h
+++ b/layout/style/nsCSSRuleProcessor.h
@@ -78,17 +78,19 @@ public:
 
   static void Startup();
   static void FreeSystemMetrics();
   static PRBool HasSystemMetric(nsIAtom* aMetric);
 
   /*
    * Returns true if the given RuleProcessorData matches one of the
    * selectors in aSelectorList.  Note that this method will assume
-   * the matching is not for styling purposes.
+   * the matching is not for styling purposes.  aSelectorList must not
+   * include any pseudo-element selectors.  aSelectorList is allowed
+   * to be null; in this case PR_FALSE will be returned.
    */
   static PRBool SelectorListMatches(RuleProcessorData& aData,
                                     nsCSSSelectorList* aSelectorList);
 
   // nsIStyleRuleProcessor
   NS_IMETHOD RulesMatching(ElementRuleProcessorData* aData);
 
   NS_IMETHOD RulesMatching(PseudoElementRuleProcessorData* aData);
--- a/layout/style/test/Makefile.in
+++ b/layout/style/test/Makefile.in
@@ -167,16 +167,17 @@ GARBAGE += css_properties.js
 		xbl_bindings.xml \
 		empty.html \
 		media_queries_iframe.html \
 		media_queries_dynamic_xbl_binding.xml \
 		media_queries_dynamic_xbl_iframe.html \
 		media_queries_dynamic_xbl_style.css \
 		bug453896_iframe.html \
 		bug517224.sjs \
+		test_bug525952.html \
 		$(NULL)
 
 _BROWSER_FILES = \
 		browser_bug453896.js \
 		bug453896_iframe.html \
 		media_queries_iframe.html \
 		$(NULL)
 
new file mode 100644
--- /dev/null
+++ b/layout/style/test/test_bug525952.html
@@ -0,0 +1,47 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=525952
+-->
+<head>
+  <title>Test for Bug 525952</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=525952">Mozilla Bug 525952</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 525952 **/
+var bodies = document.querySelectorAll("::before, div::before, body");
+is(bodies.length, 1, "Unexpected length");
+is(bodies[0], document.body, "Unexpected element");
+
+is(document.querySelector("div > ::after, body"), document.body,
+   "Unexpected return value");
+
+var emptyList = document.querySelectorAll("::before, div::before");
+is(emptyList.length, 0, "Unexpected empty list length");
+
+is(document.querySelectorAll("div > ::after").length, 0,
+   "Pseudo-element matched something?");
+
+is(document.body.mozMatchesSelector("::first-line"), false,
+   "body shouldn't match ::first-line");
+
+is(document.body.mozMatchesSelector("::first-line, body"), true,
+   "body should match 'body'");
+
+is(document.body.mozMatchesSelector("::first-line, body, ::first-letter"), true,
+   "body should match 'body' here too");
+
+</script>
+</pre>
+</body>
+</html>