Skip to content

Commit

Permalink
[@scope] Generalize kPseudoParentUnparsed to re-use it for :scope
Browse files Browse the repository at this point in the history
We now look for both '&' and ':scope' when handling otherwise-discarded
parts of forgiving selector lists, and store the appropriate
CSSNestingType in the selector's RareData.

Bug: 1280240
Change-Id: I57e6f91393400e797acbd4fab263d57d08db5115
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4281581
Reviewed-by: Steinar H Gunderson <sesse@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1115563}
  • Loading branch information
andruud authored and Chromium LUCI CQ committed Mar 10, 2023
1 parent f345363 commit 9bf895e
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 32 deletions.
16 changes: 10 additions & 6 deletions third_party/blink/renderer/core/css/css_selector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,6 @@ PseudoId CSSSelector::GetPseudoId(PseudoType type) {
case kPseudoOptional:
case kPseudoOutOfRange:
case kPseudoParent:
case kPseudoParentUnparsed:
case kPseudoPart:
case kPseudoPastCue:
case kPseudoPaused:
Expand All @@ -379,6 +378,7 @@ PseudoId CSSSelector::GetPseudoId(PseudoType type) {
case kPseudoTarget:
case kPseudoToggle:
case kPseudoUnknown:
case kPseudoUnparsed:
case kPseudoValid:
case kPseudoVertical:
case kPseudoVideoPersistent:
Expand Down Expand Up @@ -820,7 +820,6 @@ void CSSSelector::UpdatePseudoType(const AtomicString& value,
case kPseudoOptional:
case kPseudoOutOfRange:
case kPseudoParent:
case kPseudoParentUnparsed:
case kPseudoPastCue:
case kPseudoPaused:
case kPseudoPictureInPicture:
Expand All @@ -839,6 +838,7 @@ void CSSSelector::UpdatePseudoType(const AtomicString& value,
case kPseudoTarget:
case kPseudoToggle:
case kPseudoUnknown:
case kPseudoUnparsed:
case kPseudoValid:
case kPseudoVertical:
case kPseudoVisited:
Expand All @@ -859,17 +859,21 @@ void CSSSelector::UpdatePseudoType(const AtomicString& value,
}
}

void CSSSelector::SetUnparsedPlaceholder(const AtomicString& value) {
void CSSSelector::SetUnparsedPlaceholder(CSSNestingType unparsed_nesting_type,
const AtomicString& value) {
DCHECK(match_ == kPseudoClass);
SetPseudoType(kPseudoParentUnparsed);
SetPseudoType(kPseudoUnparsed);
CreateRareData();
SetValue(value);
data_.rare_data_->bits_.unparsed_nesting_type_ = unparsed_nesting_type;
}

CSSNestingType CSSSelector::GetNestingType() const {
switch (GetPseudoType()) {
case CSSSelector::kPseudoParent:
case CSSSelector::kPseudoParentUnparsed:
return CSSNestingType::kNesting;
case CSSSelector::kPseudoUnparsed:
return data_.rare_data_->bits_.unparsed_nesting_type_;
case CSSSelector::kPseudoScope:
// TODO(crbug.com/1280240): Handle unparsed :scope.
return CSSNestingType::kScope;
Expand Down Expand Up @@ -920,7 +924,7 @@ bool CSSSelector::SerializeSimpleSelector(StringBuilder& builder) const {
builder.Append('.');
SerializeIdentifier(SerializingValue(), builder);
} else if (match_ == kPseudoClass || match_ == kPagePseudoClass) {
if (GetPseudoType() == kPseudoParentUnparsed) {
if (GetPseudoType() == kPseudoUnparsed) {
builder.Append(Value());
} else if (GetPseudoType() != kPseudoState &&
GetPseudoType() != kPseudoParent) {
Expand Down
17 changes: 13 additions & 4 deletions third_party/blink/renderer/core/css/css_selector.h
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,6 @@ class CORE_EXPORT CSSSelector {
kPseudoOnlyOfType,
kPseudoOptional,
kPseudoParent, // Written as & (in nested rules).
// Something that was unparsable, but contained a & and thus must be kept
// for serialization purposes.
kPseudoParentUnparsed,
kPseudoPart,
kPseudoPlaceholder,
kPseudoPlaceholderShown,
Expand All @@ -272,6 +269,10 @@ class CORE_EXPORT CSSSelector {
kPseudoState,
kPseudoTarget,
kPseudoUnknown,
// Something that was unparsable, but contained either a nesting
// selector (&), or a :scope pseudo-class, and must therefore be kept
// for serialization purposes.
kPseudoUnparsed,
kPseudoValid,
kPseudoVertical,
kPseudoVisited,
Expand Down Expand Up @@ -348,10 +349,15 @@ class CORE_EXPORT CSSSelector {
const CSSParserContext&,
bool has_arguments,
CSSParserMode);
void SetUnparsedPlaceholder(const AtomicString&);
void SetUnparsedPlaceholder(CSSNestingType, const AtomicString&);
// If this simple selector contains a parent selector (&), returns kNesting.
// Otherwise, if this simple selector contains a :scope pseudo-class,
// returns kScope. Otherwise, returns kNone.
//
// Note that this means that a selector which contains both '&' and :scope
// (which can happen for kPseudoUnparsed) will return kNesting. This is OK,
// since any selector which is nest-containing is also treated as
// scope-containing during parsing.
CSSNestingType GetNestingType() const;
void UpdatePseudoPage(const AtomicString&, const Document*);
static PseudoType NameToPseudoType(const AtomicString&,
Expand Down Expand Up @@ -578,6 +584,9 @@ class CORE_EXPORT CSSSelector {
// containing complex selector in its argument. e.g. :has(:is(.a .b))
bool contains_complex_logical_combinations_;
} has_;

// See GetNestingType.
CSSNestingType unparsed_nesting_type_;
} bits_;
QualifiedName attribute_; // Used for attribute selector
AtomicString argument_; // Used for :contains, :lang, :dir, :toggle, etc.
Expand Down
46 changes: 31 additions & 15 deletions third_party/blink/renderer/core/css/parser/css_selector_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ CSSSelectorParser::ConsumeForgivingComplexSelectorList(
}
output_.resize(subpos); // Drop what we parsed so far.
valid_and_invalid_counter.CountInvalid();
AddPlaceholderParentSelectorIfNeeded(argument);
AddPlaceholderSelectorIfNeeded(argument);
} else {
valid_and_invalid_counter.CountValid();
}
Expand All @@ -452,24 +452,40 @@ CSSSelectorParser::ConsumeForgivingComplexSelectorList(
return reset_vector.CommitAddedElements();
}

// If the argument was unparsable but contained a & token,
// we need to keep it so that we still consider the :is()
// as nest-containing; furthermore, we need to keep it
// on serialization so that round-tripping does not risk
// making the :is() no longer nest-containing. We have similar
// weaknesses here as in CSS custom properties, such as not
// preserving comments fully.
void CSSSelectorParser::AddPlaceholderParentSelectorIfNeeded(
// If the argument was unparsable but contained a parent-referencing selector
// (& or :scope), we need to keep it so that we still consider the :is()
// as containing that selector; furthermore, we need to keep it on serialization
// so that a round-trip doesn't lose this information.
// We have similar weaknesses here as in CSS custom properties,
// such as not preserving comments fully.
void CSSSelectorParser::AddPlaceholderSelectorIfNeeded(
const CSSParserTokenRange& argument) {
const bool contains_nest_token = std::any_of(
argument.begin(), argument.end(), [](const CSSParserToken& token) {
return token.GetType() == kDelimiterToken && token.Delimiter() == '&';
});
if (contains_nest_token) {
CSSNestingType nesting_type = CSSNestingType::kNone;

const CSSParserToken* previous_token = nullptr;

for (const CSSParserToken& token : argument) {
if (token.GetType() == kDelimiterToken && token.Delimiter() == '&') {
nesting_type = CSSNestingType::kNesting;
// Note that a nest-containing selector is also scope-containing, so
// no need to look for :scope if '&' has been found.
break;
}
if (previous_token && previous_token->GetType() == kColonToken &&
token.GetType() == kIdentToken &&
EqualIgnoringASCIICase(token.Value(), "scope")) {
DCHECK_EQ(nesting_type, CSSNestingType::kNone);
nesting_type = CSSNestingType::kScope;
}

previous_token = &token;
}

if (nesting_type != CSSNestingType::kNone) {
CSSSelector placeholder_selector;
placeholder_selector.SetMatch(CSSSelector::kPseudoClass);
placeholder_selector.SetUnparsedPlaceholder(
AtomicString(argument.Serialize()));
nesting_type, AtomicString(argument.Serialize()));
placeholder_selector.SetLastInTagHistory(true);
output_.push_back(placeholder_selector);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,7 @@ class CORE_EXPORT CSSSelectorParser {
// https://drafts.csswg.org/selectors/#typedef-relative-selector-list
CSSSelectorList* ConsumeForgivingRelativeSelectorList(CSSParserTokenRange&);
CSSSelectorList* ConsumeRelativeSelectorList(CSSParserTokenRange&);
void AddPlaceholderParentSelectorIfNeeded(
const CSSParserTokenRange& argument);
void AddPlaceholderSelectorIfNeeded(const CSSParserTokenRange& argument);

base::span<CSSSelector> ConsumeNestedRelativeSelector(
CSSParserTokenRange& range);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,7 @@ static absl::optional<CSSSelector::PseudoType> GetImplicitlyAddedPseudo(
return last->GetPseudoType();
}

TEST(CSSSelectorParserTest, NestingContextImpliedDescendant) {
TEST(CSSSelectorParserTest, NestingTypeImpliedDescendant) {
// Nesting selector (&)
EXPECT_EQ(CSSSelector::kPseudoParent,
GetImplicitlyAddedPseudo(".foo", CSSNestingType::kNesting));
Expand Down Expand Up @@ -1263,6 +1263,14 @@ TEST(CSSSelectorParserTest, NestingContextImpliedDescendant) {
GetImplicitlyAddedPseudo(".foo > &", CSSNestingType::kScope));
EXPECT_EQ(absl::nullopt, GetImplicitlyAddedPseudo(".foo > :is(.b, &)",
CSSNestingType::kScope));
EXPECT_EQ(absl::nullopt, GetImplicitlyAddedPseudo(".foo > :is(.b, !&)",
CSSNestingType::kScope));
EXPECT_EQ(absl::nullopt, GetImplicitlyAddedPseudo(".foo > :is(.b, :scope)",
CSSNestingType::kScope));
EXPECT_EQ(absl::nullopt, GetImplicitlyAddedPseudo(".foo > :is(.b, :SCOPE)",
CSSNestingType::kScope));
EXPECT_EQ(absl::nullopt, GetImplicitlyAddedPseudo(".foo > :is(.b, !:scope)",
CSSNestingType::kScope));
EXPECT_EQ(absl::nullopt,
GetImplicitlyAddedPseudo("& .foo", CSSNestingType::kScope));

Expand Down
4 changes: 2 additions & 2 deletions third_party/blink/renderer/core/css/rule_feature_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,13 @@ bool SupportsInvalidation(CSSSelector::PseudoType type) {
case CSSSelector::kPseudoXrOverlay:
case CSSSelector::kPseudoIs:
case CSSSelector::kPseudoWhere:
case CSSSelector::kPseudoParent: // Same as kPseudoIs.
case CSSSelector::kPseudoParentUnparsed: // Never invalidates.
case CSSSelector::kPseudoParent: // Same as kPseudoIs.
case CSSSelector::kPseudoTargetText:
case CSSSelector::kPseudoHighlight:
case CSSSelector::kPseudoSpellingError:
case CSSSelector::kPseudoGrammarError:
case CSSSelector::kPseudoHas:
case CSSSelector::kPseudoUnparsed: // Never invalidates.
case CSSSelector::kPseudoViewTransition:
case CSSSelector::kPseudoViewTransitionGroup:
case CSSSelector::kPseudoViewTransitionImagePair:
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/css/selector_checker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1840,7 +1840,7 @@ bool SelectorChecker::CheckPseudoClass(const SelectorCheckingContext& context,
return !toggle->ValueMatches(State(0));
}
}
case CSSSelector::kPseudoParentUnparsed:
case CSSSelector::kPseudoUnparsed:
// Only kept around for parsing; can never match anything
// (because we don't know what it's supposed to mean).
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ const char* PseudoTypeToString(CSSSelector::PseudoType pseudo_type) {
DEFINE_STRING_MAPPING(PseudoViewTransitionNew);
DEFINE_STRING_MAPPING(PseudoViewTransitionOld);
DEFINE_STRING_MAPPING(PseudoParent);
DEFINE_STRING_MAPPING(PseudoParentUnparsed)
DEFINE_STRING_MAPPING(PseudoUnparsed)
DEFINE_STRING_MAPPING(PseudoInitial)
#undef DEFINE_STRING_MAPPING
}
Expand Down

0 comments on commit 9bf895e

Please sign in to comment.