LibWeb: Add 'Attribute' as a CSS SimpleSelector::Type

Previously, SimpleSelectors optionally had Attribute-selector data
as well as their main type. Now, they're either one or the other,
which better matches the spec, and makes parsing and matching more
straightforward.

Userland/Libraries/LibWeb/CSS/Parser/DeprecatedCSSParser.cpp

@@ -424,29 +424,31 @@ public:
         if (!peek() || peek() == '{' || peek() == ',' || is_combinator(peek()))
             return {};
 
-        CSS::Selector::SimpleSelector::Type type;
+        CSS::Selector::SimpleSelector simple_selector;
 
         if (peek() == '*') {
-            type = CSS::Selector::SimpleSelector::Type::Universal;
+            simple_selector.type = CSS::Selector::SimpleSelector::Type::Universal;
             consume_one();
-            CSS::Selector::SimpleSelector result;
-            result.type = type;
-            return result;
+            return simple_selector;
         }
 
         if (peek() == '.') {
-            type = CSS::Selector::SimpleSelector::Type::Class;
+            simple_selector.type = CSS::Selector::SimpleSelector::Type::Class;
             consume_one();
         } else if (peek() == '#') {
-            type = CSS::Selector::SimpleSelector::Type::Id;
+            simple_selector.type = CSS::Selector::SimpleSelector::Type::Id;
             consume_one();
         } else if (isalpha(peek())) {
-            type = CSS::Selector::SimpleSelector::Type::TagName;
+            simple_selector.type = CSS::Selector::SimpleSelector::Type::TagName;
+        } else if (peek() == '[') {
+            simple_selector.type = CSS::Selector::SimpleSelector::Type::Attribute;
         } else {
-            type = CSS::Selector::SimpleSelector::Type::Universal;
+            simple_selector.type = CSS::Selector::SimpleSelector::Type::Universal;
         }
 
