From ffc81cbfad8af71674b8d63706b7166fa71509d5 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Mon, 12 Jul 2021 17:58:47 +0100 Subject: [PATCH] LibWeb: Use Selectors instead of a String for :not() selectors Rather than parsing the selector every time we want to check it, we now parse it once at the beginning. A bonus effect of this is that we now support a selector list in :not(), instead of just a single selector, though only when using the new parser. --- Base/res/html/misc/not-selector.html | 20 +++++++++++++++++++ Base/res/html/misc/welcome.html | 1 + .../LibWeb/CSS/Parser/DeprecatedCSSParser.cpp | 6 +++++- .../Libraries/LibWeb/CSS/Parser/Parser.cpp | 3 ++- .../LibWeb/CSS/Parser/StyleFunctionRule.h | 5 ----- Userland/Libraries/LibWeb/CSS/Selector.h | 4 ++-- .../Libraries/LibWeb/CSS/SelectorEngine.cpp | 15 ++++++-------- Userland/Libraries/LibWeb/Dump.cpp | 5 ++++- 8 files changed, 40 insertions(+), 19 deletions(-) create mode 100644 Base/res/html/misc/not-selector.html diff --git a/Base/res/html/misc/not-selector.html b/Base/res/html/misc/not-selector.html new file mode 100644 index 00000000000..21200ccf486 --- /dev/null +++ b/Base/res/html/misc/not-selector.html @@ -0,0 +1,20 @@ + + +:only-child test + + + +
I am not a descendant and should be green.
+
+
I am a descendant and should be yellow.
+
+ + diff --git a/Base/res/html/misc/welcome.html b/Base/res/html/misc/welcome.html index 494b64cdbd7..785b0976251 100644 --- a/Base/res/html/misc/welcome.html +++ b/Base/res/html/misc/welcome.html @@ -109,6 +109,7 @@
  • :nth-last-child
  • :empty
  • :root
    • +
  • :not
  • form
  • borders
  • css
    • diff --git a/Userland/Libraries/LibWeb/CSS/Parser/DeprecatedCSSParser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/DeprecatedCSSParser.cpp index 6e7972d8f42..2fd9ed2b5a7 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/DeprecatedCSSParser.cpp +++ b/Userland/Libraries/LibWeb/CSS/Parser/DeprecatedCSSParser.cpp @@ -606,7 +606,11 @@ public: pseudo_class.type = CSS::Selector::SimpleSelector::PseudoClass::Type::Checked; } else if (pseudo_name.starts_with("not", CaseSensitivity::CaseInsensitive)) { pseudo_class.type = CSS::Selector::SimpleSelector::PseudoClass::Type::Not; - pseudo_class.not_selector = capture_selector_args(pseudo_name); + auto not_selector = Web::parse_selector(m_context, capture_selector_args(pseudo_name)); + if (not_selector) { + pseudo_class.not_selector.clear(); + pseudo_class.not_selector.append(not_selector.release_nonnull()); + } } else { dbgln("Unknown pseudo class: '{}'", pseudo_name); return {}; diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp index 4e5480e1475..679d28cfc64 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp @@ -469,7 +469,8 @@ RefPtr Parser::parse_single_selector(TokenStream& tokens, bool is_r } } else if (pseudo_function.name().equals_ignoring_case("not")) { pseudo_class.type = Selector::SimpleSelector::PseudoClass::Type::Not; - pseudo_class.not_selector = pseudo_function.values_as_string(); + auto function_token_stream = TokenStream(pseudo_function.values()); + pseudo_class.not_selector = parse_a_selector(function_token_stream); } else { dbgln("Unknown pseudo class: '{}'()", pseudo_function.name()); return {}; diff --git a/Userland/Libraries/LibWeb/CSS/Parser/StyleFunctionRule.h b/Userland/Libraries/LibWeb/CSS/Parser/StyleFunctionRule.h index 7934e9d1b1a..ff7bebf5176 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/StyleFunctionRule.h +++ b/Userland/Libraries/LibWeb/CSS/Parser/StyleFunctionRule.h @@ -24,11 +24,6 @@ public: String const& name() const { return m_name; } Vector const& values() const { return m_values; } - // FIXME: This method is a temporary hack while much of the parser still expects a string, rather than tokens. - String values_as_string() const - { - return ""; - } String to_string() const; diff --git a/Userland/Libraries/LibWeb/CSS/Selector.h b/Userland/Libraries/LibWeb/CSS/Selector.h index b7c0f18e16f..12f5f9f358b 100644 --- a/Userland/Libraries/LibWeb/CSS/Selector.h +++ b/Userland/Libraries/LibWeb/CSS/Selector.h @@ -8,6 +8,7 @@ #pragma once #include +#include #include #include #include @@ -64,8 +65,7 @@ public: // Only used when "pseudo_class" is "NthChild" or "NthLastChild". NthChildPattern nth_child_pattern; - // FIXME: This wants to be a Selector, rather than parsing it each time it is used. - String not_selector {}; + NonnullRefPtrVector not_selector {}; }; PseudoClass pseudo_class; diff --git a/Userland/Libraries/LibWeb/CSS/SelectorEngine.cpp b/Userland/Libraries/LibWeb/CSS/SelectorEngine.cpp index efd9938453b..de5dd52b676 100644 --- a/Userland/Libraries/LibWeb/CSS/SelectorEngine.cpp +++ b/Userland/Libraries/LibWeb/CSS/SelectorEngine.cpp @@ -113,15 +113,12 @@ static bool matches_pseudo_class(CSS::Selector::SimpleSelector::PseudoClass cons if (!element.has_attribute("checked")) return false; return true; - case CSS::Selector::SimpleSelector::PseudoClass::Type::Not: { - if (pseudo_class.not_selector.is_empty()) - return false; - auto not_selector = Web::parse_selector(CSS::DeprecatedParsingContext(element), pseudo_class.not_selector); - if (!not_selector) - return false; - auto not_matches = matches(not_selector.release_nonnull(), element); - return !not_matches; - } + case CSS::Selector::SimpleSelector::PseudoClass::Type::Not: + for (auto& selector : pseudo_class.not_selector) { + if (matches(selector, element)) + return false; + } + return true; case CSS::Selector::SimpleSelector::PseudoClass::Type::NthChild: case CSS::Selector::SimpleSelector::PseudoClass::Type::NthLastChild: auto const step_size = pseudo_class.nth_child_pattern.step_size; diff --git a/Userland/Libraries/LibWeb/Dump.cpp b/Userland/Libraries/LibWeb/Dump.cpp index 1bb330dca60..466c1b62c97 100644 --- a/Userland/Libraries/LibWeb/Dump.cpp +++ b/Userland/Libraries/LibWeb/Dump.cpp @@ -393,7 +393,10 @@ void dump_selector(StringBuilder& builder, CSS::Selector const& selector) builder.appendff(" pseudo_class={}", pseudo_class_description); if (pseudo_class.type == CSS::Selector::SimpleSelector::PseudoClass::Type::Not) { - builder.appendff("({})", pseudo_class.not_selector); + builder.append("("); + for (auto& selector : pseudo_class.not_selector) + dump_selector(builder, selector); + builder.append(")"); } else if ((pseudo_class.type == CSS::Selector::SimpleSelector::PseudoClass::Type::NthChild) || (pseudo_class.type == CSS::Selector::SimpleSelector::PseudoClass::Type::NthLastChild)) { builder.appendff("(step={}, offset={})", pseudo_class.nth_child_pattern.step_size, pseudo_class.nth_child_pattern.offset);