From 453f48a20f7850d89c505b1bc0ee4401dfa2560d Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@mit.edu>
Date: Thu, 10 Dec 2009 14:36:07 -0800
Subject: [PATCH] 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 | 19 ++++++++++-
 layout/style/nsCSSRuleProcessor.cpp   | 10 ++++++
 layout/style/nsCSSRuleProcessor.h     |  4 ++-
 layout/style/test/Makefile.in         |  1 +
 layout/style/test/test_bug525952.html | 47 +++++++++++++++++++++++++++
 5 files changed, 79 insertions(+), 2 deletions(-)
 create mode 100644 layout/style/test/test_bug525952.html

diff --git a/content/base/src/nsGenericElement.cpp b/content/base/src/nsGenericElement.cpp
index 5e8e044e040f..cfce03ad98ee 100644
--- a/content/base/src/nsGenericElement.cpp
+++ b/content/base/src/nsGenericElement.cpp
@@ -5140,6 +5140,8 @@ nsGenericElement::GetLinkTarget(nsAString& aTarget)
 }
 
 // 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,
@@ -5155,13 +5157,28 @@ ParseSelectorList(nsINode* aNode,
   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;
diff --git a/layout/style/nsCSSRuleProcessor.cpp b/layout/style/nsCSSRuleProcessor.cpp
index 16e91e29b6db..1b6fcacd4747 100644
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -1278,6 +1278,8 @@ static PRBool SelectorMatches(RuleProcessorData &data,
                               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 &&
@@ -2421,6 +2423,13 @@ AddRule(RuleValue* aRuleInfo, RuleCascadeData* aCascade)
   
   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
@@ -2736,6 +2745,7 @@ nsCSSRuleProcessor::SelectorListMatches(RuleProcessorData& aData,
   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)) {
diff --git a/layout/style/nsCSSRuleProcessor.h b/layout/style/nsCSSRuleProcessor.h
index 9bfe38f07208..300f934261e8 100644
--- a/layout/style/nsCSSRuleProcessor.h
+++ b/layout/style/nsCSSRuleProcessor.h
@@ -83,7 +83,9 @@ public:
   /*
    * 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);
diff --git a/layout/style/test/Makefile.in b/layout/style/test/Makefile.in
index a1c80697c147..7f43232eddaa 100644
--- a/layout/style/test/Makefile.in
+++ b/layout/style/test/Makefile.in
@@ -172,6 +172,7 @@ _TEST_FILES =	test_acid3_test46.html \
 		media_queries_dynamic_xbl_style.css \
 		bug453896_iframe.html \
 		bug517224.sjs \
+		test_bug525952.html \
 		$(NULL)
 
 _BROWSER_FILES = \
diff --git a/layout/style/test/test_bug525952.html b/layout/style/test/test_bug525952.html
new file mode 100644
index 000000000000..a0761d2e1793
--- /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>