LibWeb/CSS: Keep invalid parts of <forgiving-selector-list>s around

Attempt 2! Reverts 2a5dbedad4

This time, set up a different combinator when producing a relative
invalid selector rather than a standalone one. This fixes the crash.

Original description below for simplicity because it still applies.

---

Selectors like `:is(.valid, &!?!?!invalid)` need to keep the invalid
part around, even though it will never match, for a couple of reasons:

- Serialization needs to include them
- For nesting, we care if a `&` appeared anywhere in the selector, even
  in an invalid part.

So this patch introduces an `Invalid` simple selector type, which simply
holds its original ComponentValues. We search through these looking for
`&`, and we dump them out directly when asked to serialize.

Libraries/LibWeb/CSS/Parser/SelectorParsing.cpp

@@ -60,6 +60,29 @@ Optional<Selector::PseudoElement> Parser::parse_as_pseudo_element_selector()
     return simple_selector.pseudo_element();
 }
 
+static NonnullRefPtr<Selector> create_invalid_selector(Selector::Combinator combinator, Vector<ComponentValue> component_values)
+{
+    // Trim leading and trailing whitespace
+    while (!component_values.is_empty() && component_values.first().is(Token::Type::Whitespace)) {
+        component_values.take_first();
+    }
+    while (!component_values.is_empty() && component_values.last().is(Token::Type::Whitespace)) {
+        component_values.take_last();
+    }
+
+    Selector::SimpleSelector simple {
+        .type = Selector::SimpleSelector::Type::Invalid,
+        .value = Selector::SimpleSelector::Invalid {
+            .component_values = move(component_values),
+        }
+    };
+    Selector::CompoundSelector compound {
+        .combinator = combinator,
+        .simple_selectors = { move(simple) }
+    };
+    return Selector::create({ move(compound) });
+}
+
 template<typename T>
 Parser::ParseErrorOr<SelectorList> Parser::parse_a_selector_list(TokenStream<T>& tokens, SelectorType mode, SelectorParsingMode parsing_mode)
 {
@@ -70,8 +93,12 @@ Parser::ParseErrorOr<SelectorList> Parser::parse_a_selector_list(TokenStream<T>&
         auto stream = TokenStream(selector_parts);
         auto selector = parse_complex_selector(stream, mode);
         if (selector.is_error()) {
-            if (parsing_mode == SelectorParsingMode::Forgiving)
+            if (parsing_mode == SelectorParsingMode::Forgiving) {
+                // Keep the invalid selector around for serialization and nesting
+                auto combinator = mode == SelectorType::Standalone ? Selector::Combinator::None : Selector::Combinator::Descendant;
+                selectors.append(create_invalid_selector(combinator, move(selector_parts)));
                 continue;
+            }
             return selector.error();
         }
         selectors.append(selector.release_value());

Libraries/LibWeb/CSS/Selector.cpp

@@ -11,6 +11,28 @@
 
 namespace Web::CSS {
 
+static bool component_value_contains_nesting_selector(Parser::ComponentValue const& component_value)
+{
+    if (component_value.is_delim('&'))
+        return true;
+
+    if (component_value.is_block()) {
+        for (auto const& child_value : component_value.block().value) {
+            if (component_value_contains_nesting_selector(child_value))
+                return true;
+        }
+    }
+
+    if (component_value.is_function()) {
+        for (auto const& child_value : component_value.function().value) {
+            if (component_value_contains_nesting_selector(child_value))
+                return true;
+        }
+    }
+
+    return false;
+}
+
 Selector::Selector(Vector<CompoundSelector>&& compound_selectors)
     : m_compound_selectors(move(compound_selectors))
 {
@@ -44,6 +66,17 @@ Selector::Selector(Vector<CompoundSelector>&& compound_selectors)
                 if (m_contains_the_nesting_selector)
                     break;
             }
+            if (simple_selector.type == SimpleSelector::Type::Invalid) {
+                auto& invalid = simple_selector.value.get<SimpleSelector::Invalid>();
+                for (auto& item : invalid.component_values) {
+                    if (component_value_contains_nesting_selector(item)) {
+                        m_contains_the_nesting_selector = true;
+                        break;
+                    }
+                }
+                if (m_contains_the_nesting_selector)
+                    break;
+            }
         }
         if (m_contains_the_nesting_selector)
             break;
@@ -184,6 +217,9 @@ u32 Selector::specificity() const
                 // The parented case is handled by replacing & with :is().
                 // So if we got here, the specificity is 0.
                 break;
+            case SimpleSelector::Type::Invalid:
+                // Ignore invalid selectors
+                break;
             }
         }
     }
@@ -360,6 +396,12 @@ String Selector::SimpleSelector::serialize() const
         // AD-HOC: Not in spec yet.
         s.append('&');
         break;
+    case Type::Invalid:
+        // AD-HOC: We're not told how to do these. Just serialize their component values.
+        auto invalid = value.get<Invalid>();
+        for (auto const& component_value : invalid.component_values)
+            s.append(component_value.to_string());
+        break;
     }
     return MUST(s.to_string());
 }