-        if (type != CSS::Selector::SimpleSelector::Type::Universal) {
+        if ((simple_selector.type != CSS::Selector::SimpleSelector::Type::Universal)
+            && (simple_selector.type != CSS::Selector::SimpleSelector::Type::Attribute)) {
+
             while (is_valid_selector_char(peek()))
                 buffer.append(consume_one());
             PARSE_VERIFY(!buffer.is_empty());
@@ -454,18 +456,16 @@ public:
 
         auto value = String::copy(buffer);
 
-        if (type == CSS::Selector::SimpleSelector::Type::TagName) {
+        if (simple_selector.type == CSS::Selector::SimpleSelector::Type::TagName) {
             // Some stylesheets use uppercase tag names, so here's a hack to just lowercase them internally.
             value = value.to_lowercase();
         }
 
-        CSS::Selector::SimpleSelector simple_selector;
-        simple_selector.type = type;
         simple_selector.value = value;
         buffer.clear();
 
-        if (peek() == '[') {
-            CSS::Selector::SimpleSelector::AttributeMatchType attribute_match_type = CSS::Selector::SimpleSelector::AttributeMatchType::HasAttribute;
+        if (simple_selector.type == CSS::Selector::SimpleSelector::Type::Attribute) {
+            CSS::Selector::SimpleSelector::Attribute::MatchType attribute_match_type = CSS::Selector::SimpleSelector::Attribute::MatchType::HasAttribute;
             String attribute_name;
             String attribute_value;
             bool in_value = false;
@@ -475,10 +475,10 @@ public:
                 char ch = consume_one();
                 if (ch == '=' || (ch == '~' && peek() == '=')) {
                     if (ch == '=') {
-                        attribute_match_type = CSS::Selector::SimpleSelector::AttributeMatchType::ExactValueMatch;
+                        attribute_match_type = CSS::Selector::SimpleSelector::Attribute::MatchType::ExactValueMatch;
                     } else if (ch == '~') {
                         consume_one();
-                        attribute_match_type = CSS::Selector::SimpleSelector::AttributeMatchType::ContainsWord;
+                        attribute_match_type = CSS::Selector::SimpleSelector::Attribute::MatchType::ContainsWord;
                     }
                     attribute_name = String::copy(buffer);
                     buffer.clear();
@@ -503,9 +503,9 @@ public:
             else
                 attribute_name = String::copy(buffer);
             buffer.clear();
-            simple_selector.attribute_match_type = attribute_match_type;
-            simple_selector.attribute_name = attribute_name;
-            simple_selector.attribute_value = attribute_value;
+            simple_selector.attribute.match_type = attribute_match_type;
+            simple_selector.attribute.name = attribute_name;
+            simple_selector.attribute.value = attribute_value;
             if (expected_end_of_attribute_selector != ']') {
                 if (!consume_specific(expected_end_of_attribute_selector))
                     return {};

Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp

@@ -243,26 +243,18 @@ Optional<Selector> Parser::parse_single_selector(TokenStream<T>& tokens, bool is
         if (check_for_eof_or_whitespace(current_value))
             return {};
 
-        Selector::SimpleSelector::Type type;
-        String value;
+        Selector::SimpleSelector simple_selector;
         // FIXME: Handle namespace prefixes.
 
         if (current_value.is(Token::Type::Delim) && ((Token)current_value).delim() == "*") {
-
-            // FIXME: Handle selectors like `*.foo`.
-            type = Selector::SimpleSelector::Type::Universal;
-            Selector::SimpleSelector result;
-            result.type = type;
-            return result;
-        }
-
-        if (current_value.is(Token::Type::Hash)) {
+            simple_selector.type = Selector::SimpleSelector::Type::Universal;
+        } else if (current_value.is(Token::Type::Hash)) {
             if (((Token)current_value).m_hash_type != Token::HashType::Id) {
                 dbgln("Selector contains hash token that is not an id: {}", current_value.to_debug_string());
                 return {};
             }
-            type = Selector::SimpleSelector::Type::Id;
-            value = ((Token)current_value).m_value.to_string();
+            simple_selector.type = Selector::SimpleSelector::Type::Id;
+            simple_selector.value = ((Token)current_value).m_value.to_string();
         } else if (current_value.is(Token::Type::Delim) && ((Token)current_value).delim() == ".") {
             current_value = tokens.next_token();
             if (check_for_eof_or_whitespace(current_value))
@@ -273,33 +265,19 @@ Optional<Selector> Parser::parse_single_selector(TokenStream<T>& tokens, bool is
                 return {};
             }
 
-            type = Selector::SimpleSelector::Type::Class;
-            value = current_value.token().ident().to_lowercase_string();
-        } else if (current_value.is(Token::Type::Delim) && current_value.token().delim() == "*") {
-            type = Selector::SimpleSelector::Type::Universal;
+            simple_selector.type = Selector::SimpleSelector::Type::Class;
+            simple_selector.value = current_value.token().ident().to_lowercase_string();
         } else if (current_value.is(Token::Type::Ident)) {
-            type = Selector::SimpleSelector::Type::TagName;
-            value = current_value.token().ident().to_lowercase_string();
-        } else if ((current_value.is(Token::Type::Delim) && current_value.token().delim() == ":")
-            || (current_value.is_block() && current_value.block().is_square())) {
+            simple_selector.type = Selector::SimpleSelector::Type::TagName;
+            simple_selector.value = current_value.token().ident().to_lowercase_string();
+        } else if ((current_value.is(Token::Type::Delim) && current_value.token().delim() == ":")) {
             // FIXME: This is a temporary hack until we make the Selector::SimpleSelector::Type changes.
-            type = Selector::SimpleSelector::Type::Universal;
+            simple_selector.type = Selector::SimpleSelector::Type::Universal;
             tokens.reconsume_current_input_token();
-        } else {
-            dbgln("Invalid simple selector!");
-            return {};
-        }
-
-        Selector::SimpleSelector simple_selector;
-        simple_selector.type = type;
-        simple_selector.value = value;
-
-        current_value = tokens.next_token();
-        if (check_for_eof_or_whitespace(current_value))
-            return simple_selector;
+        } else if (current_value.is_block() && current_value.block().is_square()) {
+            simple_selector.type = Selector::SimpleSelector::Type::Attribute;
 
-        // FIXME: Attribute selectors want to be their own Selector::SimpleSelector::Type according to the spec.
-        if (current_value.is_block() && current_value.block().is_square()) {
+            auto& attribute = simple_selector.attribute;
 
             Vector<StyleComponentValueRule> const& attribute_parts = current_value.block().values();
 
@@ -315,8 +293,8 @@ Optional<Selector> Parser::parse_single_selector(TokenStream<T>& tokens, bool is
                 return {};
             }
 
-            simple_selector.attribute_match_type = Selector::SimpleSelector::AttributeMatchType::HasAttribute;
-            simple_selector.attribute_name = attribute_part.token().ident();
+            attribute.match_type = Selector::SimpleSelector::Attribute::MatchType::HasAttribute;
+            attribute.name = attribute_part.token().ident();
 
             if (attribute_parts.size() == 1)
                 return simple_selector;
@@ -329,7 +307,7 @@ Optional<Selector> Parser::parse_single_selector(TokenStream<T>& tokens, bool is
             }
 
             if (delim_part.token().delim() == "=") {
-                simple_selector.attribute_match_type = Selector::SimpleSelector::AttributeMatchType::ExactValueMatch;
+                attribute.match_type = Selector::SimpleSelector::Attribute::MatchType::ExactValueMatch;
                 attribute_index++;
             } else {
                 attribute_index++;
@@ -345,19 +323,19 @@ Optional<Selector> Parser::parse_single_selector(TokenStream<T>& tokens, bool is
                 }
 
                 if (delim_part.token().delim() == "~") {
-                    simple_selector.attribute_match_type = Selector::SimpleSelector::AttributeMatchType::ContainsWord;
+                    attribute.match_type = Selector::SimpleSelector::Attribute::MatchType::ContainsWord;
                     attribute_index++;
                 } else if (delim_part.token().delim() == "*") {
-                    simple_selector.attribute_match_type = Selector::SimpleSelector::AttributeMatchType::ContainsString;
+                    attribute.match_type = Selector::SimpleSelector::Attribute::MatchType::ContainsString;
                     attribute_index++;
                 } else if (delim_part.token().delim() == "|") {
-                    simple_selector.attribute_match_type = Selector::SimpleSelector::AttributeMatchType::StartsWithSegment;
+                    attribute.match_type = Selector::SimpleSelector::Attribute::MatchType::StartsWithSegment;
                     attribute_index++;
                 } else if (delim_part.token().delim() == "^") {
-                    simple_selector.attribute_match_type = Selector::SimpleSelector::AttributeMatchType::StartsWithString;
+                    attribute.match_type = Selector::SimpleSelector::Attribute::MatchType::StartsWithString;
                     attribute_index++;
                 } else if (delim_part.token().delim() == "$") {
-                    simple_selector.attribute_match_type = Selector::SimpleSelector::AttributeMatchType::EndsWithString;
+                    attribute.match_type = Selector::SimpleSelector::Attribute::MatchType::EndsWithString;
                     attribute_index++;
                 }
             }
@@ -372,12 +350,18 @@ Optional<Selector> Parser::parse_single_selector(TokenStream<T>& tokens, bool is
                 dbgln("Expected a string or ident for the value to match attribute against, got: '{}'", value_part.to_debug_string());
                 return {};
             }
-            simple_selector.attribute_value = value_part.token().is(Token::Type::Ident) ? value_part.token().ident() : value_part.token().string();
+            attribute.value = value_part.token().is(Token::Type::Ident) ? value_part.token().ident() : value_part.token().string();
 
             // FIXME: Handle case-sensitivity suffixes. https://www.w3.org/TR/selectors-4/#attribute-case
-            return simple_selector;
+        } else {
+            dbgln("Invalid simple selector!");
+            return {};
         }
 
+        current_value = tokens.next_token();
+        if (check_for_eof_or_whitespace(current_value))
+            return simple_selector;
+
         // FIXME: Pseudo-class selectors want to be their own Selector::SimpleSelector::Type according to the spec.
         if (current_value.is(Token::Type::Colon)) {
             bool is_pseudo = false;

Userland/Libraries/LibWeb/CSS/Selector.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, Sam Atkins <atkinssj@gmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -21,6 +22,7 @@ public:
             TagName,
             Id,
             Class,
+            Attribute,
         };
         Type type { Type::Invalid };
 
@@ -56,20 +58,22 @@ public:
 
         FlyString value;
 
-        enum class AttributeMatchType {
-            None,
-            HasAttribute,
-            ExactValueMatch,
-            ContainsWord,      // [att~=val]
-            ContainsString,    // [att*=val]
-            StartsWithSegment, // [att|=val]
-            StartsWithString,  // [att^=val]
-            EndsWithString,    // [att$=val]
+        struct Attribute {
+            enum class MatchType {
+                None,
+                HasAttribute,
+                ExactValueMatch,
+                ContainsWord,      // [att~=val]
+                ContainsString,    // [att*=val]
+                StartsWithSegment, // [att|=val]
+                StartsWithString,  // [att^=val]
+                EndsWithString,    // [att$=val]
+            };
+            MatchType match_type { MatchType::None };
+            FlyString name;
+            String value;
         };
-
-        AttributeMatchType attribute_match_type { AttributeMatchType::None };
-        FlyString attribute_name;
-        String attribute_value;
+        Attribute attribute;
 
         struct NthChildPattern {
             int step_size = 0;

Userland/Libraries/LibWeb/CSS/SelectorEngine.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, Sam Atkins <atkinssj@gmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -24,6 +25,38 @@ static bool matches_hover_pseudo_class(DOM::Element const& element)
     return element.is_ancestor_of(*hovered_node);
 }
 
+static bool matches_attribute(CSS::Selector::SimpleSelector::Attribute const& attribute, DOM::Element const& element)
+{
+    switch (attribute.match_type) {
+    case CSS::Selector::SimpleSelector::Attribute::MatchType::HasAttribute:
+        return element.has_attribute(attribute.name);
+        break;
+    case CSS::Selector::SimpleSelector::Attribute::MatchType::ExactValueMatch:
+        return element.attribute(attribute.name) == attribute.value;
+        break;
+    case CSS::Selector::SimpleSelector::Attribute::MatchType::ContainsWord:
+        return element.attribute(attribute.name).split(' ').contains_slow(attribute.value);
+        break;
+    case CSS::Selector::SimpleSelector::Attribute::MatchType::ContainsString:
+        return element.attribute(attribute.name).contains(attribute.value);
+        break;
+    case CSS::Selector::SimpleSelector::Attribute::MatchType::StartsWithSegment:
+        return element.attribute(attribute.name).split('-').first() == attribute.value;
+        break;
+    case CSS::Selector::SimpleSelector::Attribute::MatchType::StartsWithString:
+        return element.attribute(attribute.name).starts_with(attribute.value);
+        break;
+    case CSS::Selector::SimpleSelector::Attribute::MatchType::EndsWithString:
+        return element.attribute(attribute.name).ends_with(attribute.value);
+        break;
+    case CSS::Selector::SimpleSelector::Attribute::MatchType::None:
+        VERIFY_NOT_REACHED();
+        break;
+    }
+
+    return false;
+}
+
 static bool matches(CSS::Selector::SimpleSelector const& component, DOM::Element const& element)
 {
     switch (component.pseudo_element) {
@@ -171,39 +204,6 @@ static bool matches(CSS::Selector::SimpleSelector const& component, DOM::Element
         break;
     }
 
-    switch (component.attribute_match_type) {
-    case CSS::Selector::SimpleSelector::AttributeMatchType::HasAttribute:
-        if (!element.has_attribute(component.attribute_name))
-            return false;
-        break;
-    case CSS::Selector::SimpleSelector::AttributeMatchType::ExactValueMatch:
-        if (element.attribute(component.attribute_name) != component.attribute_value)
-            return false;
-        break;
-    case CSS::Selector::SimpleSelector::AttributeMatchType::ContainsWord:
-        if (!element.attribute(component.attribute_name).split(' ').contains_slow(component.attribute_value))
-            return false;
-        break;
-    case CSS::Selector::SimpleSelector::AttributeMatchType::ContainsString:
-        if (!element.attribute(component.attribute_name).contains(component.attribute_value))
-            return false;
-        break;
-    case CSS::Selector::SimpleSelector::AttributeMatchType::StartsWithSegment:
-        if (element.attribute(component.attribute_name).split('-').first() != component.attribute_value)
-            return false;
-        break;
-    case CSS::Selector::SimpleSelector::AttributeMatchType::StartsWithString:
-        if (!element.attribute(component.attribute_name).starts_with(component.attribute_value))
-            return false;
-        break;
-    case CSS::Selector::SimpleSelector::AttributeMatchType::EndsWithString:
-        if (!element.attribute(component.attribute_name).ends_with(component.attribute_value))
-            return false;
-        break;
-    default:
-        break;
-    }
-
     switch (component.type) {
     case CSS::Selector::SimpleSelector::Type::Universal:
         return true;
@@ -213,6 +213,8 @@ static bool matches(CSS::Selector::SimpleSelector const& component, DOM::Element
         return element.has_class(component.value);
     case CSS::Selector::SimpleSelector::Type::TagName:
         return component.value == element.local_name();
+    case CSS::Selector::SimpleSelector::Type::Attribute:
+        return matches_attribute(component.attribute, element);
     default:
         VERIFY_NOT_REACHED();
     }

Userland/Libraries/LibWeb/Dump.cpp

@@ -314,31 +314,8 @@ void dump_selector(StringBuilder& builder, CSS::Selector const& selector)
             case CSS::Selector::SimpleSelector::Type::TagName:
                 type_description = "TagName";
                 break;
-            }
-            const char* attribute_match_type_description = "";
-            switch (simple_selector.attribute_match_type) {
-            case CSS::Selector::SimpleSelector::AttributeMatchType::None:
-                break;
-            case CSS::Selector::SimpleSelector::AttributeMatchType::HasAttribute:
-                attribute_match_type_description = "HasAttribute";
-                break;
-            case CSS::Selector::SimpleSelector::AttributeMatchType::ExactValueMatch:
-                attribute_match_type_description = "ExactValueMatch";
-                break;
-            case CSS::Selector::SimpleSelector::AttributeMatchType::ContainsWord:
-                attribute_match_type_description = "ContainsWord";
-                break;
-            case CSS::Selector::SimpleSelector::AttributeMatchType::ContainsString:
-                attribute_match_type_description = "ContainsString";
-                break;
-            case CSS::Selector::SimpleSelector::AttributeMatchType::StartsWithSegment:
-                attribute_match_type_description = "StartsWithSegment";
-                break;
-            case CSS::Selector::SimpleSelector::AttributeMatchType::StartsWithString:
-                attribute_match_type_description = "StartsWithString";
-                break;
-            case CSS::Selector::SimpleSelector::AttributeMatchType::EndsWithString:
-                attribute_match_type_description = "EndsWithString";
+            case CSS::Selector::SimpleSelector::Type::Attribute:
+                type_description = "Attribute";
                 break;
             }
 
@@ -406,8 +383,38 @@ void dump_selector(StringBuilder& builder, CSS::Selector const& selector)
             builder.appendff("{}:{}", type_description, simple_selector.value);
             if (simple_selector.pseudo_class != CSS::Selector::SimpleSelector::PseudoClass::None)
                 builder.appendff(" pseudo_class={}", pseudo_class_description);
-            if (simple_selector.attribute_match_type != CSS::Selector::SimpleSelector::AttributeMatchType::None) {
-                builder.appendff(" [{}, name='{}', value='{}']", attribute_match_type_description, simple_selector.attribute_name, simple_selector.attribute_value);
+
+            if (simple_selector.type == CSS::Selector::SimpleSelector::Type::Attribute) {
+                char const* attribute_match_type_description = "";
+
+                switch (simple_selector.attribute.match_type) {
+                case CSS::Selector::SimpleSelector::Attribute::MatchType::None:
+                    break;
+                case CSS::Selector::SimpleSelector::Attribute::MatchType::HasAttribute:
+                    type_description = "HasAttribute";
+                    break;
+                case CSS::Selector::SimpleSelector::Attribute::MatchType::ExactValueMatch:
+                    type_description = "ExactValueMatch";
+                    break;
+                case CSS::Selector::SimpleSelector::Attribute::MatchType::ContainsWord:
+                    type_description = "ContainsWord";
+                    break;
+                case CSS::Selector::SimpleSelector::Attribute::MatchType::ContainsString:
+                    type_description = "ContainsString";
+                    break;
+                case CSS::Selector::SimpleSelector::Attribute::MatchType::StartsWithSegment:
+                    type_description = "StartsWithSegment";
+                    break;
+                case CSS::Selector::SimpleSelector::Attribute::MatchType::StartsWithString:
+                    type_description = "StartsWithString";
+                    break;
+                case CSS::Selector::SimpleSelector::Attribute::MatchType::EndsWithString:
+                    type_description = "EndsWithString";
+                    break;
+                }
+                break;
+
+                builder.appendff(" [{}, name='{}', value='{}']", attribute_match_type_description, simple_selector.attribute.name, simple_selector.attribute.value);
             }
 
             if (i != complex_selector.compound_selector.size() - 1)