Issue #1592 - Part 3: Ensure only tree-abiding pseudo-elements will follow ::slotted()

7 files changed, 92 insertions, 28 deletions

diff --git a/layout/style/StyleRule.cpp b/layout/style/StyleRule.cpp
index 1b8c31c4ad..c9f14995de 100644
--- a/layout/style/StyleRule.cpp
+++ b/layout/style/StyleRule.cpp
@@ -318,7 +318,8 @@ nsCSSSelector::nsCSSSelector(void)
     mNext(nullptr),
     mNameSpace(kNameSpaceID_Unknown),
     mOperator(0),
-    mPseudoType(CSSPseudoElementType::NotPseudo)
+    mPseudoType(CSSPseudoElementType::NotPseudo),
+    mHybridPseudoType(CSSPseudoElementType::NotPseudo)
 {
   MOZ_COUNT_CTOR(nsCSSSelector);
 }
diff --git a/layout/style/StyleRule.h b/layout/style/StyleRule.h
index d619b5090b..f2af2717b5 100644
--- a/layout/style/StyleRule.h
+++ b/layout/style/StyleRule.h
@@ -177,6 +177,10 @@ public:
     return mLowercaseTag && !mCasedTag;
   }
 
+  inline bool IsHybridPseudoElement() const {
+    return HybridPseudoType() != mozilla::CSSPseudoElementType::NotPseudo;
+  }
+
   // Calculate the specificity of this selector (not including its mNext!).
   int32_t CalcWeight() const;
 
@@ -218,6 +222,12 @@ public:
   void SetPseudoType(mozilla::CSSPseudoElementType aType) {
     mPseudoType = aType;
   }
+  mozilla::CSSPseudoElementType HybridPseudoType() const {
+    return mHybridPseudoType;
+  }
+  void SetHybridPseudoType(mozilla::CSSPseudoElementType aType) {
+    mHybridPseudoType = aType;
+  }
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
@@ -241,6 +251,7 @@ private:
   // The underlying type of CSSPseudoElementType is uint8_t and
   // it packs well with mOperator. (char16_t + uint8_t is less than 32bits.)
   mozilla::CSSPseudoElementType mPseudoType;
+  mozilla::CSSPseudoElementType mHybridPseudoType;
 
   nsCSSSelector(const nsCSSSelector& aCopy) = delete;
   nsCSSSelector& operator=(const nsCSSSelector& aCopy) = delete;
diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp
index 42c1733b77..db29dc6441 100644
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -6101,6 +6101,8 @@ CSSParserImpl::ParsePseudoSelector(int32_t&              aDataMask,
   CSSEnabledState enabledState = EnabledState();
   CSSPseudoElementType pseudoElementType =
     nsCSSPseudoElements::GetPseudoType(pseudo, enabledState);
+  bool pseudoElementIsTreeAbiding =
+    nsCSSPseudoElements::IsTreeAbidingPseudoElement(pseudoElementType);
   CSSPseudoClassType pseudoClassType =
     nsCSSPseudoClasses::GetPseudoType(pseudo, enabledState);
   bool pseudoClassIsUserAction =
@@ -6125,19 +6127,21 @@ CSSParserImpl::ParsePseudoSelector(int32_t&              aDataMask,
     }
   }
 
-  // We handle the ::slotted() pseudo-element as if it it were a pseudo-class.
-  // This is because the spec allows it to be followed by ::after/::before,
-  // but our platform does not have a mechanism to handle multiple
-  // pseudo-elements. It would be tedious to refactor pseudo-element
-  // handling to accommodate for an edge case like this.
-  bool isSlotPseudo = false;
+  // We handle certain pseudo-elements as if they were a pseudo-class.
+  // Our platform does not have the mechanism to handle multiple
+  // pseudo-elements and proper storage if they have an argument.
+  CSSPseudoElementType hybridPseudoElementType =
+    CSSPseudoElementType::NotPseudo;
   if (parsingPseudoElement &&
-      pseudoElementType == CSSPseudoElementType::slotted) {
-    parsingPseudoElement = false;
+      nsCSSPseudoElements::IsHybridPseudoElement(pseudoElementType)) {
+    hybridPseudoElementType = pseudoElementType;
     pseudoElementType = CSSPseudoElementType::NotPseudo;
-    pseudoClassType = CSSPseudoClassType::slotted;
-    isSlotPseudo = true;
-    aFlags |= SelectorParsingFlags::eDisallowCombinators;
+    parsingPseudoElement = false;
+
+    if (hybridPseudoElementType == CSSPseudoElementType::slotted) {
+      pseudoClassType = CSSPseudoClassType::slotted;
+      aFlags |= SelectorParsingFlags::eDisallowCombinators;
+    }
   }
 
   isTreePseudo = (pseudoElementType == CSSPseudoElementType::XULTree);