@@ -602,6 +644,7 @@ Selector::SimpleSelector Selector::SimpleSelector::absolutized(Selector::SimpleS
     case Type::Class:
     case Type::Attribute:
     case Type::PseudoElement:
+    case Type::Invalid:
         // Everything else isn't affected
         return *this;
     }

Libraries/LibWeb/CSS/Selector.h

@@ -12,6 +12,7 @@
 #include <AK/String.h>
 #include <AK/Vector.h>
 #include <LibWeb/CSS/Keyword.h>
+#include <LibWeb/CSS/Parser/ComponentValue.h>
 #include <LibWeb/CSS/PseudoClass.h>
 
 namespace Web::CSS {
@@ -92,6 +93,7 @@ public:
             PseudoClass,
             PseudoElement,
             Nesting,
+            Invalid,
         };
 
         struct ANPlusBPattern {
@@ -195,8 +197,12 @@ public:
             CaseType case_type;
         };
 
+        struct Invalid {
+            Vector<Parser::ComponentValue> component_values;
+        };
+
         Type type;
-        Variant<Empty, Attribute, PseudoClassSelector, PseudoElement, Name, QualifiedName> value {};
+        Variant<Empty, Attribute, PseudoClassSelector, PseudoElement, Name, QualifiedName, Invalid> value {};
 
         Attribute const& attribute() const { return value.get<Attribute>(); }
         Attribute& attribute() { return value.get<Attribute>(); }

Libraries/LibWeb/CSS/SelectorEngine.cpp

@@ -764,6 +764,9 @@ static inline bool matches(CSS::Selector::SimpleSelector const& component, Optio
         // :is() is handled already, by us replacing it with :is() directly, so if we
         // got here, it's :scope.
         return matches_pseudo_class(CSS::Selector::SimpleSelector::PseudoClassSelector { .type = CSS::PseudoClass::Scope }, style_sheet_for_rule, element, shadow_host, scope, selector_kind);
+    case CSS::Selector::SimpleSelector::Type::Invalid:
+        // Invalid selectors never match
+        return false;
     }
     VERIFY_NOT_REACHED();
 }

Libraries/LibWeb/Dump.cpp

@@ -506,6 +506,9 @@ void dump_selector(StringBuilder& builder, CSS::Selector const& selector, int in
             case CSS::Selector::SimpleSelector::Type::Nesting:
                 type_description = "Nesting";
                 break;
+            case CSS::Selector::SimpleSelector::Type::Invalid:
+                type_description = "INVALID";
+                break;
             }
 
             builder.appendff("{}:", type_description);
@@ -599,6 +602,14 @@ void dump_selector(StringBuilder& builder, CSS::Selector const& selector, int in
                 builder.appendff(", value='{}']", attribute.value);
             }
 
+            if (simple_selector.type == CSS::Selector::SimpleSelector::Type::Invalid) {
+                auto invalid = simple_selector.value.get<CSS::Selector::SimpleSelector::Invalid>();
+                builder.append(" '"sv);
+                for (auto const& component_value : invalid.component_values)
+                    builder.append(component_value.to_string());
+                builder.append("'"sv);
+            }
+
             if (i != relative_selector.simple_selectors.size() - 1)
                 builder.append(", "sv);
         }

Tests/LibWeb/TestConfig.ini

@@ -61,7 +61,6 @@ Text/input/wpt-import/css/css-flexbox/flex-item-compressible-001.html
 Ref/input/wpt-import/css/css-nesting/has-nesting.html
 Ref/input/wpt-import/css/css-nesting/host-nesting-003.html
 Ref/input/wpt-import/css/css-nesting/host-nesting-004.html
-Ref/input/wpt-import/css/css-nesting/nest-containing-forgiving.html
 Ref/input/wpt-import/css/CSS2/floats/floats-wrap-top-below-bfc-002r.xht
 Ref/input/wpt-import/css/CSS2/floats/float-nowrap-3.html
 Ref/input/wpt-import/css/CSS2/floats/overflow-scroll-float-paint-order.html

Tests/LibWeb/Text/expected/css/serialize-invalid-forgiving-selectors.txt

@@ -0,0 +1,2 @@
+:is(hello, !?) -> :is(hello, !?)
+:is(what, :fake(:imaginary), really) -> :is(what, :fake(:imaginary), really)

Tests/LibWeb/Text/input/css/serialize-invalid-forgiving-selectors.html

@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<style id="style"></style>
+<script src="../include.js"></script>
+<script>
+    test(() => {
+        const selectors = [
+            ":is(hello, !?)",
+            ":is(what, :fake(:imaginary), really)",
+        ];
+        let stylesheet = document.styleSheets[0];
+        for (let selector of selectors) {
+            stylesheet.insertRule(`${selector} { }`);
+            println(`${selector} -> ${stylesheet.cssRules[0].selectorText}`);
+            stylesheet.deleteRule(0);
+        }
+    });
+</script>