From 0232bcdfa4e685689ff883023018989fd76d45e8 Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Sat, 18 Mar 2023 12:44:56 +0800 Subject: [PATCH 01/14] Issue #1592 - Part 1a: Prevent crashing if a slot element was selected via DOM Inspector --- layout/base/RestyleManager.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/layout/base/RestyleManager.cpp b/layout/base/RestyleManager.cpp index 9e80ef7bc7..da9b47ece9 100644 --- a/layout/base/RestyleManager.cpp +++ b/layout/base/RestyleManager.cpp @@ -3839,8 +3839,13 @@ RestyleManager::ComputeAndProcessStyleChange(nsStyleContext* aNewContext, MOZ_ASSERT(mReframingStyleContexts, "should have rsc"); MOZ_ASSERT(aNewContext->StyleDisplay()->mDisplay == StyleDisplay::Contents); nsIFrame* frame = GetNearestAncestorFrame(aElement); - MOZ_ASSERT(frame, "display:contents node in map although it's a " - "display:none descendant?"); + // Return early if we don't have a frame. + if (!frame) { + NS_ASSERTION(frame, + "display:contents node in map although it's a " + "display:none descendant?"); + return; + } TreeMatchContext treeMatchContext(true, nsRuleWalker::eRelevantLinkUnvisited, frame->PresContext()->Document()); From 460e8db94c8b1d370251ef67c5db948a438d327f Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Sat, 18 Mar 2023 17:04:55 +0800 Subject: [PATCH 02/14] Issue #1592 - Part 1b: Move UA rule to html.css Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1468127 --- layout/style/res/html.css | 6 ++++++ layout/style/res/ua.css | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/layout/style/res/html.css b/layout/style/res/html.css index 0ce8901252..93539c0890 100644 --- a/layout/style/res/html.css +++ b/layout/style/res/html.css @@ -893,3 +893,9 @@ rtc > rt { ruby, rb, rt, rtc { unicode-bidi: isolate; } + +/* Shadow DOM v1 + * https://drafts.csswg.org/css-scoping/#slots-in-shadow-tree */ +slot { + display: contents; +} diff --git a/layout/style/res/ua.css b/layout/style/res/ua.css index a8425d472a..ab51f67c53 100644 --- a/layout/style/res/ua.css +++ b/layout/style/res/ua.css @@ -474,9 +474,3 @@ div:-moz-native-anonymous.moz-custom-content-container { width: 100%; height: 100%; } - -/* Shadow DOM v1 - * https://drafts.csswg.org/css-scoping/#slots-in-shadow-tree */ -slot { - display: contents; -} \ No newline at end of file From ab63b7b94640427500fa22b9a70f033288cc3286 Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Sun, 19 Mar 2023 16:57:55 +0800 Subject: [PATCH 03/14] Issue #1592 - Part 1c: Pass SelectorParsingFlags as a reference --- layout/style/nsCSSParser.cpp | 50 +++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp index fee2445ec7..717c8ba465 100644 --- a/layout/style/nsCSSParser.cpp +++ b/layout/style/nsCSSParser.cpp @@ -776,7 +776,7 @@ protected: // aFlags is not set. nsSelectorParsingStatus ParsePseudoSelector(int32_t& aDataMask, nsCSSSelector& aSelector, - SelectorParsingFlags aFlags, + SelectorParsingFlags& aFlags, nsIAtom** aPseudoElement, nsAtomList** aPseudoElementArgs, CSSPseudoElementType* aPseudoElementType); @@ -784,9 +784,9 @@ protected: nsSelectorParsingStatus ParseAttributeSelector(int32_t& aDataMask, nsCSSSelector& aSelector); - nsSelectorParsingStatus ParseTypeOrUniversalSelector(int32_t& aDataMask, - nsCSSSelector& aSelector, - SelectorParsingFlags aFlags); + nsSelectorParsingStatus ParseTypeOrUniversalSelector(int32_t& aDataMask, + nsCSSSelector& aSelector, + SelectorParsingFlags& aFlags); nsSelectorParsingStatus ParsePseudoClassWithIdentArg(nsCSSSelector& aSelector, CSSPseudoClassType aType); @@ -796,22 +796,22 @@ protected: nsSelectorParsingStatus ParsePseudoClassWithSelectorListArg(nsCSSSelector& aSelector, CSSPseudoClassType aType, - SelectorParsingFlags aFlags); + SelectorParsingFlags& aFlags); - nsSelectorParsingStatus ParseNegatedSimpleSelector(int32_t& aDataMask, - nsCSSSelector& aSelector, - SelectorParsingFlags aFlags); + nsSelectorParsingStatus ParseNegatedSimpleSelector(int32_t& aDataMask, + nsCSSSelector& aSelector, + SelectorParsingFlags& aFlags); // If aStopChar is non-zero, the selector list is done when we hit // aStopChar. Otherwise, it's done when we hit EOF. bool ParseSelectorList(nsCSSSelectorList*& aListHead, char16_t aStopChar, - SelectorParsingFlags aFlags = SelectorParsingFlags::eNone); + SelectorParsingFlags& aFlags); bool ParseSelectorGroup(nsCSSSelectorList*& aListHead, - SelectorParsingFlags aFlags); + SelectorParsingFlags& aFlags); bool ParseSelector(nsCSSSelectorList* aList, char16_t aPrevCombinator, - SelectorParsingFlags aFlags); + SelectorParsingFlags& aFlags); enum { eParseDeclaration_InBraces = 1 << 0, @@ -2343,7 +2343,8 @@ CSSParserImpl::ParseSelectorString(const nsSubstring& aSelectorString, css::ErrorReporter reporter(scanner, mSheet, mChildLoader, aURI); InitScanner(scanner, reporter, aURI, aURI, nullptr); - bool success = ParseSelectorList(*aSelectorList, char16_t(0)); + SelectorParsingFlags flags = SelectorParsingFlags::eNone; + bool success = ParseSelectorList(*aSelectorList, char16_t(0), flags); // We deliberately do not call OUTPUT_ERROR here, because all our // callers map a failure return to a JS exception, and if that JS @@ -5457,9 +5458,10 @@ CSSParserImpl::ParseRuleSet(RuleAppendFunc aAppendFunc, void* aData, { // First get the list of selectors for the rule nsCSSSelectorList* slist = nullptr; + SelectorParsingFlags flags = SelectorParsingFlags::eNone; uint32_t linenum, colnum; if (!GetNextTokenLocation(true, &linenum, &colnum) || - !ParseSelectorList(slist, char16_t('{'))) { + !ParseSelectorList(slist, char16_t('{'), flags)) { REPORT_UNEXPECTED(PEBadSelectorRSIgnored); OUTPUT_ERROR(); SkipRuleSet(aInsideBraces); @@ -5496,7 +5498,7 @@ CSSParserImpl::ParseRuleSet(RuleAppendFunc aAppendFunc, void* aData, bool CSSParserImpl::ParseSelectorList(nsCSSSelectorList*& aListHead, char16_t aStopChar, - SelectorParsingFlags aFlags) + SelectorParsingFlags& aFlags) { nsCSSSelectorList* list = nullptr; if (! ParseSelectorGroup(list, aFlags)) { @@ -5580,7 +5582,7 @@ static bool IsUniversalSelector(const nsCSSSelector& aSelector) bool CSSParserImpl::ParseSelectorGroup(nsCSSSelectorList*& aList, - SelectorParsingFlags aFlags) + SelectorParsingFlags& aFlags) { char16_t combinator = 0; nsAutoPtr list(new nsCSSSelectorList()); @@ -5681,9 +5683,9 @@ CSSParserImpl::ParseClassSelector(int32_t& aDataMask, // namespace|type or namespace|* or *|* or * // CSSParserImpl::nsSelectorParsingStatus -CSSParserImpl::ParseTypeOrUniversalSelector(int32_t& aDataMask, - nsCSSSelector& aSelector, - SelectorParsingFlags aFlags) +CSSParserImpl::ParseTypeOrUniversalSelector(int32_t& aDataMask, + nsCSSSelector& aSelector, + SelectorParsingFlags& aFlags) { nsAutoString buffer; if (mToken.IsSymbol('*')) { // universal element selector, or universal namespace @@ -6048,7 +6050,7 @@ CSSParserImpl::ParseAttributeSelector(int32_t& aDataMask, CSSParserImpl::nsSelectorParsingStatus CSSParserImpl::ParsePseudoSelector(int32_t& aDataMask, nsCSSSelector& aSelector, - SelectorParsingFlags aFlags, + SelectorParsingFlags& aFlags, nsIAtom** aPseudoElement, nsAtomList** aPseudoElementArgs, CSSPseudoElementType* aPseudoElementType) @@ -6336,9 +6338,9 @@ CSSParserImpl::ParsePseudoSelector(int32_t& aDataMask, // Parse the argument of a negation pseudo-class :not() // CSSParserImpl::nsSelectorParsingStatus -CSSParserImpl::ParseNegatedSimpleSelector(int32_t& aDataMask, - nsCSSSelector& aSelector, - SelectorParsingFlags aFlags) +CSSParserImpl::ParseNegatedSimpleSelector(int32_t& aDataMask, + nsCSSSelector& aSelector, + SelectorParsingFlags& aFlags) { aFlags |= SelectorParsingFlags::eIsNegated; @@ -6616,7 +6618,7 @@ CSSParserImpl::ParsePseudoClassWithNthPairArg(nsCSSSelector& aSelector, CSSParserImpl::nsSelectorParsingStatus CSSParserImpl::ParsePseudoClassWithSelectorListArg(nsCSSSelector& aSelector, CSSPseudoClassType aType, - SelectorParsingFlags aFlags) + SelectorParsingFlags& aFlags) { bool isSingleSelector = nsCSSPseudoClasses::HasSingleSelectorArg(aType); @@ -6684,7 +6686,7 @@ CSSParserImpl::ParsePseudoClassWithSelectorListArg(nsCSSSelector& aSelector, bool CSSParserImpl::ParseSelector(nsCSSSelectorList* aList, char16_t aPrevCombinator, - SelectorParsingFlags aFlags) + SelectorParsingFlags& aFlags) { if (! GetToken(true)) { REPORT_UNEXPECTED_EOF(PESelectorEOF); From 77ad970db6ace890f2eeb97973b495d109e147ae Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Sat, 18 Mar 2023 09:36:25 +0800 Subject: [PATCH 04/14] Issue #1592 - Part 2: Parse ::slotted() pseudo-element as if it were a pseudo-class - Block slot elements from being matched by ::slotted - Ensure ::slotted() is serialized as a pseudo-element - Add pref to control whether the pseudo-class is enabled --- layout/style/StyleRule.cpp | 5 ++++ layout/style/nsCSSParser.cpp | 33 ++++++++++++++++++++++++++- layout/style/nsCSSPseudoClassList.h | 3 +++ layout/style/nsCSSPseudoClasses.cpp | 12 ++++++++-- layout/style/nsCSSPseudoClasses.h | 1 + layout/style/nsCSSPseudoElementList.h | 4 ++++ layout/style/nsCSSRuleProcessor.cpp | 23 +++++++++++++++++++ modules/libpref/init/all.js | 3 +++ 8 files changed, 81 insertions(+), 3 deletions(-) diff --git a/layout/style/StyleRule.cpp b/layout/style/StyleRule.cpp index 0ae939098a..1952087b90 100644 --- a/layout/style/StyleRule.cpp +++ b/layout/style/StyleRule.cpp @@ -915,6 +915,11 @@ nsCSSSelector::AppendToStringWithoutCombinatorsOrNegations // Append each pseudo-class in the linked list for (nsPseudoClassList* list = mPseudoClassList; list; list = list->mNext) { + // Serialize pseudo-elements that were treated as if they were a + // pseudo-class to the two colon syntax. + if (nsCSSPseudoClasses::IsHybridPseudoElement(list->mType)) { + aString.Append(char16_t(':')); + } nsCSSPseudoClasses::PseudoTypeToString(list->mType, temp); // This should not be escaped since (a) the pseudo-class string // has a ":" that can't be escaped and (b) all pseudo-classes at diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp index 717c8ba465..2e64df5b26 100644 --- a/layout/style/nsCSSParser.cpp +++ b/layout/style/nsCSSParser.cpp @@ -6121,6 +6121,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; + if (parsingPseudoElement && + pseudoElementType == CSSPseudoElementType::slotted) { + parsingPseudoElement = false; + pseudoElementType = CSSPseudoElementType::NotPseudo; + pseudoClassType = CSSPseudoClassType::slotted; + isSlotPseudo = true; + aFlags |= SelectorParsingFlags::eDisallowCombinators; + } + #ifdef MOZ_XUL isTreePseudo = (pseudoElementType == CSSPseudoElementType::XULTree); // If a tree pseudo-element is using the function syntax, it will @@ -6198,6 +6213,8 @@ CSSParserImpl::ParsePseudoSelector(int32_t& aDataMask, } } + bool disallowPseudoElements = + !!(aFlags & SelectorParsingFlags::eDisallowPseudoElements); if (!parsingPseudoElement && isPseudoClass) { aDataMask |= SEL_MASK_PCLASS; if (eCSSToken_Function == mToken.mType) { @@ -6222,6 +6239,13 @@ CSSParserImpl::ParsePseudoSelector(int32_t& aDataMask, return parsingStatus; } } + else if (CSSPseudoClassType::slotted == pseudoClassType && + !isSlotPseudo) { + // Reject the :slotted() pseudo-class form. + REPORT_UNEXPECTED_TOKEN(PEPseudoSelNewStyleOnly); + UngetToken(); + return eSelectorParsingStatus_Error; + } else if (nsCSSPseudoClasses::HasStringArg(pseudoClassType)) { parsingStatus = ParsePseudoClassWithIdentArg(aSelector, pseudoClassType); @@ -6233,6 +6257,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; + } parsingStatus = ParsePseudoClassWithSelectorListArg(aSelector, pseudoClassType, flags); @@ -6259,7 +6290,7 @@ CSSParserImpl::ParsePseudoSelector(int32_t& aDataMask, } // Pseudo-elements might not be allowed from appearing // (e.g. as an argument to the functional part of a pseudo-class). - if (aFlags & SelectorParsingFlags::eDisallowPseudoElements) { + if (disallowPseudoElements) { UngetToken(); return eSelectorParsingStatus_Error; } diff --git a/layout/style/nsCSSPseudoClassList.h b/layout/style/nsCSSPseudoClassList.h index cbe3bd8f92..6ec09714cc 100644 --- a/layout/style/nsCSSPseudoClassList.h +++ b/layout/style/nsCSSPseudoClassList.h @@ -93,6 +93,9 @@ CSS_PSEUDO_CLASS(nthLastChild, ":nth-last-child", 0, "") CSS_PSEUDO_CLASS(nthOfType, ":nth-of-type", 0, "") CSS_PSEUDO_CLASS(nthLastOfType, ":nth-last-of-type", 0, "") +// Match slot nodes. +CSS_PSEUDO_CLASS(slotted, ":slotted", 0, "layout.css.slotted-pseudo.enabled") + // Match nodes that are HTML but not XHTML CSS_PSEUDO_CLASS(mozIsHTML, ":-moz-is-html", 0, "") diff --git a/layout/style/nsCSSPseudoClasses.cpp b/layout/style/nsCSSPseudoClasses.cpp index 0fc460a514..3c9bbf9bd3 100644 --- a/layout/style/nsCSSPseudoClasses.cpp +++ b/layout/style/nsCSSPseudoClasses.cpp @@ -106,7 +106,8 @@ bool nsCSSPseudoClasses::HasSingleSelectorArg(Type aType) { return aType == Type::host || - aType == Type::hostContext; + aType == Type::hostContext || + aType == Type::slotted; } bool @@ -126,7 +127,8 @@ nsCSSPseudoClasses::HasSelectorListArg(Type aType) aType == Type::mozAny || aType == Type::mozAnyPrivate || aType == Type::host || - aType == Type::hostContext; + aType == Type::hostContext || + aType == Type::slotted; } bool @@ -169,3 +171,9 @@ nsCSSPseudoClasses::IsUserActionPseudoClass(Type aType) aType == Type::active || aType == Type::focus; } + +/* static */ bool +nsCSSPseudoClasses::IsHybridPseudoElement(Type aType) +{ + return aType == Type::slotted; +} diff --git a/layout/style/nsCSSPseudoClasses.h b/layout/style/nsCSSPseudoClasses.h index ff2da74ff0..e4738f64ba 100644 --- a/layout/style/nsCSSPseudoClasses.h +++ b/layout/style/nsCSSPseudoClasses.h @@ -64,6 +64,7 @@ public: static bool HasOptionalSelectorListArg(Type aType); static bool IsHiddenFromSerialization(Type aType); static bool IsUserActionPseudoClass(Type aType); + static bool IsHybridPseudoElement(Type aType); // Should only be used on types other than Count and NotPseudoClass static void PseudoTypeToString(Type aType, nsAString& aString); diff --git a/layout/style/nsCSSPseudoElementList.h b/layout/style/nsCSSPseudoElementList.h index b8393d3952..93ce44e788 100644 --- a/layout/style/nsCSSPseudoElementList.h +++ b/layout/style/nsCSSPseudoElementList.h @@ -28,6 +28,10 @@ CSS_PSEUDO_ELEMENT(after, ":after", CSS_PSEUDO_ELEMENT_IS_CSS2) 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(backdrop, ":backdrop", 0) CSS_PSEUDO_ELEMENT(firstLetter, ":first-letter", diff --git a/layout/style/nsCSSRuleProcessor.cpp b/layout/style/nsCSSRuleProcessor.cpp index 5618cfa6ac..9c685d151c 100644 --- a/layout/style/nsCSSRuleProcessor.cpp +++ b/layout/style/nsCSSRuleProcessor.cpp @@ -1963,6 +1963,29 @@ static bool SelectorMatches(Element* aElement, } break; + case CSSPseudoClassType::slotted: + { + // Slot elements cannot be matched. + if (aElement->IsHTMLElement(nsGkAtoms::slot)) { + return false; + } + + // The current element must have an assigned slot. + if (!aElement->GetAssignedSlot()) { + return false; + } + + NodeMatchContext nodeContext(EventStates(), + aNodeMatchContext.mIsRelevantLink); + if (!SelectorListMatches(aElement, + pseudoClass, + nodeContext, + aTreeMatchContext)) { + return false; + } + } + break; + case CSSPseudoClassType::host: { ShadowRoot* shadow = aElement->GetShadowRoot(); diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 10bd5aaf46..8f1cf2e9e9 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -2557,6 +2557,9 @@ pref("layout.css.legacy-negation-pseudo.enabled", false); // Is support for the :is() and :where() selectors enabled? pref("layout.css.is-where-pseudo.enabled", true); +// Is support for the ::slotted() selector enabled? +pref("layout.css.slotted-pseudo.enabled", true); + // Is support for the :scope selector enabled? pref("layout.css.scope-pseudo.enabled", true); From 92b31dd2555c7ce377ed0181ec1100dab7c9cdd9 Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Sun, 19 Mar 2023 15:47:12 +0800 Subject: [PATCH 05/14] Issue #1592 - Part 3: Ensure only tree-abiding pseudo-elements will follow ::slotted() --- layout/style/StyleRule.cpp | 3 +- layout/style/StyleRule.h | 11 ++++ layout/style/nsCSSParser.cpp | 72 +++++++++++++++++---------- layout/style/nsCSSPseudoElementList.h | 3 +- layout/style/nsCSSPseudoElements.cpp | 16 ++++++ layout/style/nsCSSPseudoElements.h | 14 ++++++ layout/style/nsCSSRuleProcessor.cpp | 1 + 7 files changed, 92 insertions(+), 28 deletions(-) diff --git a/layout/style/StyleRule.cpp b/layout/style/StyleRule.cpp index 1952087b90..9e90a63d56 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 2e64df5b26..39d81601c7 100644 --- a/layout/style/nsCSSParser.cpp +++ b/layout/style/nsCSSParser.cpp @@ -6097,6 +6097,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 = @@ -6121,19 +6123,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; + } } #ifdef MOZ_XUL @@ -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, @@ -6752,7 +6771,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 ef09f2d42a..fb871ff858 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 22c744ad06..f749959d16 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 { @@ -80,6 +84,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" @@ -107,6 +115,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 9c685d151c..543368709b 100644 --- a/layout/style/nsCSSRuleProcessor.cpp +++ b/layout/style/nsCSSRuleProcessor.cpp @@ -1393,6 +1393,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). From 518c41fd7aa62390efd3c3de714eafd75223e930 Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Sat, 18 Mar 2023 12:43:51 +0800 Subject: [PATCH 06/14] Issue #1592 - Part 4: Walk ::slotted()-containing rules for slottables - Check against all selector parts and not the leftmost selector only for ::slotted() - Walk rules for ::slotted() regardless if the shadow root is opened/closed - Ensure that ::slotted() rules are walked in the right order - Fix ::slotted inheritance from topmost shadow root --- dom/xbl/nsBindingManager.cpp | 39 +++++++++++ layout/style/nsCSSRuleProcessor.cpp | 104 +++++++++++++++------------- layout/style/nsRuleProcessorData.h | 8 +++ 3 files changed, 104 insertions(+), 47 deletions(-) diff --git a/dom/xbl/nsBindingManager.cpp b/dom/xbl/nsBindingManager.cpp index 8f18f112af..c6dc58ec89 100644 --- a/dom/xbl/nsBindingManager.cpp +++ b/dom/xbl/nsBindingManager.cpp @@ -681,6 +681,45 @@ nsBindingManager::WalkRules(nsIStyleRuleProcessor::EnumFunc aFunc, aData->mElementIsFeatureless = false; aData->mTreeMatchContext.mOnlyMatchHostPseudo = false; + // Walk the rules in shadow root for ::slotted() pseudo-element rules + // if we have an assigned slot. + if (aData->mElement->GetAssignedSlot()) { + aData->mTreeMatchContext.mRestrictToSlottedPseudo = true; + + AutoTArray stack; + bool foundTopmostScope = false; + for (nsIContent* parent = aData->mElement->GetFlattenedTreeParent(); + parent; + parent = parent->GetFlattenedTreeParent()) { + ShadowRoot* currentShadow = parent->GetShadowRoot(); + if (!currentShadow) { + continue; + } + + nsXBLBinding* binding = currentShadow->GetAssociatedBinding(); + if (!binding) { + continue; + } + stack.AppendElement(binding); + + if (!foundTopmostScope) { + aData->mTreeMatchContext.mScopedRoot = parent; + foundTopmostScope = true; + } + } + + while (!stack.IsEmpty()) { + uint32_t index = stack.Length() - 1; + nsXBLBinding* binding = stack.ElementAt(index); + stack.RemoveElementAt(index); + + aData->mTreeMatchContext.mIsTopmostScope = (index == 0); + binding->WalkRules(aFunc, aData); + } + + aData->mTreeMatchContext.mRestrictToSlottedPseudo = false; + } + // Walk the binding scope chain, starting with the binding attached to our // content, up till we run out of scopes or we get cut off. nsIContent *content = aData->mElement; diff --git a/layout/style/nsCSSRuleProcessor.cpp b/layout/style/nsCSSRuleProcessor.cpp index 543368709b..63c228fda2 100644 --- a/layout/style/nsCSSRuleProcessor.cpp +++ b/layout/style/nsCSSRuleProcessor.cpp @@ -2686,6 +2686,12 @@ SelectorMatchesTree(Element* aPrevElement, aTreeMatchContext.mCurrentStyleScope = styleScope; } selector = selector->mNext; + if (!selector && + !aTreeMatchContext.mIsTopmostScope && + aTreeMatchContext.mRestrictToSlottedPseudo && + aTreeMatchContext.mScopedRoot != element) { + return false; + } } else { // for adjacent sibling and child combinators, if we didn't find @@ -2764,6 +2770,52 @@ static bool SelectorListMatches(Element* aElement, aPreventComplexSelectors); } +static +inline bool LookForTargetPseudo(nsCSSSelector* aSelector, + TreeMatchContext* aMatchContext, + nsRestyleHint* possibleChange) { + if (aMatchContext->mOnlyMatchHostPseudo) { + while (aSelector && aSelector->mNext != nullptr) { + aSelector = aSelector->mNext; + } + + for (nsPseudoClassList* pseudoClass = aSelector->mPseudoClassList; + pseudoClass; + pseudoClass = pseudoClass->mNext) { + if (pseudoClass->mType == CSSPseudoClassType::host || + pseudoClass->mType == CSSPseudoClassType::hostContext) { + if (possibleChange) { + // :host-context will walk ancestors looking for a match of a + // compound selector, thus any changes to ancestors may require + // restyling the subtree. + *possibleChange |= eRestyle_Subtree; + } + return true; + } + } + return false; + } + else if (aMatchContext->mRestrictToSlottedPseudo) { + for (nsCSSSelector* selector = aSelector; + selector; + selector = selector->mNext) { + if (!selector->mPseudoClassList) { + continue; + } + for (nsPseudoClassList* pseudoClass = selector->mPseudoClassList; + pseudoClass; + pseudoClass = pseudoClass->mNext) { + if (pseudoClass->mType == CSSPseudoClassType::slotted) { + return true; + } + } + } + return false; + } + // We're not restricted to a specific pseudo-class. + return true; +} + static inline void ContentEnumFunc(const RuleValue& value, nsCSSSelector* aSelector, ElementDependentRuleProcessorData* data, NodeMatchContext& nodeContext, @@ -2778,29 +2830,11 @@ void ContentEnumFunc(const RuleValue& value, nsCSSSelector* aSelector, // We won't match; nothing else to do here return; } - // If mOnlyMatchHostPseudo is set, then we only want to match against - // selectors that contain a :host-context pseudo class. - if (data->mTreeMatchContext.mOnlyMatchHostPseudo) { - nsCSSSelector* selector = aSelector; - while (selector && selector->mNext != nullptr) { - selector = selector->mNext; - } - bool seenHostPseudo = false; - for (nsPseudoClassList* pseudoClass = selector->mPseudoClassList; - pseudoClass; - pseudoClass = pseudoClass->mNext) { - if (pseudoClass->mType == CSSPseudoClassType::host || - pseudoClass->mType == CSSPseudoClassType::hostContext) { - seenHostPseudo = true; - break; - } - } - - if (!seenHostPseudo) { - return; - } + if (!LookForTargetPseudo(aSelector, &data->mTreeMatchContext, nullptr)) { + return; } + if (!data->mTreeMatchContext.SetStyleScopeForSelectorMatching(data->mElement, data->mScope)) { // The selector is for a rule in a scoped style sheet, and the subject @@ -3145,35 +3179,11 @@ AttributeEnumFunc(nsCSSSelector* aSelector, nsRestyleHint possibleChange = RestyleHintForSelectorWithAttributeChange(aData->change, aSelector, aRightmostSelector); - // If mOnlyMatchHostPseudo is set, then we only want to match against - // selectors that contain a :host-context pseudo class. - if (data->mTreeMatchContext.mOnlyMatchHostPseudo) { - nsCSSSelector* selector = aSelector; - while (selector && selector->mNext != nullptr) { - selector = selector->mNext; - } - bool seenHostPseudo = false; - for (nsPseudoClassList* pseudoClass = selector->mPseudoClassList; - pseudoClass; - pseudoClass = pseudoClass->mNext) { - if (pseudoClass->mType == CSSPseudoClassType::host || - pseudoClass->mType == CSSPseudoClassType::hostContext) { - // :host-context will walk ancestors looking for a match of a compound - // selector, thus any changes to ancestors may require restyling the - // subtree. - possibleChange |= eRestyle_Subtree; - seenHostPseudo = true; - break; - } - } - - if (!seenHostPseudo) { - return; - } + if (!LookForTargetPseudo(aSelector, &data->mTreeMatchContext, &possibleChange)) { + return; } - // If, ignoring eRestyle_SomeDescendants, enumData->change already includes // all the bits of possibleChange, don't bother calling SelectorMatches, since // even if it returns false enumData->change won't change. If possibleChange diff --git a/layout/style/nsRuleProcessorData.h b/layout/style/nsRuleProcessorData.h index 28abf204f9..49fb341b76 100644 --- a/layout/style/nsRuleProcessorData.h +++ b/layout/style/nsRuleProcessorData.h @@ -371,6 +371,9 @@ struct MOZ_STACK_CLASS TreeMatchContext { // match. bool mOnlyMatchHostPseudo; + // Restrict matching to selectors that contain a :slotted() pseudo-class. + bool mRestrictToSlottedPseudo; + // Root of scoped stylesheet (set and unset by the supplier of the // scoped stylesheet). nsIContent* mScopedRoot; @@ -403,6 +406,9 @@ struct MOZ_STACK_CLASS TreeMatchContext { // for an HTML5 scoped style sheet. bool mForScopedStyle; + // Whether we're currently in the topmost scope for shadow DOM. + bool mIsTopmostScope; + enum MatchVisited { eNeverMatchVisited, eMatchVisitedDefault @@ -429,6 +435,7 @@ struct MOZ_STACK_CLASS TreeMatchContext { , mVisitedHandling(aVisitedHandling) , mDocument(aDocument) , mOnlyMatchHostPseudo(false) + , mRestrictToSlottedPseudo(false) , mScopedRoot(nullptr) , mIsHTMLDocument(aDocument->IsHTMLDocument()) , mCompatMode(aDocument->GetCompatibilityMode()) @@ -436,6 +443,7 @@ struct MOZ_STACK_CLASS TreeMatchContext { , mSkippingParentDisplayBasedStyleFixup(false) , mForScopedStyle(false) , mCurrentStyleScope(nullptr) + , mIsTopmostScope(false) { if (aMatchVisited != eNeverMatchVisited) { nsILoadContext* loadContext = mDocument->GetLoadContext(); From 8d2533ad70566ff4fd69d4d446b5e28e28f7bb2f Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Sat, 18 Mar 2023 23:08:00 +0800 Subject: [PATCH 07/14] Issue #1592 - Part 5: Use flattened element tree when looking for a parent while matching ::slotted() --- layout/style/nsCSSRuleProcessor.cpp | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/layout/style/nsCSSRuleProcessor.cpp b/layout/style/nsCSSRuleProcessor.cpp index 63c228fda2..5f1eae5481 100644 --- a/layout/style/nsCSSRuleProcessor.cpp +++ b/layout/style/nsCSSRuleProcessor.cpp @@ -2566,6 +2566,11 @@ SelectorMatchesTree(Element* aPrevElement, // The relevant link must be an ancestor of the node being matched. aFlags = SelectorMatchesTreeFlags(aFlags & ~eLookForRelevantLink); nsIContent* parent = prevElement->GetParent(); + // Operate on the flattened element tree when matching the + // ::slotted() pseudo-element. + if (aTreeMatchContext.mRestrictToSlottedPseudo) { + parent = prevElement->GetFlattenedTreeParent(); + } if (parent) { if (aTreeMatchContext.mForStyling) parent->SetFlags(NODE_HAS_SLOW_SELECTOR_LATER_SIBLINGS); @@ -2576,7 +2581,12 @@ SelectorMatchesTree(Element* aPrevElement, // for descendant combinators and child combinators, the element // to test against is the parent else { - nsIContent *content = prevElement->GetParent(); + nsIContent* content = prevElement->GetParent(); + // Operate on the flattened element tree when matching the + // ::slotted() pseudo-element. + if (aTreeMatchContext.mRestrictToSlottedPseudo) { + content = prevElement->GetFlattenedTreeParent(); + } // In the shadow tree, the shadow host behaves as if it // is a featureless parent of top-level elements of the shadow @@ -2584,11 +2594,12 @@ SelectorMatchesTree(Element* aPrevElement, // left most selector because ancestors of the host are not in // the selector match list. ShadowRoot* shadowRoot = content ? - ShadowRoot::FromNode(content) : nullptr; + ShadowRoot::FromNode(content) : + nullptr; if (shadowRoot && !selector->mNext && !crossedShadowRootBoundary) { - content = shadowRoot->GetHost(); - crossedShadowRootBoundary = true; - contentIsFeatureless = true; + content = shadowRoot->GetHost(); + crossedShadowRootBoundary = true; + contentIsFeatureless = true; } // GetParent could return a document fragment; we only want From 91d2b6f4cf0024913945ccc443002134ccee9f8a Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Sun, 19 Mar 2023 18:48:24 +0800 Subject: [PATCH 08/14] Issue #1592 - Part 6: Allow pseudo-classes with a forgiving selector list argument to follow pseudo-elements Pseudo-classes with a forgiving selector list argument are allowed to follow a pseudo-element, but must treat any selector that is not of the same type as invalid. It doesn't make any sense, but that's the behavior of other tainted browsers. --- layout/style/nsCSSParser.cpp | 62 ++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp index 39d81601c7..92cc84a7c1 100644 --- a/layout/style/nsCSSParser.cpp +++ b/layout/style/nsCSSParser.cpp @@ -119,7 +119,8 @@ enum class SelectorParsingFlags { eIsForgiving = 1 << 1, eDisallowCombinators = 1 << 2, eDisallowPseudoElements = 1 << 3, - eInheritNamespace = 1 << 4 + eInheritNamespace = 1 << 4, + eForceEmptyList = 1 << 5 }; MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(SelectorParsingFlags) @@ -6103,6 +6104,8 @@ CSSParserImpl::ParsePseudoSelector(int32_t& aDataMask, nsCSSPseudoClasses::GetPseudoType(pseudo, enabledState); bool pseudoClassIsUserAction = nsCSSPseudoClasses::IsUserActionPseudoClass(pseudoClassType); + bool pseudoClassHasForgivingSelectorListArg = + nsCSSPseudoClasses::HasForgivingSelectorListArg(pseudoClassType); if (nsCSSAnonBoxes::IsNonElement(pseudo)) { // Non-element anonymous boxes should not match any rule. @@ -6199,6 +6202,7 @@ CSSParserImpl::ParsePseudoSelector(int32_t& aDataMask, return eSelectorParsingStatus_Error; } + bool forceEmptyList = false; if (aSelector.IsPseudoElement() || aSelector.IsHybridPseudoElement()) { CSSPseudoElementType type = aSelector.IsPseudoElement() ? aSelector.PseudoType() : @@ -6216,13 +6220,20 @@ CSSParserImpl::ParsePseudoSelector(int32_t& aDataMask, return eSelectorParsingStatus_Error; } - 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; + if (isPseudoClass) { + if (pseudoClassHasForgivingSelectorListArg) { + // XXX: Pseudo-classes with a forgiving selector list argument are + // allowed to follow a pseudo-element, but must treat any selector + // that is not of the same type as invalid. It doesn't make any + // sense, but that's the behavior of other tainted browsers. + forceEmptyList = true; + } else if (!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); @@ -6235,13 +6246,30 @@ CSSParserImpl::ParsePseudoSelector(int32_t& aDataMask, !!(aFlags & SelectorParsingFlags::eDisallowPseudoElements); if (!parsingPseudoElement && isPseudoClass) { aDataMask |= SEL_MASK_PCLASS; + + // Only pseudo-classes with a forgiving selector list argument + // are allowed if we're forced to be empty. + if ((aFlags & SelectorParsingFlags::eForceEmptyList) && + !pseudoClassHasForgivingSelectorListArg) { + if (eCSSToken_Function == mToken.mType) { + SkipUntil(')'); + } + return eSelectorParsingStatus_Continue; + } + if (eCSSToken_Function == mToken.mType) { nsSelectorParsingStatus parsingStatus; - // Only the combinators restriction should be passed down the chain. - SelectorParsingFlags flags = - (aFlags & SelectorParsingFlags::eDisallowCombinators) ? - SelectorParsingFlags::eDisallowCombinators : - SelectorParsingFlags::eNone; + + // Pass only a few parsing flags down the chain. + SelectorParsingFlags flags = SelectorParsingFlags::eNone; + if (aFlags & SelectorParsingFlags::eDisallowCombinators) { + flags |= SelectorParsingFlags::eDisallowCombinators; + } + if (aFlags & SelectorParsingFlags::eForceEmptyList || + forceEmptyList) { + flags |= SelectorParsingFlags::eForceEmptyList; + } + if (sLegacyNegationPseudoClassEnabled && CSSPseudoClassType::negation == pseudoClassType) { // :not() can't be itself negated @@ -6806,6 +6834,14 @@ CSSParserImpl::ParseSelector(nsCSSSelectorList* aList, } } + // Treat every other selector as invalid. + if ((aFlags & SelectorParsingFlags::eForceEmptyList) && + (selector->mIDList || selector->mClassList || + selector->mAttrList || selector->mNegations || + !selector->mPseudoClassList)) { + return false; + } + if (parsingStatus == eSelectorParsingStatus_Error) { return false; } From 19226fd560216e73ddc344bc69f4a46dea5d8439 Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Sun, 19 Mar 2023 19:10:46 +0800 Subject: [PATCH 09/14] Issue #1592 - Part 7: Slottables cannot be matched from the outer tree. --- layout/style/nsCSSRuleProcessor.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/layout/style/nsCSSRuleProcessor.cpp b/layout/style/nsCSSRuleProcessor.cpp index 5f1eae5481..d24ab209ab 100644 --- a/layout/style/nsCSSRuleProcessor.cpp +++ b/layout/style/nsCSSRuleProcessor.cpp @@ -1380,8 +1380,9 @@ enum class SelectorMatchesFlags : uint8_t { // pseudo-element. IS_PSEUDO_CLASS_ARGUMENT = 1 << 2, - // The selector should be blocked from matching the :host pseudo-class. - IS_HOST_INACCESSIBLE = 1 << 3 + // The selector should be blocked from matching because it is called + // from outside the shadow tree. + IS_OUTSIDE_SHADOW_TREE = 1 << 3 }; MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(SelectorMatchesFlags) @@ -1400,7 +1401,7 @@ static inline bool ActiveHoverQuirkMatches(nsCSSSelector* aSelector, aSelectorFlags & (SelectorMatchesFlags::UNKNOWN | SelectorMatchesFlags::HAS_PSEUDO_ELEMENT | SelectorMatchesFlags::IS_PSEUDO_CLASS_ARGUMENT | - SelectorMatchesFlags::IS_HOST_INACCESSIBLE)) { + SelectorMatchesFlags::IS_OUTSIDE_SHADOW_TREE)) { return false; } @@ -1798,6 +1799,8 @@ static bool SelectorMatches(Element* aElement, } } + const bool isOutsideShadowTree = + !!(aSelectorFlags & SelectorMatchesFlags::IS_OUTSIDE_SHADOW_TREE); const bool isNegated = (aDependence != nullptr); // The selectors for which we set node bits are, unfortunately, early // in this function (because they're pseudo-classes, which are @@ -1966,6 +1969,11 @@ static bool SelectorMatches(Element* aElement, case CSSPseudoClassType::slotted: { + // Slottables cannot be matched from the outer tree. + if (isOutsideShadowTree) { + return false; + } + // Slot elements cannot be matched. if (aElement->IsHTMLElement(nsGkAtoms::slot)) { return false; @@ -1997,7 +2005,7 @@ static bool SelectorMatches(Element* aElement, // style). if (!shadow || aSelector->HasFeatureSelectors() || - aSelectorFlags & SelectorMatchesFlags::IS_HOST_INACCESSIBLE) { + isOutsideShadowTree) { return false; } @@ -4232,7 +4240,7 @@ nsCSSRuleProcessor::RestrictedSelectorListMatches(Element* aElement, NodeMatchContext nodeContext(EventStates(), false); SelectorMatchesFlags flags = aElement->IsInShadowTree() ? SelectorMatchesFlags::NONE : - SelectorMatchesFlags::IS_HOST_INACCESSIBLE; + SelectorMatchesFlags::IS_OUTSIDE_SHADOW_TREE; return SelectorListMatches(aElement, aSelectorList, nodeContext, From 4cd0de04d0d92a3724fb3e9360a19dd9a30972ec Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Mon, 20 Mar 2023 01:41:56 +0800 Subject: [PATCH 10/14] Issue #1592 - Part 8: Test the assigned slot for type/class/ID/attribute instead of the slottable when matching ::slotted() --- dom/xbl/nsBindingManager.cpp | 2 ++ layout/style/nsCSSRuleProcessor.cpp | 51 ++++++++++++++++++++--------- layout/style/nsRuleProcessorData.h | 5 +++ 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/dom/xbl/nsBindingManager.cpp b/dom/xbl/nsBindingManager.cpp index c6dc58ec89..b50c86c002 100644 --- a/dom/xbl/nsBindingManager.cpp +++ b/dom/xbl/nsBindingManager.cpp @@ -713,9 +713,11 @@ nsBindingManager::WalkRules(nsIStyleRuleProcessor::EnumFunc aFunc, nsXBLBinding* binding = stack.ElementAt(index); stack.RemoveElementAt(index); + aData->mTreeMatchContext.mForAssignedSlot = true; aData->mTreeMatchContext.mIsTopmostScope = (index == 0); binding->WalkRules(aFunc, aData); } + aData->mTreeMatchContext.mForAssignedSlot = false; aData->mTreeMatchContext.mRestrictToSlottedPseudo = false; } diff --git a/layout/style/nsCSSRuleProcessor.cpp b/layout/style/nsCSSRuleProcessor.cpp index d24ab209ab..55b7293f6a 100644 --- a/layout/style/nsCSSRuleProcessor.cpp +++ b/layout/style/nsCSSRuleProcessor.cpp @@ -48,6 +48,7 @@ #include "nsCSSRules.h" #include "nsStyleSet.h" #include "mozilla/dom/Element.h" +#include "mozilla/dom/HTMLSlotElement.h" #include "mozilla/dom/ShadowRoot.h" #include "nsNthIndexCache.h" #include "mozilla/ArrayUtils.h" @@ -684,6 +685,7 @@ void RuleHash::EnumerateAllRules(Element* aElement, ElementDependentRuleProcesso filter->AssertHasAllAncestors(aElement); } #endif + bool isForAssignedSlot = aData->mTreeMatchContext.mForAssignedSlot; // Merge the lists while there are still multiple lists to merge. while (valueCount > 1) { int32_t valueIndex = 0; @@ -696,6 +698,7 @@ void RuleHash::EnumerateAllRules(Element* aElement, ElementDependentRuleProcesso } } const RuleValue *cur = mEnumList[valueIndex].mCurValue; + aData->mTreeMatchContext.mForAssignedSlot = isForAssignedSlot; ContentEnumFunc(*cur, cur->mSelector, aData, aNodeContext, filter); cur++; if (cur == mEnumList[valueIndex].mEnd) { @@ -709,6 +712,7 @@ void RuleHash::EnumerateAllRules(Element* aElement, ElementDependentRuleProcesso for (const RuleValue *value = mEnumList[0].mCurValue, *end = mEnumList[0].mEnd; value != end; ++value) { + aData->mTreeMatchContext.mForAssignedSlot = isForAssignedSlot; ContentEnumFunc(*value, value->mSelector, aData, aNodeContext, filter); } } @@ -1727,24 +1731,29 @@ static bool SelectorMatches(Element* aElement, return false; } + Element* targetElement = aElement; + if (aTreeMatchContext.mForAssignedSlot) { + targetElement = aElement->GetAssignedSlot()->AsElement(); + } + // namespace/tag match // optimization : bail out early if we can if ((kNameSpaceID_Unknown != aSelector->mNameSpace && - aElement->GetNameSpaceID() != aSelector->mNameSpace)) + targetElement->GetNameSpaceID() != aSelector->mNameSpace)) return false; if (aSelector->mLowercaseTag) { nsIAtom* selectorTag = - (aTreeMatchContext.mIsHTMLDocument && aElement->IsHTMLElement()) ? + (aTreeMatchContext.mIsHTMLDocument && targetElement->IsHTMLElement()) ? aSelector->mLowercaseTag : aSelector->mCasedTag; - if (selectorTag != aElement->NodeInfo()->NameAtom()) { + if (selectorTag != targetElement->NodeInfo()->NameAtom()) { return false; } } nsAtomList* IDList = aSelector->mIDList; if (IDList) { - nsIAtom* id = aElement->GetID(); + nsIAtom* id = targetElement->GetID(); if (id) { // case sensitivity: bug 93371 const bool isCaseSensitive = @@ -1779,7 +1788,7 @@ static bool SelectorMatches(Element* aElement, nsAtomList* classList = aSelector->mClassList; if (classList) { // test for class match - const nsAttrValue *elementClasses = aElement->GetClasses(); + const nsAttrValue *elementClasses = targetElement->GetClasses(); if (!elementClasses) { // Element has no classes but we have a class selector return false; @@ -1969,6 +1978,10 @@ static bool SelectorMatches(Element* aElement, case CSSPseudoClassType::slotted: { + if (aTreeMatchContext.mForAssignedSlot) { + aTreeMatchContext.mForAssignedSlot = false; + } + // Slottables cannot be matched from the outer tree. if (isOutsideShadowTree) { return false; @@ -2370,7 +2383,7 @@ static bool SelectorMatches(Element* aElement, bool result = true; if (aSelector->mAttrList) { // test for attribute match - if (!aElement->HasAttrs()) { + if (!targetElement->HasAttrs()) { // if no attributes on the content, no match return false; } else { @@ -2380,7 +2393,7 @@ static bool SelectorMatches(Element* aElement, do { bool isHTML = - (aTreeMatchContext.mIsHTMLDocument && aElement->IsHTMLElement()); + (aTreeMatchContext.mIsHTMLDocument && targetElement->IsHTMLElement()); matchAttribute = isHTML ? attr->mLowercaseAttr : attr->mCasedAttr; if (attr->mNameSpace == kNameSpaceID_Unknown) { // Attr selector with a wildcard namespace. We have to examine all @@ -2392,7 +2405,7 @@ static bool SelectorMatches(Element* aElement, // actually has attributes in), short-circuiting if we ever match. result = false; const nsAttrName* attrName; - for (uint32_t i = 0; (attrName = aElement->GetAttrNameAt(i)); ++i) { + for (uint32_t i = 0; (attrName = targetElement->GetAttrNameAt(i)); ++i) { if (attrName->LocalName() != matchAttribute) { continue; } @@ -2403,7 +2416,7 @@ static bool SelectorMatches(Element* aElement, #ifdef DEBUG bool hasAttr = #endif - aElement->GetAttr(attrName->NamespaceID(), + targetElement->GetAttr(attrName->NamespaceID(), attrName->LocalName(), value); NS_ASSERTION(hasAttr, "GetAttrNameAt lied"); result = AttrMatchesValue(attr, value, isHTML); @@ -2421,12 +2434,12 @@ static bool SelectorMatches(Element* aElement, } else if (attr->mFunction == NS_ATTR_FUNC_EQUALS) { result = - aElement-> + targetElement-> AttrValueIs(attr->mNameSpace, matchAttribute, attr->mValue, attr->IsValueCaseSensitive(isHTML) ? eCaseMatters : eIgnoreCase); } - else if (!aElement->HasAttr(attr->mNameSpace, matchAttribute)) { + else if (!targetElement->HasAttr(attr->mNameSpace, matchAttribute)) { result = false; } else if (attr->mFunction != NS_ATTR_FUNC_SET) { @@ -2434,7 +2447,7 @@ static bool SelectorMatches(Element* aElement, #ifdef DEBUG bool hasAttr = #endif - aElement->GetAttr(attr->mNameSpace, matchAttribute, value); + targetElement->GetAttr(attr->mNameSpace, matchAttribute, value); NS_ASSERTION(hasAttr, "HasAttr lied"); result = AttrMatchesValue(attr, value, isHTML); } @@ -2449,7 +2462,7 @@ static bool SelectorMatches(Element* aElement, for (nsCSSSelector *negation = aSelector->mNegations; result && negation; negation = negation->mNegations) { bool dependence = false; - result = !SelectorMatches(aElement, negation, aNodeMatchContext, + result = !SelectorMatches(targetElement, negation, aNodeMatchContext, aTreeMatchContext, SelectorMatchesFlags::IS_PSEUDO_CLASS_ARGUMENT, &dependence); @@ -2843,7 +2856,9 @@ void ContentEnumFunc(const RuleValue& value, nsCSSSelector* aSelector, if (nodeContext.mIsRelevantLink) { data->mTreeMatchContext.SetHaveRelevantLink(); } - if (ancestorFilter && + // XXX: Ignore the ancestor filter if we're testing the assigned slot. + bool useAncestorFilter = !(data->mTreeMatchContext.mForAssignedSlot); + if (useAncestorFilter && ancestorFilter && !ancestorFilter->MightHaveMatchingAncestor( value.mAncestorSelectorHashes)) { // We won't match; nothing else to do here @@ -2915,7 +2930,13 @@ nsCSSRuleProcessor::RulesMatching(ElementRuleProcessorData *aData) NodeMatchContext nodeContext(EventStates(), nsCSSRuleProcessor::IsLink(aData->mElement), aData->mElementIsFeatureless); - cascade->mRuleHash.EnumerateAllRules(aData->mElement, aData, nodeContext); + // Test against the assigned slot rather than the slottable if we're + // matching the ::slotted() pseudo. + Element* targetElement = aData->mElement; + if (aData->mTreeMatchContext.mForAssignedSlot) { + targetElement = aData->mElement->GetAssignedSlot()->AsElement(); + } + cascade->mRuleHash.EnumerateAllRules(targetElement, aData, nodeContext); } } diff --git a/layout/style/nsRuleProcessorData.h b/layout/style/nsRuleProcessorData.h index 49fb341b76..05b39f0e71 100644 --- a/layout/style/nsRuleProcessorData.h +++ b/layout/style/nsRuleProcessorData.h @@ -409,6 +409,10 @@ struct MOZ_STACK_CLASS TreeMatchContext { // Whether we're currently in the topmost scope for shadow DOM. bool mIsTopmostScope; + // Whether we're testing for the assigned slot instead of the slottable + // when matching type/class/ID/attribute. + bool mForAssignedSlot; + enum MatchVisited { eNeverMatchVisited, eMatchVisitedDefault @@ -444,6 +448,7 @@ struct MOZ_STACK_CLASS TreeMatchContext { , mForScopedStyle(false) , mCurrentStyleScope(nullptr) , mIsTopmostScope(false) + , mForAssignedSlot(false) { if (aMatchVisited != eNeverMatchVisited) { nsILoadContext* loadContext = mDocument->GetLoadContext(); From 9a071f3b7af7461168e0b80cdea8f4ff7f64780b Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Sun, 19 Mar 2023 23:43:34 +0800 Subject: [PATCH 11/14] Issue #1592 - Part 9: Post a restyle event after changing the slot of a slottable --- dom/base/ShadowRoot.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/dom/base/ShadowRoot.cpp b/dom/base/ShadowRoot.cpp index d432c96e41..8482da5e99 100644 --- a/dom/base/ShadowRoot.cpp +++ b/dom/base/ShadowRoot.cpp @@ -9,6 +9,7 @@ #include "mozilla/dom/DocumentFragment.h" #include "ChildIterator.h" #include "nsContentUtils.h" +#include "nsLayoutUtils.h" #include "nsDOMClassInfoID.h" #include "nsIDOMHTMLElement.h" #include "nsIStyleSheetLinkingElement.h" @@ -162,6 +163,16 @@ ShadowRoot::AddSlot(HTMLSlotElement* aSlot) oldSlot->RemoveAssignedNode(assignedNode); currentSlot->AppendAssignedNode(assignedNode); + Element* restyleElement; + if (assignedNode->IsElement()) { + restyleElement = assignedNode->AsElement(); + } else { + // This is likely a text node. Use the host instead. + restyleElement = GetHost(); + } + nsLayoutUtils::PostRestyleEvent( + restyleElement, eRestyle_Subtree, nsChangeHint(0)); + doEnqueueSlotChange = true; } From bc12e05bd3a13d2db5614dc9675d02cfaf33ef6c Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Mon, 20 Mar 2023 03:00:11 +0800 Subject: [PATCH 12/14] Issue #1592 - Part 10: Slot elements should restyle their parent on attribute changes --- dom/html/HTMLSlotElement.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dom/html/HTMLSlotElement.cpp b/dom/html/HTMLSlotElement.cpp index 4d4b137dd5..9286ff9591 100644 --- a/dom/html/HTMLSlotElement.cpp +++ b/dom/html/HTMLSlotElement.cpp @@ -110,6 +110,13 @@ HTMLSlotElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } } + if (nsIContent* parent = GetParent()) { + if (parent->IsElement()) { + nsLayoutUtils::PostRestyleEvent( + parent->AsElement(), eRestyle_Subtree, nsChangeHint(0)); + } + } + return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue, aOldValue, aNotify); } From b29522749a9ba7245485766d0101c700847a11b6 Mon Sep 17 00:00:00 2001 From: FranklinDM Date: Wed, 22 Mar 2023 17:29:00 +0800 Subject: [PATCH 13/14] Issue #1592 - Follow-up: Don't post a restyle event if restyleElement is null This fixes a potential crash caused if restyleElement is null. --- dom/base/ShadowRoot.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dom/base/ShadowRoot.cpp b/dom/base/ShadowRoot.cpp index 8482da5e99..abf6301123 100644 --- a/dom/base/ShadowRoot.cpp +++ b/dom/base/ShadowRoot.cpp @@ -163,6 +163,7 @@ ShadowRoot::AddSlot(HTMLSlotElement* aSlot) oldSlot->RemoveAssignedNode(assignedNode); currentSlot->AppendAssignedNode(assignedNode); + Element* restyleElement; if (assignedNode->IsElement()) { restyleElement = assignedNode->AsElement(); @@ -170,8 +171,10 @@ ShadowRoot::AddSlot(HTMLSlotElement* aSlot) // This is likely a text node. Use the host instead. restyleElement = GetHost(); } - nsLayoutUtils::PostRestyleEvent( - restyleElement, eRestyle_Subtree, nsChangeHint(0)); + if (restyleElement) { + nsLayoutUtils::PostRestyleEvent( + restyleElement, eRestyle_Subtree, nsChangeHint(0)); + } doEnqueueSlotChange = true; } From 078b1b73dc77baa2d48e9fa2421fec97485a81d0 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Thu, 23 Mar 2023 22:35:03 +0100 Subject: [PATCH 14/14] Issue #2161 - Ctrl + Enter should cause keypress event even though the key combination doesn't input any character Currently, we dispatch keypress event when Enter is pressed without modifiers or only with the Shift key (line break). However, other browsers dispatch keypress events for Ctrl + Enter also even if it doesn't cause any text input. So, we should fire keypress events for Ctrl + Enter, even in strict keypress dispatching mode. Note that with other modifiers, it depends on the browser and/or platform and we can't dispatch the event for consistent behavior. This means web developers shouldn't rely one keypress events to catch Alt + Enter, Meta + Enter and two or more modifiers + Enter. Based on BZ 1438133 Resolves #2161 --- dom/events/test/test_dom_keyboard_event.html | 156 +++++++++++++++++++ widget/TextEventDispatcher.cpp | 3 +- widget/TextEvents.h | 30 ++++ 3 files changed, 187 insertions(+), 2 deletions(-) diff --git a/dom/events/test/test_dom_keyboard_event.html b/dom/events/test/test_dom_keyboard_event.html index e850659042..9fc858ccf9 100644 --- a/dom/events/test/test_dom_keyboard_event.html +++ b/dom/events/test/test_dom_keyboard_event.html @@ -8,6 +8,9 @@

