ソースを参照

LibWeb: Bring Selector terminology in line with the CSS spec

- CompoundSelector -> *deleted*
- ComplexSelector -> CompoundSelector
- Relation -> Combinator

Our Selector is really a ComplexSelector, but only the Parser and
SelectorEngine need to know that, so keeping it named Selector makes it
more understandable for users.

Our CompoundSelector is really a CompoundSelectorAndCombinator.
Combining the two makes sense in our codebase, but the accurate name is
so long that I think it makes the code less readable.

Renamed some Combinators to also match the spec terminology:

- AdjacentSibling -> NextSibling
- GeneralSibling -> SubsequentSibling

The previous names are somewhat ambiguous, so hopefully this is clearer.
Sam Atkins 4 年 前
コミット
6ea5d03f43

+ 8 - 8
Userland/Libraries/LibWeb/CSS/Parser/DeprecatedCSSParser.cpp

@@ -1081,9 +1081,9 @@ public:
         return simple_selector;
     }
 
-    Optional<CSS::Selector::ComplexSelector> parse_complex_selector()
+    Optional<CSS::Selector::CompoundSelector> parse_complex_selector()
     {
-        auto relation = CSS::Selector::ComplexSelector::Relation::Descendant;
+        auto relation = CSS::Selector::Combinator::Descendant;
 
         if (peek() == '{' || peek() == ',')
             return {};
@@ -1091,13 +1091,13 @@ public:
         if (is_combinator(peek())) {
             switch (peek()) {
             case '>':
-                relation = CSS::Selector::ComplexSelector::Relation::ImmediateChild;
+                relation = CSS::Selector::Combinator::ImmediateChild;
                 break;
             case '+':
-                relation = CSS::Selector::ComplexSelector::Relation::AdjacentSibling;
+                relation = CSS::Selector::Combinator::NextSibling;
                 break;
             case '~':
-                relation = CSS::Selector::ComplexSelector::Relation::GeneralSibling;
+                relation = CSS::Selector::Combinator::SubsequentSibling;
                 break;
             }
             consume_one();
@@ -1119,12 +1119,12 @@ public:
         if (simple_selectors.is_empty())
             return {};
 
-        return CSS::Selector::ComplexSelector { relation, move(simple_selectors) };
+        return CSS::Selector::CompoundSelector { relation, move(simple_selectors) };
     }
 
     void parse_selector()
     {
-        Vector<CSS::Selector::ComplexSelector> complex_selectors;
+        Vector<CSS::Selector::CompoundSelector> complex_selectors;
 
         for (;;) {
             auto index_before = index;
@@ -1141,7 +1141,7 @@ public:
 
         if (complex_selectors.is_empty())
             return;
-        complex_selectors.first().relation = CSS::Selector::ComplexSelector::Relation::None;
+        complex_selectors.first().combinator = CSS::Selector::Combinator::None;
 
         current_rule.selectors.append(CSS::Selector::create(move(complex_selectors)));
     }

+ 9 - 9
Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp

@@ -229,7 +229,7 @@ RefPtr<Selector> Parser::parse_single_selector(TokenStream<T>& tokens, bool is_r
 
     // FIXME: Bring this all in line with the spec. https://www.w3.org/TR/selectors-4/
 
-    Vector<Selector::ComplexSelector> selectors;
+    Vector<Selector::CompoundSelector> selectors;
 
     auto check_for_eof_or_whitespace = [&](T& current_value) -> bool {
         if (current_value.is(Token::Type::EndOfFile))
@@ -491,8 +491,8 @@ RefPtr<Selector> Parser::parse_single_selector(TokenStream<T>& tokens, bool is_r
         return simple_selector;
     };
 
-    auto parse_complex_selector = [&]() -> Optional<Selector::ComplexSelector> {
-        auto relation = Selector::ComplexSelector::Relation::Descendant;
+    auto parse_complex_selector = [&]() -> Optional<Selector::CompoundSelector> {
+        auto combinator = Selector::Combinator::Descendant;
 
         tokens.skip_whitespace();
 
@@ -500,13 +500,13 @@ RefPtr<Selector> Parser::parse_single_selector(TokenStream<T>& tokens, bool is_r
         if (current_value.is(Token::Type::Delim)) {
             auto delim = ((Token)current_value).delim();
             if (delim == ">") {
-                relation = Selector::ComplexSelector::Relation::ImmediateChild;
+                combinator = Selector::Combinator::ImmediateChild;
                 tokens.next_token();
             } else if (delim == "+") {
-                relation = Selector::ComplexSelector::Relation::AdjacentSibling;
+                combinator = Selector::Combinator::NextSibling;
                 tokens.next_token();
             } else if (delim == "~") {
-                relation = Selector::ComplexSelector::Relation::GeneralSibling;
+                combinator = Selector::Combinator::SubsequentSibling;
                 tokens.next_token();
             } else if (delim == "|") {
                 tokens.next_token();
@@ -516,7 +516,7 @@ RefPtr<Selector> Parser::parse_single_selector(TokenStream<T>& tokens, bool is_r
                     return {};
 
                 if (next.is(Token::Type::Delim) && next.token().delim() == "|") {
-                    relation = Selector::ComplexSelector::Relation::Column;
+                    combinator = Selector::Combinator::Column;
                     tokens.next_token();
                 }
             }
@@ -541,7 +541,7 @@ RefPtr<Selector> Parser::parse_single_selector(TokenStream<T>& tokens, bool is_r
         if (simple_selectors.is_empty())
             return {};
 
-        return Selector::ComplexSelector { relation, move(simple_selectors) };
+        return Selector::CompoundSelector { combinator, move(simple_selectors) };
     };
 
     for (;;) {
@@ -558,7 +558,7 @@ RefPtr<Selector> Parser::parse_single_selector(TokenStream<T>& tokens, bool is_r
         return {};
 
     if (!is_relative)
-        selectors.first().relation = Selector::ComplexSelector::Relation::None;
+        selectors.first().combinator = Selector::Combinator::None;
 
     return Selector::create(move(selectors));
 }

+ 4 - 4
Userland/Libraries/LibWeb/CSS/Selector.cpp

@@ -11,8 +11,8 @@
 
 namespace Web::CSS {
 
-Selector::Selector(Vector<ComplexSelector>&& component_lists)
-    : m_complex_selectors(move(component_lists))
+Selector::Selector(Vector<CompoundSelector>&& compound_selectors)
+    : m_compound_selectors(move(compound_selectors))
 {
 }
 
@@ -26,8 +26,8 @@ u32 Selector::specificity() const
     unsigned tag_names = 0;
     unsigned classes = 0;
 
-    for (auto& list : m_complex_selectors) {
-        for (auto& simple_selector : list.compound_selector) {
+    for (auto& list : m_compound_selectors) {
+        for (auto& simple_selector : list.simple_selectors) {
             switch (simple_selector.type) {
             case SimpleSelector::Type::Id:
                 ++ids;

+ 22 - 18
Userland/Libraries/LibWeb/CSS/Selector.h

@@ -15,6 +15,9 @@
 
 namespace Web::CSS {
 
+using SelectorList = NonnullRefPtrVector<class Selector>;
+
+// This is a <complex-selector> in the spec. https://www.w3.org/TR/selectors-4/#complex
 class Selector : public RefCounted<Selector> {
 public:
     struct SimpleSelector {
@@ -65,7 +68,7 @@ public:
             // Only used when "pseudo_class" is "NthChild" or "NthLastChild".
             NthChildPattern nth_child_pattern;
 
-            NonnullRefPtrVector<Selector> not_selector {};
+            SelectorList not_selector {};
         };
         PseudoClass pseudo_class;
 
@@ -98,36 +101,37 @@ public:
         Attribute attribute;
     };
 
-    struct ComplexSelector {
-        enum class Relation {
-            None,
-            ImmediateChild,
-            Descendant,
-            AdjacentSibling,
-            GeneralSibling,
-            Column,
-        };
-        Relation relation { Relation::None };
+    enum class Combinator {
+        None,
+        ImmediateChild,    // >
+        Descendant,        // <whitespace>
+        NextSibling,       // +
+        SubsequentSibling, // ~
+        Column,            // ||
+    };
 
-        using CompoundSelector = Vector<SimpleSelector>;
-        CompoundSelector compound_selector;
+    struct CompoundSelector {
+        // Spec-wise, the <combinator> is not part of a <compound-selector>,
+        // but it is more understandable to put them together.
+        Combinator combinator { Combinator::None };
+        Vector<SimpleSelector> simple_selectors;
     };
 
-    static NonnullRefPtr<Selector> create(Vector<ComplexSelector>&& complex_selectors)
+    static NonnullRefPtr<Selector> create(Vector<CompoundSelector>&& compound_selectors)
     {
-        return adopt_ref(*new Selector(move(complex_selectors)));
+        return adopt_ref(*new Selector(move(compound_selectors)));
     }
 
     ~Selector();
 
-    Vector<ComplexSelector> const& complex_selectors() const { return m_complex_selectors; }
+    Vector<CompoundSelector> const& compound_selectors() const { return m_compound_selectors; }
 
     u32 specificity() const;
 
 private:
-    explicit Selector(Vector<ComplexSelector>&&);
+    explicit Selector(Vector<CompoundSelector>&&);
 
-    Vector<ComplexSelector> m_complex_selectors;
+    Vector<CompoundSelector> m_compound_selectors;
 };
 
 }

+ 12 - 12
Userland/Libraries/LibWeb/CSS/SelectorEngine.cpp

@@ -195,15 +195,15 @@ static bool matches(CSS::Selector::SimpleSelector const& component, DOM::Element
 
 static bool matches(CSS::Selector const& selector, int component_list_index, DOM::Element const& element)
 {
-    auto& component_list = selector.complex_selectors()[component_list_index];
-    for (auto& component : component_list.compound_selector) {
-        if (!matches(component, element))
+    auto& relative_selector = selector.compound_selectors()[component_list_index];
+    for (auto& simple_selector : relative_selector.simple_selectors) {
+        if (!matches(simple_selector, element))
             return false;
     }
-    switch (component_list.relation) {
-    case CSS::Selector::ComplexSelector::Relation::None:
+    switch (relative_selector.combinator) {
+    case CSS::Selector::Combinator::None:
         return true;
-    case CSS::Selector::ComplexSelector::Relation::Descendant:
+    case CSS::Selector::Combinator::Descendant:
         VERIFY(component_list_index != 0);
         for (auto* ancestor = element.parent(); ancestor; ancestor = ancestor->parent()) {
             if (!is<DOM::Element>(*ancestor))
@@ -212,24 +212,24 @@ static bool matches(CSS::Selector const& selector, int component_list_index, DOM
                 return true;
         }
         return false;
-    case CSS::Selector::ComplexSelector::Relation::ImmediateChild:
+    case CSS::Selector::Combinator::ImmediateChild:
         VERIFY(component_list_index != 0);
         if (!element.parent() || !is<DOM::Element>(*element.parent()))
             return false;
         return matches(selector, component_list_index - 1, verify_cast<DOM::Element>(*element.parent()));
-    case CSS::Selector::ComplexSelector::Relation::AdjacentSibling:
+    case CSS::Selector::Combinator::NextSibling:
         VERIFY(component_list_index != 0);
         if (auto* sibling = element.previous_element_sibling())
             return matches(selector, component_list_index - 1, *sibling);
         return false;
-    case CSS::Selector::ComplexSelector::Relation::GeneralSibling:
+    case CSS::Selector::Combinator::SubsequentSibling:
         VERIFY(component_list_index != 0);
         for (auto* sibling = element.previous_element_sibling(); sibling; sibling = sibling->previous_element_sibling()) {
             if (matches(selector, component_list_index - 1, *sibling))
                 return true;
         }
         return false;
-    case CSS::Selector::ComplexSelector::Relation::Column:
+    case CSS::Selector::Combinator::Column:
         TODO();
     }
     VERIFY_NOT_REACHED();
@@ -237,8 +237,8 @@ static bool matches(CSS::Selector const& selector, int component_list_index, DOM
 
 bool matches(CSS::Selector const& selector, DOM::Element const& element)
 {
-    VERIFY(!selector.complex_selectors().is_empty());
-    return matches(selector, selector.complex_selectors().size() - 1, element);
+    VERIFY(!selector.compound_selectors().is_empty());
+    return matches(selector, selector.compound_selectors().size() - 1, element);
 }
 
 }

+ 11 - 11
Userland/Libraries/LibWeb/Dump.cpp

@@ -267,27 +267,27 @@ void dump_selector(StringBuilder& builder, CSS::Selector const& selector)
 {
     builder.append("  CSS::Selector:\n");
 
-    for (auto& complex_selector : selector.complex_selectors()) {
+    for (auto& relative_selector : selector.compound_selectors()) {
         builder.append("    ");
 
         char const* relation_description = "";
-        switch (complex_selector.relation) {
-        case CSS::Selector::ComplexSelector::Relation::None:
+        switch (relative_selector.combinator) {
+        case CSS::Selector::Combinator::None:
             relation_description = "None";
             break;
-        case CSS::Selector::ComplexSelector::Relation::ImmediateChild:
+        case CSS::Selector::Combinator::ImmediateChild:
             relation_description = "ImmediateChild";
             break;
-        case CSS::Selector::ComplexSelector::Relation::Descendant:
+        case CSS::Selector::Combinator::Descendant:
             relation_description = "Descendant";
             break;
-        case CSS::Selector::ComplexSelector::Relation::AdjacentSibling:
+        case CSS::Selector::Combinator::NextSibling:
             relation_description = "AdjacentSibling";
             break;
-        case CSS::Selector::ComplexSelector::Relation::GeneralSibling:
+        case CSS::Selector::Combinator::SubsequentSibling:
             relation_description = "GeneralSibling";
             break;
-        case CSS::Selector::ComplexSelector::Relation::Column:
+        case CSS::Selector::Combinator::Column:
             relation_description = "Column";
             break;
         }
@@ -295,8 +295,8 @@ void dump_selector(StringBuilder& builder, CSS::Selector const& selector)
         if (*relation_description)
             builder.appendff("{{{}}} ", relation_description);
 
-        for (size_t i = 0; i < complex_selector.compound_selector.size(); ++i) {
-            auto& simple_selector = complex_selector.compound_selector[i];
+        for (size_t i = 0; i < relative_selector.simple_selectors.size(); ++i) {
+            auto& simple_selector = relative_selector.simple_selectors[i];
             char const* type_description = "Unknown";
             switch (simple_selector.type) {
             case CSS::Selector::SimpleSelector::Type::Invalid:
@@ -459,7 +459,7 @@ void dump_selector(StringBuilder& builder, CSS::Selector const& selector)
                 builder.appendff(" [{}, name='{}', value='{}']", attribute_match_type_description, simple_selector.attribute.name, simple_selector.attribute.value);
             }
 
-            if (i != complex_selector.compound_selector.size() - 1)
+            if (i != relative_selector.simple_selectors.size() - 1)
                 builder.append(", ");
         }
         builder.append("\n");