Bug 635286. Fix dynamic handling of :not() inside :-moz-any(). r=dbaron, a=dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 24 Feb 2011 13:42:15 -0500
changeset 63053 66167f6ecccef47a89dfb126de9726aee1513da5
parent 63052 a8e0f63f173fd735293564157a19487ca2006355
child 63054 5509926d2efd996422d70d950463e83d83425359
push id19006
push userbzbarsky@mozilla.com
push dateThu, 24 Feb 2011 18:42:43 +0000
treeherdermozilla-central@66167f6eccce [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron, dbaron
bugs635286
milestone2.0b13pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 635286. Fix dynamic handling of :not() inside :-moz-any(). r=dbaron, a=dbaron
layout/style/nsCSSRuleProcessor.cpp
layout/style/test/Makefile.in
layout/style/test/test_bug635286.html
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -2715,94 +2715,105 @@ PRBool IsStateSelector(nsCSSSelector& aS
 
 static PRBool
 AddSelector(RuleCascadeData* aCascade,
             // The part between combinators at the top level of the selector
             nsCSSSelector* aSelectorInTopLevel,
             // The part we should look through (might be in :not or :-moz-any())
             nsCSSSelector* aSelectorPart)
 {
-  // Track both document states and attribute dependence in pseudo-classes.
-  for (nsPseudoClassList* pseudoClass = aSelectorPart->mPseudoClassList;
-       pseudoClass; pseudoClass = pseudoClass->mNext) {
-    switch (pseudoClass->mType) {
-      case nsCSSPseudoClasses::ePseudoClass_mozLocaleDir: {
-        aCascade->mSelectorDocumentStates |= NS_DOCUMENT_STATE_RTL_LOCALE;
-        break;
+  // 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.
+  for (nsCSSSelector* negation = aSelectorPart; negation;
+       negation = negation->mNegations) {
+    // Track both document states and attribute dependence in pseudo-classes.
+    for (nsPseudoClassList* pseudoClass = negation->mPseudoClassList;
+         pseudoClass; pseudoClass = pseudoClass->mNext) {
+      switch (pseudoClass->mType) {
+        case nsCSSPseudoClasses::ePseudoClass_mozLocaleDir: {
+          aCascade->mSelectorDocumentStates |= NS_DOCUMENT_STATE_RTL_LOCALE;
+          break;
+        }
+        case nsCSSPseudoClasses::ePseudoClass_mozWindowInactive: {
+          aCascade->mSelectorDocumentStates |= NS_DOCUMENT_STATE_WINDOW_INACTIVE;
+          break;
+        }
+        case nsCSSPseudoClasses::ePseudoClass_mozTableBorderNonzero: {
+          nsTArray<nsCSSSelector*> *array =
+            aCascade->AttributeListFor(nsGkAtoms::border);
+          if (!array) {
+            return PR_FALSE;
+          }
+          array->AppendElement(aSelectorInTopLevel);
+          break;
+        }
+        default: {
+          break;
+        }
       }
-      case nsCSSPseudoClasses::ePseudoClass_mozWindowInactive: {
-        aCascade->mSelectorDocumentStates |= NS_DOCUMENT_STATE_WINDOW_INACTIVE;
-        break;
+    }
+
+    // Build mStateSelectors.
+    if (IsStateSelector(*negation))
+      aCascade->mStateSelectors.AppendElement(aSelectorInTopLevel);
+
+    // Build mIDSelectors
+    if (negation->mIDList) {
+      aCascade->mIDSelectors.AppendElement(aSelectorInTopLevel);
+    }
+
+    // Build mClassSelectors
+    if (negation == aSelectorInTopLevel) {
+      for (nsAtomList* curClass = negation->mClassList; curClass;
+           curClass = curClass->mNext) {
+        ClassSelectorEntry *entry =
+          static_cast<ClassSelectorEntry*>(PL_DHashTableOperate(&aCascade->mClassSelectors,
+                                                                curClass->mAtom,
+                                                                PL_DHASH_ADD));
+        if (entry) {
+          entry->mSelectors.AppendElement(aSelectorInTopLevel);
+        }
       }
-      case nsCSSPseudoClasses::ePseudoClass_mozTableBorderNonzero: {
-        nsTArray<nsCSSSelector*> *array =
-          aCascade->AttributeListFor(nsGkAtoms::border);
+    } else if (negation->mClassList) {
+      aCascade->mPossiblyNegatedClassSelectors.AppendElement(aSelectorInTopLevel);
+    }
+
+    // Build mAttributeSelectors.
+    for (nsAttrSelector *attr = negation->mAttrList; attr;
+         attr = attr->mNext) {
+      nsTArray<nsCSSSelector*> *array =
+        aCascade->AttributeListFor(attr->mCasedAttr);
+      if (!array) {
+        return PR_FALSE;
+      }
+      array->AppendElement(aSelectorInTopLevel);
+      if (attr->mLowercaseAttr != attr->mCasedAttr) {
+        array = aCascade->AttributeListFor(attr->mLowercaseAttr);
         if (!array) {
           return PR_FALSE;
         }
         array->AppendElement(aSelectorInTopLevel);
-        break;
-      }
-      default: {
-        break;
-      }
-    }
-  }
-
-  // Build mStateSelectors.
-  if (IsStateSelector(*aSelectorPart))
-    aCascade->mStateSelectors.AppendElement(aSelectorInTopLevel);
-
-  // Build mIDSelectors
-  if (aSelectorPart->mIDList) {
-    aCascade->mIDSelectors.AppendElement(aSelectorInTopLevel);
-  }
-
-  // Build mClassSelectors
-  if (aSelectorPart == aSelectorInTopLevel) {
-    for (nsAtomList* curClass = aSelectorPart->mClassList; curClass;
-         curClass = curClass->mNext) {
-      ClassSelectorEntry *entry =
-        static_cast<ClassSelectorEntry*>(PL_DHashTableOperate(&aCascade->mClassSelectors,
-                                                              curClass->mAtom,
-                                                              PL_DHASH_ADD));
-      if (entry) {
-        entry->mSelectors.AppendElement(aSelectorInTopLevel);
       }
     }
-  } else if (aSelectorPart->mClassList) {
-    aCascade->mPossiblyNegatedClassSelectors.AppendElement(aSelectorInTopLevel);
-  }
-
-  // Build mAttributeSelectors.
-  for (nsAttrSelector *attr = aSelectorPart->mAttrList; attr;
-       attr = attr->mNext) {
-    nsTArray<nsCSSSelector*> *array =
-      aCascade->AttributeListFor(attr->mCasedAttr);
-    if (!array) {
-      return PR_FALSE;
-    }
-    array->AppendElement(aSelectorInTopLevel);
-    if (attr->mLowercaseAttr != attr->mCasedAttr) {
-      array = aCascade->AttributeListFor(attr->mLowercaseAttr);
-      if (!array) {
-        return PR_FALSE;
-      }
-      array->AppendElement(aSelectorInTopLevel);
-    }
-  }
-
-  // Recur through any :-moz-any selectors
-  for (nsPseudoClassList* pseudoClass = aSelectorPart->mPseudoClassList;
-       pseudoClass; pseudoClass = pseudoClass->mNext) {
-    if (pseudoClass->mType == nsCSSPseudoClasses::ePseudoClass_any) {
-      for (nsCSSSelectorList *l = pseudoClass->u.mSelectors; l; l = l->mNext) {
-        nsCSSSelector *s = l->mSelectors;
-        if (!AddSelector(aCascade, aSelectorInTopLevel, s)) {
-          return PR_FALSE;
+
+    // Recur through any :-moz-any selectors
+    for (nsPseudoClassList* pseudoClass = negation->mPseudoClassList;
+         pseudoClass; pseudoClass = pseudoClass->mNext) {
+      if (pseudoClass->mType == nsCSSPseudoClasses::ePseudoClass_any) {
+        for (nsCSSSelectorList *l = pseudoClass->u.mSelectors; l; l = l->mNext) {
+          nsCSSSelector *s = l->mSelectors;
+          if (!AddSelector(aCascade, aSelectorInTopLevel, s)) {
+            return PR_FALSE;
+          }
         }
       }
     }
   }
 
   return PR_TRUE;
 }
 
@@ -2864,29 +2875,18 @@ AddRule(RuleSelectorPair* aRuleInfo, Rul
            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. 
-    for (nsCSSSelector* negation = selector; negation;
-         negation = negation->mNegations) {
-      if (!AddSelector(cascade, selector, negation)) {
-        return PR_FALSE;
-      }
+    if (!AddSelector(cascade, selector, selector)) {
+      return PR_FALSE;
     }
   }
 
   return PR_TRUE;
 }
 
 struct PerWeightData {
   PRInt32 mWeight;
--- a/layout/style/test/Makefile.in
+++ b/layout/style/test/Makefile.in
@@ -130,16 +130,17 @@ GARBAGE += css_properties.js
 		test_bug470769.html \
 		test_bug499655.html \
 		test_bug499655.xhtml \
 		test_bug517224.html \
 		test_bug524175.html \
 		test_bug534804.html \
 		test_bug573255.html \
 		test_bug580685.html \
+		test_bug635286.html \
 		test_cascade.html \
 		test_compute_data_with_start_struct.html \
 		test_computed_style.html \
 		test_computed_style_no_pseudo.html \
 		test_css_cross_domain.html \
 		test_css_eof_handling.html \
 		test_descriptor_storage.html \
 		test_descriptor_syntax_errors.html \
new file mode 100644
--- /dev/null
+++ b/layout/style/test/test_bug635286.html
@@ -0,0 +1,51 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=635286
+-->
+<head>
+  <title>Test for Bug 635286</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+  <style type="text/css">
+    div { background: transparent; }
+    :-moz-any(#case1.before)      { background: gray; }
+    #case2:not(.after)            { background: gray; }
+    :-moz-any(#case3:not(.after)) { background: gray; }
+    #case4:not(:-moz-any(.after)) { background: gray; }
+  </style>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=635286">Mozilla Bug 635286</a>
+<div id="case1" class="before">case1, :-moz-any()</div>
+<div id="case2" class="before">case2, :not()</div>
+<div id="case3" class="before">case3, :not() in :-moz-any()</div>
+<div id="case4" class="before">case4, :-moz-any() in :not()</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Bug 635286 **/
+
+window.addEventListener("load", function() {
+  var cases = Array.slice(document.getElementsByTagName("div"));
+  cases.forEach(function(aCase, aIndex) {
+    aCase.className = "after";
+  });
+  window.setTimeout(function() {
+    cases.forEach(function(aCase, aIndex) {
+      is(window.getComputedStyle(aCase, null)
+           .getPropertyValue("background-color"),
+         "transparent",
+         aCase.textContent);
+    });
+    SimpleTest.finish();
+  }, 1);
+}, false);
+
+SimpleTest.waitForExplicitFinish();
+
+</script>
+</pre>
+</body>
+</html>