+

+

+

@@ -293,10 +296,163 @@ function testSynthesizedKeyLocation() window.removeEventListener("keyup", handler, true); } +// We're using TextEventDispatcher to decide if we should keypress event +// on content in the default event group. So, we can test if keypress +// event is NOT fired unexpectedly with synthesizeKey(). +function testEnterKeyPressEvent() +{ + let keydownFired, keypressFired, beforeinputFired; + function onEvent(aEvent) { + switch (aEvent.type) { + case "keydown": + keydownFired = true; + return; + case "keypress": + keypressFired = true; + return; + case "beforeinput": + beforeinputFired = true; + return; + } + } + + for (let targetId of ["input", "textarea", "input_readonly"]) { + let target = document.getElementById(targetId); + + function reset() { + keydownFired = keypressFired = beforeinputFired = false; + target.value = ""; + } + + target.addEventListener("keydown", onEvent); + target.addEventListener("keypress", onEvent); + target.addEventListener("beforeinput", onEvent); + + const kDescription = "<" + targetId.replace("_", " ") + ">: "; + let isEditable = kDescription.includes("readonly"); + let isTextarea = kDescription.includes("textarea"); + + target.focus(); + + reset(); + synthesizeKey("KEY_Enter", {}); + is(keydownFired, true, + kDescription + "keydown event should be fired when Enter key is pressed"); + is(keypressFired, true, + kDescription + "keypress event should be fired when Enter key is pressed"); + if (isEditable) { + todo_is(beforeinputFired, true, + kDescription + "beforeinput event should be fired when Enter key is pressed"); + } else { + is(beforeinputFired, false, + kDescription + "beforeinput event shouldn't be fired when Enter key is pressed"); + } + if (isTextarea) { + is(target.value, "\n", + kDescription + "Enter key should cause inputting a line break in