From 2a5dbedad4e76fbaaab4ba6a6e0e0a740260af05 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 13 Nov 2024 21:37:34 +0100 Subject: [PATCH] Revert "LibWeb/CSS: Keep invalid parts of ``s around" This reverts commit 698dd600f2dfc57ffad3c004628d5abf0816635a. This caused multiple tests to crash on macOS: https://github.com/LadybirdBrowser/ladybird/pull/2317#issuecomment-2474725826 --- .../LibWeb/CSS/Parser/SelectorParsing.cpp | 28 +----------- Libraries/LibWeb/CSS/Selector.cpp | 43 ------------------- Libraries/LibWeb/CSS/Selector.h | 8 +--- Libraries/LibWeb/CSS/SelectorEngine.cpp | 3 -- Libraries/LibWeb/Dump.cpp | 11 ----- Tests/LibWeb/TestConfig.ini | 1 + .../serialize-invalid-forgiving-selectors.txt | 2 - ...serialize-invalid-forgiving-selectors.html | 17 -------- 8 files changed, 3 insertions(+), 110 deletions(-) delete mode 100644 Tests/LibWeb/Text/expected/css/serialize-invalid-forgiving-selectors.txt delete mode 100644 Tests/LibWeb/Text/input/css/serialize-invalid-forgiving-selectors.html diff --git a/Libraries/LibWeb/CSS/Parser/SelectorParsing.cpp b/Libraries/LibWeb/CSS/Parser/SelectorParsing.cpp index 00c8e2866ed..16762e45a2f 100644 --- a/Libraries/LibWeb/CSS/Parser/SelectorParsing.cpp +++ b/Libraries/LibWeb/CSS/Parser/SelectorParsing.cpp @@ -60,29 +60,6 @@ Optional Parser::parse_as_pseudo_element_selector() return simple_selector.pseudo_element(); } -static NonnullRefPtr create_invalid_selector(Vector 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 = Selector::Combinator::None, - .simple_selectors = { move(simple) } - }; - return Selector::create({ move(compound) }); -} - template Parser::ParseErrorOr Parser::parse_a_selector_list(TokenStream& tokens, SelectorType mode, SelectorParsingMode parsing_mode) { @@ -93,11 +70,8 @@ Parser::ParseErrorOr Parser::parse_a_selector_list(TokenStream& auto stream = TokenStream(selector_parts); auto selector = parse_complex_selector(stream, mode); if (selector.is_error()) { - if (parsing_mode == SelectorParsingMode::Forgiving) { - // Keep the invalid selector around for serialization and nesting - selectors.append(create_invalid_selector(move(selector_parts))); + if (parsing_mode == SelectorParsingMode::Forgiving) continue; - } return selector.error(); } selectors.append(selector.release_value()); diff --git a/Libraries/LibWeb/CSS/Selector.cpp b/Libraries/LibWeb/CSS/Selector.cpp index 2282ff08451..266391e4f8a 100644 --- a/Libraries/LibWeb/CSS/Selector.cpp +++ b/Libraries/LibWeb/CSS/Selector.cpp @@ -11,28 +11,6 @@ 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&& compound_selectors) : m_compound_selectors(move(compound_selectors)) { @@ -66,17 +44,6 @@ Selector::Selector(Vector&& compound_selectors) if (m_contains_the_nesting_selector) break; } - if (simple_selector.type == SimpleSelector::Type::Invalid) { - auto& invalid = simple_selector.value.get(); - 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; @@ -217,9 +184,6 @@ 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; } } } @@ -396,12 +360,6 @@ 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(); - for (auto const& component_value : invalid.component_values) - s.append(component_value.to_string()); - break; } return MUST(s.to_string()); } @@ -644,7 +602,6 @@ 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; } diff --git a/Libraries/LibWeb/CSS/Selector.h b/Libraries/LibWeb/CSS/Selector.h index b8d2425861a..3e0a1055c3f 100644 --- a/Libraries/LibWeb/CSS/Selector.h +++ b/Libraries/LibWeb/CSS/Selector.h @@ -12,7 +12,6 @@ #include #include #include -#include #include namespace Web::CSS { @@ -93,7 +92,6 @@ public: PseudoClass, PseudoElement, Nesting, - Invalid, }; struct ANPlusBPattern { @@ -197,12 +195,8 @@ public: CaseType case_type; }; - struct Invalid { - Vector component_values; - }; - Type type; - Variant value {}; + Variant value {}; Attribute const& attribute() const { return value.get(); } Attribute& attribute() { return value.get(); } diff --git a/Libraries/LibWeb/CSS/SelectorEngine.cpp b/Libraries/LibWeb/CSS/SelectorEngine.cpp index 1a9f3b30849..6802fa73cc1 100644 --- a/Libraries/LibWeb/CSS/SelectorEngine.cpp +++ b/Libraries/LibWeb/CSS/SelectorEngine.cpp @@ -764,9 +764,6 @@ 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(); } diff --git a/Libraries/LibWeb/Dump.cpp b/Libraries/LibWeb/Dump.cpp index 7de37a60911..0632bcd734a 100644 --- a/Libraries/LibWeb/Dump.cpp +++ b/Libraries/LibWeb/Dump.cpp @@ -506,9 +506,6 @@ 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); @@ -602,14 +599,6 @@ 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(); - 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); } diff --git a/Tests/LibWeb/TestConfig.ini b/Tests/LibWeb/TestConfig.ini index 15e49763f3d..63ad0d7874a 100644 --- a/Tests/LibWeb/TestConfig.ini +++ b/Tests/LibWeb/TestConfig.ini @@ -61,6 +61,7 @@ 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 diff --git a/Tests/LibWeb/Text/expected/css/serialize-invalid-forgiving-selectors.txt b/Tests/LibWeb/Text/expected/css/serialize-invalid-forgiving-selectors.txt deleted file mode 100644 index 3a4510100c4..00000000000 --- a/Tests/LibWeb/Text/expected/css/serialize-invalid-forgiving-selectors.txt +++ /dev/null @@ -1,2 +0,0 @@ -:is(hello, !?) -> :is(hello, !?) -:is(what, :fake(:imaginary), really) -> :is(what, :fake(:imaginary), really) diff --git a/Tests/LibWeb/Text/input/css/serialize-invalid-forgiving-selectors.html b/Tests/LibWeb/Text/input/css/serialize-invalid-forgiving-selectors.html deleted file mode 100644 index d08b4ae3fb7..00000000000 --- a/Tests/LibWeb/Text/input/css/serialize-invalid-forgiving-selectors.html +++ /dev/null @@ -1,17 +0,0 @@ - - - -