@@ -6195,21 +6199,35 @@ CSSParserImpl::ParsePseudoSelector(int32_t&              aDataMask,
     return eSelectorParsingStatus_Error;
   }
 
-  if (aSelector.IsPseudoElement()) {
-    CSSPseudoElementType type = aSelector.PseudoType();
+  if (aSelector.IsPseudoElement() || aSelector.IsHybridPseudoElement()) {
+    CSSPseudoElementType type = aSelector.IsPseudoElement() ?
+                                aSelector.PseudoType() :
+                                aSelector.HybridPseudoType();
+    bool supportsTreeAbiding =
+      nsCSSPseudoElements::PseudoElementSupportsTreeAbiding(type);
+    bool supportsUserAction =
+      nsCSSPseudoElements::PseudoElementSupportsUserActionState(type);
     if (type >= CSSPseudoElementType::Count ||
-        !nsCSSPseudoElements::PseudoElementSupportsUserActionState(type)) {
-      // We only allow user action pseudo-classes on certain pseudo-elements.
+        (!supportsTreeAbiding && !supportsUserAction)) {
+      // We only allow user action pseudo-classes and/or tree-abiding
+      // pseudo-elements on certain pseudo-elements.
       REPORT_UNEXPECTED_TOKEN(PEPseudoSelNoUserActionPC);
       UngetToken();
       return eSelectorParsingStatus_Error;
     }
-    if (!isPseudoClass || !pseudoClassIsUserAction) {
+
+    if (isPseudoClass &&
+        (!supportsUserAction || !pseudoClassIsUserAction)) {
       // CSS 4 Selectors says that pseudo-elements can only be followed by
       // a user action pseudo-class.
       REPORT_UNEXPECTED_TOKEN(PEPseudoClassNotUserAction);
       UngetToken();
       return eSelectorParsingStatus_Error;
+    } else if (isPseudoElement &&
+               (!supportsTreeAbiding || !pseudoElementIsTreeAbiding)) {
+      REPORT_UNEXPECTED_TOKEN(PEPseudoClassNotUserAction);
+      UngetToken();
+      return eSelectorParsingStatus_Error;
     }
   }
 
@@ -6239,9 +6257,9 @@ CSSParserImpl::ParsePseudoSelector(int32_t&              aDataMask,
           return parsingStatus;
         }
       }
-      else if (CSSPseudoClassType::slotted == pseudoClassType &&
-               !isSlotPseudo) {
-        // Reject the :slotted() pseudo-class form.
+      else if (nsCSSPseudoClasses::IsHybridPseudoElement(pseudoClassType) &&
+               hybridPseudoElementType == CSSPseudoElementType::NotPseudo) {
+        // Reject the single colon syntax for hybrid pseudo-elements.
         REPORT_UNEXPECTED_TOKEN(PEPseudoSelNewStyleOnly);
         UngetToken();
         return eSelectorParsingStatus_Error;
@@ -6257,12 +6275,13 @@ CSSParserImpl::ParsePseudoSelector(int32_t&              aDataMask,
       else {
         MOZ_ASSERT(nsCSSPseudoClasses::HasSelectorListArg(pseudoClassType),
                    "unexpected pseudo with function token");
-        // Ensure that the ::slotted() pseudo-element is rejected if
-        // pseudo-elements are disallowed.
-        if (CSSPseudoClassType::slotted == pseudoClassType &&
-            disallowPseudoElements) {
-          UngetToken();
-          return eSelectorParsingStatus_Error;
+        if (hybridPseudoElementType != CSSPseudoElementType::NotPseudo) {
+          aSelector.SetHybridPseudoType(hybridPseudoElementType);
+          // Ensure hybrid pseudo-elements are rejected if they're not allowed.
+          if (disallowPseudoElements) {
+            UngetToken();
+            return eSelectorParsingStatus_Error;
+          }
         }
         parsingStatus = ParsePseudoClassWithSelectorListArg(aSelector,
                                                             pseudoClassType,
@@ -6747,7 +6766,8 @@ CSSParserImpl::ParseSelector(nsCSSSelectorList* aList,
         selector->mClassList = pseudoElementArgs.forget();
         selector->SetPseudoType(pseudoElementType);
       }
-    } else if (selector->IsPseudoElement()) {
+    } else if (selector->IsPseudoElement() ||
+               selector->IsHybridPseudoElement()) {
       // Once we parsed a pseudo-element, we can only parse
       // pseudo-classes (and only a limited set, which
       // ParsePseudoSelector knows how to handle).
diff --git a/layout/style/nsCSSPseudoElementList.h b/layout/style/nsCSSPseudoElementList.h
index 93ce44e788..6693d42e75 100644
--- a/layout/style/nsCSSPseudoElementList.h
+++ b/layout/style/nsCSSPseudoElementList.h
@@ -30,7 +30,8 @@ CSS_PSEUDO_ELEMENT(before, ":before", CSS_PSEUDO_ELEMENT_IS_CSS2)
 
 // XXX: ::slotted() is treated as if it were a pseudo-class, and
 // is never parsed as a pseudo-element.
-CSS_PSEUDO_ELEMENT(slotted, ":slotted", 0)
+CSS_PSEUDO_ELEMENT(slotted, ":slotted",
+                   CSS_PSEUDO_ELEMENT_SUPPORTS_TREE_ABIDING)
 
 CSS_PSEUDO_ELEMENT(backdrop, ":backdrop", 0)
 
diff --git a/layout/style/nsCSSPseudoElements.cpp b/layout/style/nsCSSPseudoElements.cpp
index 49621374fe..26a45bbbc6 100644
--- a/layout/style/nsCSSPseudoElements.cpp
+++ b/layout/style/nsCSSPseudoElements.cpp
@@ -75,6 +75,22 @@ nsCSSPseudoElements::IsCSS2PseudoElement(nsIAtom *aAtom)
   return result;
 }
 
+/* static */ bool
+nsCSSPseudoElements::IsHybridPseudoElement(CSSPseudoElementType aType)
+{
+  return aType == CSSPseudoElementType::slotted;
+}
+
+/* static */ bool
+nsCSSPseudoElements::IsTreeAbidingPseudoElement(CSSPseudoElementType aType)
+{
+  // TODO: ::marker should be added here once we have support for it.
+  return aType == CSSPseudoElementType::after ||
+         aType == CSSPseudoElementType::before ||
+         aType == CSSPseudoElementType::mozPlaceholder ||
+         aType == CSSPseudoElementType::placeholder;
+}
+
 /* static */ CSSPseudoElementType
 nsCSSPseudoElements::GetPseudoType(nsIAtom *aAtom, EnabledState aEnabledState)
 {
diff --git a/layout/style/nsCSSPseudoElements.h b/layout/style/nsCSSPseudoElements.h
index 90db43c7d3..69dcad3de5 100644
--- a/layout/style/nsCSSPseudoElements.h
+++ b/layout/style/nsCSSPseudoElements.h
@@ -40,6 +40,10 @@
 // API for creating pseudo-implementing native anonymous content in JS with this
 // pseudo-element?
 #define CSS_PSEUDO_ELEMENT_IS_JS_CREATED_NAC           (1<<5)
+// Is this pseudo-element a pseudo-element that supports a tree-abiding
+// pseudo-element following it, such as ::after or ::before? See
+// https://w3c.github.io/csswg-drafts/css-pseudo-4/#tree-abiding.
+#define CSS_PSEUDO_ELEMENT_SUPPORTS_TREE_ABIDING       (1<<6)
 
 namespace mozilla {
 
@@ -78,6 +82,10 @@ public:
 
   static bool IsCSS2PseudoElement(nsIAtom *aAtom);
 
+  static bool IsHybridPseudoElement(Type aType);
+
+  static bool IsTreeAbidingPseudoElement(Type aType);
+
 #define CSS_PSEUDO_ELEMENT(_name, _value, _flags) \
   static nsICSSPseudoElement* _name;
 #include "nsCSSPseudoElementList.h"
@@ -105,6 +113,12 @@ public:
     return PseudoElementHasFlags(aType, CSS_PSEUDO_ELEMENT_IS_JS_CREATED_NAC);
   }
 
+  static bool PseudoElementSupportsTreeAbiding(const Type aType)
+  {
+    return PseudoElementHasFlags(aType,
+                                 CSS_PSEUDO_ELEMENT_SUPPORTS_TREE_ABIDING);
+  }
+
   static bool IsEnabled(Type aType, EnabledState aEnabledState)
   {
     return !PseudoElementHasFlags(aType, CSS_PSEUDO_ELEMENT_UA_SHEET_ONLY) ||
diff --git a/layout/style/nsCSSRuleProcessor.cpp b/layout/style/nsCSSRuleProcessor.cpp
index 953c84237d..fb35b65bd6 100644
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -1372,6 +1372,7 @@ static inline bool ActiveHoverQuirkMatches(nsCSSSelector* aSelector,
   if (aSelector->HasTagSelector() || aSelector->mAttrList ||
       aSelector->mIDList || aSelector->mClassList ||
       aSelector->IsPseudoElement() ||
+      aSelector->IsHybridPseudoElement() ||
       // Having this quirk means that some selectors will no longer match,
       // so it's better to return false when we aren't sure (i.e., the
       // flags are unknown).