[:is/:where] Enforce pseudo-element restriction in ConsumeSimpleSelector

When parsing a simple selector, we currently check after-the-fact
whether we followed a pseudo-element, and whether or not the simple
selector was allowed to follow that pseudo-element. If the simple
selector was _not_ allowed to follow a pseudo-element, then parsing
failed altogether.

This does not work well for nested <forgiving-selector-list>s, which
requires partial failure. For example, ::part(foo):is(.a,:hover)
shall behave as ::part(foo):is(:hover) (instead of failing).

This CL instead checks for simple selector validity in
ConsumeSimpleSelector, by storing a "restricting pseudo element"
during parsing of the compound containing the pseudo-element. This
makes it possible for deeply nested selectors to fail at the
appropriate time and trigger the forgiving behavior.

Bug: 568705
Change-Id: Icfb20416c008e37279866a688a525b9419437086
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2450191
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815305}

third_party/blink/renderer/core/css/parser/css_selector_parser.cc

@@ -367,11 +367,22 @@ bool IsSimpleSelectorValidAfterPseudoElement(
   if (simple_selector.Match() != CSSSelector::kPseudoClass)
     return false;
   CSSSelector::PseudoType pseudo = simple_selector.GetPseudoType();
-  if (pseudo == CSSSelector::kPseudoNot) {
-    DCHECK(simple_selector.SelectorList());
-    DCHECK(simple_selector.SelectorList()->First());
-    DCHECK(!simple_selector.SelectorList()->First()->TagHistory());
-    pseudo = simple_selector.SelectorList()->First()->GetPseudoType();
+  switch (pseudo) {
+    case CSSSelector::kPseudoIs:
+    case CSSSelector::kPseudoWhere:
+      // The :is() and :where() pseudo-classes are themselves always valid.
+      // CSSSelectorParser::restricting_pseudo_element_ ensures that invalid
+      // nested selectors will be dropped if they are invalid according to
+      // this function.
+      return true;
+    case CSSSelector::kPseudoNot:
+      DCHECK(simple_selector.SelectorList());
+      DCHECK(simple_selector.SelectorList()->First());
+      DCHECK(!simple_selector.SelectorList()->First()->TagHistory());
+      pseudo = simple_selector.SelectorList()->First()->GetPseudoType();
+      break;
+    default:
+      break;
   }
   return IsPseudoClassValidAfterPseudoElement(pseudo, compound_pseudo_element);
 }
@@ -380,35 +391,27 @@ bool IsSimpleSelectorValidAfterPseudoElement(
 
 std::unique_ptr<CSSParserSelector> CSSSelectorParser::ConsumeCompoundSelector(
     CSSParserTokenRange& range) {
+  base::AutoReset<CSSSelector::PseudoType> reset_restricting(
+      &restricting_pseudo_element_, restricting_pseudo_element_);
+
   std::unique_ptr<CSSParserSelector> compound_selector;
   AtomicString namespace_prefix;
   AtomicString element_name;
-  CSSSelector::PseudoType compound_pseudo_element = CSSSelector::kPseudoUnknown;
   const bool has_q_name = ConsumeName(range, element_name, namespace_prefix);
   if (!has_q_name) {
     compound_selector = ConsumeSimpleSelector(range);
     if (!compound_selector)
       return nullptr;
     if (compound_selector->Match() == CSSSelector::kPseudoElement)
-      compound_pseudo_element = compound_selector->GetPseudoType();
+      restricting_pseudo_element_ = compound_selector->GetPseudoType();
   }
   if (context_->IsHTMLDocument())
     element_name = element_name.LowerASCII();
 
   while (std::unique_ptr<CSSParserSelector> simple_selector =
              ConsumeSimpleSelector(range)) {
-    // TODO(futhark@chromium.org): crbug.com/578131
-    // The UASheetMode check is a work-around to allow this selector in
-    // mediaControls(New).css:
-    // video::-webkit-media-text-track-region-container.scrolling
-    if (context_->Mode() != kUASheetMode &&
-        !IsSimpleSelectorValidAfterPseudoElement(*simple_selector.get(),
-                                                 compound_pseudo_element)) {
-      failed_parsing_ = true;
-      return nullptr;
-    }
     if (simple_selector->Match() == CSSSelector::kPseudoElement)
-      compound_pseudo_element = simple_selector->GetPseudoType();
+      restricting_pseudo_element_ = simple_selector->GetPseudoType();
 
     if (compound_selector)
       compound_selector = AddSimpleSelectorToCompound(
@@ -455,8 +458,15 @@ std::unique_ptr<CSSParserSelector> CSSSelectorParser::ConsumeSimpleSelector(
     selector = ConsumePseudo(range);
   else
     return nullptr;
-  if (!selector)
+  // TODO(futhark@chromium.org): crbug.com/578131
+  // The UASheetMode check is a work-around to allow this selector in
+  // mediaControls(New).css:
+  // video::-webkit-media-text-track-region-container.scrolling
+  if (!selector || (context_->Mode() != kUASheetMode &&
+                    !IsSimpleSelectorValidAfterPseudoElement(
+                        *selector.get(), restricting_pseudo_element_))) {
     failed_parsing_ = true;
+  }
   return selector;
 }
 

third_party/blink/renderer/core/css/parser/css_selector_parser.h

@@ -106,6 +106,12 @@ class CORE_EXPORT CSSSelectorParser {
   // for example :host, inner :is()/:where() pseudo classes are also only
   // allowed to contain compound selectors.
   bool inside_compound_pseudo_ = false;
+  // When parsing a compound which includes a pseudo-element, the simple
+  // selectors permitted to follow that pseudo-element may be restricted.
+  // If this is the case, then restricting_pseudo_element_ will be set to the
+  // PseudoType of the pseudo-element causing the restriction.
+  CSSSelector::PseudoType restricting_pseudo_element_ =
+      CSSSelector::kPseudoUnknown;
 
   class DisallowPseudoElementsScope {
     STACK_ALLOCATED();

third_party/blink/renderer/core/css/parser/css_selector_parser_test.cc

@@ -482,6 +482,28 @@ static const SelectorTestCase is_where_nesting_data[] = {
     {"::cue(:is(.a .b))", "::cue(:is())"},
     {"::cue(:is(.a + .b))", "::cue(:is())"},
     {"::cue(:is(.a, .b + .c))", "::cue(:is(.a))"},
+    // Only user-action pseudos + :state() are allowed after kPseudoPart:
+    {"::part(foo):is(.a)", "::part(foo):is()"},
+    {"::part(foo):is(.a:hover)", "::part(foo):is()"},
+    {"::part(foo):is(:hover.a)", "::part(foo):is()"},
+    {"::part(foo):is(:hover + .a)", "::part(foo):is()"},
+    {"::part(foo):is(.a + :hover)", "::part(foo):is()"},
+    {"::part(foo):is(:hover:enabled)", "::part(foo):is()"},
+    {"::part(foo):is(:enabled:hover)", "::part(foo):is()"},
+    {"::part(foo):is(:hover, :where(.a))",
+     "::part(foo):is(:hover, :where())"},
+    {"::part(foo):is(:hover, .a)", "::part(foo):is(:hover)"},
+    {"::part(foo):is(:state(bar), .a)", "::part(foo):is(:state(bar))"},
+    {"::part(foo):is(:enabled)", "::part(foo):is()"},
+    // Only scrollbar pseudos after kPseudoScrollbar:
+    {"::-webkit-scrollbar:is(:focus)", "::-webkit-scrollbar:is()"},
+    // Only :window-inactive after kPseudoSelection:
+    {"::selection:is(:focus)", "::selection:is()"},
+    // Only user-action pseudos after webkit pseudos:
+    {"::-webkit-input-placeholder:is(:enabled)",
+     "::-webkit-input-placeholder:is()"},
+    {"::-webkit-input-placeholder:is(:not(:enabled))",
+     "::-webkit-input-placeholder:is()"},
 
     // Valid selectors:
     {":is(.a, .b)"},
@@ -501,6 +523,19 @@ static const SelectorTestCase is_where_nesting_data[] = {
     {"::cue(:is(.a))"},
     {"::cue(:is(div.a))"},
     {"::cue(:is(.a, .b))"},
+    {"::part(foo):is(:hover)"},
+    {"::part(foo):is(:hover:focus)"},
+    {"::part(foo):is(:is(:hover))"},
+    {"::part(foo):is(:focus, :hover)"},
+    {"::part(foo):is(:focus, :is(:hover))"},
+    {"::part(foo):is(:focus, :state(bar))"},
+    {"::-webkit-scrollbar:is(:enabled)"},
+    {"::selection:is(:window-inactive)"},
+    {"::-webkit-input-placeholder:is(:hover)"},
+    {"::-webkit-input-placeholder:is(:not(:hover))"},
+    {"::-webkit-input-placeholder:where(:hover)"},
+    {"::-webkit-input-placeholder:is()"},
+    {"::-webkit-input-placeholder:is(:where(:hover))"},
     // clang-format on
 };
 

third_party/blink/web_tests/external/wpt/css/selectors/is-where-parsing-expected.txt

-This is a testharness.js-based test.
-PASS Multiple selectors with combinators
-PASS Nested :is
-PASS Nested :where
-PASS Nested inside :host, without combinators
-PASS Nested inside :host, with combinators
-PASS Pseudo-classes inside
-PASS Pseudo-classes after
-PASS Pseudo-elements after
-PASS Pseudo-elements inside
-PASS Combinators after
-FAIL After part with simple pseudo-class assert_equals: After part with simple pseudo-class: ::part(foo):is(:hover) expected "::part(foo):is(:hover)" but got "random-selector"
-PASS After part with invalid selector after
-Harness: the test ran to completion.
-