LibWeb/CSS: Split out <family-name> parsing

This type is used individually elsewhere in the spec. This also lets us
separate out the `<generic-family>` type.

Libraries/LibWeb/CSS/Parser/Parser.h

@@ -379,6 +379,7 @@ private:
     RefPtr<CSSStyleValue> parse_flex_shorthand_value(TokenStream<ComponentValue>&);
     RefPtr<CSSStyleValue> parse_flex_flow_value(TokenStream<ComponentValue>&);
     RefPtr<CSSStyleValue> parse_font_value(TokenStream<ComponentValue>&);
+    RefPtr<CSSStyleValue> parse_family_name_value(TokenStream<ComponentValue>&);
     RefPtr<CSSStyleValue> parse_font_family_value(TokenStream<ComponentValue>&);
     RefPtr<CSSStyleValue> parse_font_language_override_value(TokenStream<ComponentValue>&);
     RefPtr<CSSStyleValue> parse_font_feature_settings_value(TokenStream<ComponentValue>&);

Libraries/LibWeb/CSS/Parser/PropertyParsing.cpp

@@ -2490,81 +2490,37 @@ RefPtr<CSSStyleValue> Parser::parse_font_value(TokenStream<ComponentValue>& toke
         });
 }
 
+// https://drafts.csswg.org/css-fonts-4/#font-family-prop
 RefPtr<CSSStyleValue> Parser::parse_font_family_value(TokenStream<ComponentValue>& tokens)
 {
-    auto next_is_comma_or_eof = [&]() -> bool {
+    // [ <family-name> | <generic-family> ]#
-        return !tokens.has_next_token() || tokens.next_token().is(Token::Type::Comma);
+    // FIXME: We currently require font-family to always be a list, even with one item.
-    };
+    //        Maybe change that?
+    auto result = parse_comma_separated_value_list(tokens, [this](auto& inner_tokens) -> RefPtr<CSSStyleValue> {
+        inner_tokens.discard_whitespace();
 
-    // Note: Font-family names can either be a quoted string, or a keyword, or a series of custom-idents.
+        // <generic-family>
-    // eg, these are equivalent:
+        if (inner_tokens.next_token().is(Token::Type::Ident)) {
-    //     font-family: my cool     font\!, serif;
+            auto maybe_keyword = keyword_from_string(inner_tokens.next_token().token().ident());
-    //     font-family: "my cool font!", serif;
-    StyleValueVector font_families;
-    Vector<String> current_name_parts;
-    while (tokens.has_next_token()) {
-        auto const& peek = tokens.next_token();
-
-        if (peek.is(Token::Type::String)) {
-            // `font-family: my cool "font";` is invalid.
-            if (!current_name_parts.is_empty())
-                return nullptr;
-            tokens.discard_a_token(); // String
-            if (!next_is_comma_or_eof())
-                return nullptr;
-            font_families.append(StringStyleValue::create(peek.token().string()));
-            tokens.discard_a_token(); // Comma
-            continue;
-        }
-
-        if (peek.is(Token::Type::Ident)) {
-            // If this is a valid identifier, it's NOT a custom-ident and can't be part of a larger name.
-
-            // CSS-wide keywords are not allowed
-            if (auto builtin = parse_builtin_value(tokens))
-                return nullptr;
-
-            auto maybe_keyword = keyword_from_string(peek.token().ident());
-            // Can't have a generic-font-name as a token in an unquoted font name.
             if (maybe_keyword.has_value() && keyword_to_generic_font_family(maybe_keyword.value()).has_value()) {
-                if (!current_name_parts.is_empty())
+                inner_tokens.discard_a_token(); // Ident
-                    return nullptr;
+                inner_tokens.discard_whitespace();
-                tokens.discard_a_token(); // Ident
+                return CSSKeywordValue::create(maybe_keyword.value());
-                if (!next_is_comma_or_eof())
-                    return nullptr;
-                font_families.append(CSSKeywordValue::create(maybe_keyword.value()));
-                tokens.discard_a_token(); // Comma
-                continue;
             }
-            current_name_parts.append(tokens.consume_a_token().token().ident().to_string());
-            continue;
         }
 
-        if (peek.is(Token::Type::Comma)) {
+        // <family-name>
-            if (current_name_parts.is_empty())
+        return parse_family_name_value(inner_tokens);
-                return nullptr;
+    });
-            tokens.discard_a_token(); // Comma
-            // This is really a series of custom-idents, not just one. But for the sake of simplicity we'll make it one.
-            font_families.append(CustomIdentStyleValue::create(MUST(String::join(' ', current_name_parts))));
-            current_name_parts.clear();
-            // Can't have a trailing comma
-            if (!tokens.has_next_token())
-                return nullptr;
-            continue;
-        }
 
+    if (!result)
         return nullptr;
-    }
 
-    if (!current_name_parts.is_empty()) {
+    if (result->is_value_list())
-        // This is really a series of custom-idents, not just one. But for the sake of simplicity we'll make it one.
+        return result.release_nonnull();
-        font_families.append(CustomIdentStyleValue::create(MUST(String::join(' ', current_name_parts))));
-        current_name_parts.clear();
-    }
 
-    if (font_families.is_empty())
+    // It's a single value, so wrap it in a list - see FIXME above.
-        return nullptr;
+    return StyleValueList::create(StyleValueVector { result.release_nonnull() }, StyleValueList::Separator::Comma);
-    return StyleValueList::create(move(font_families), StyleValueList::Separator::Comma);
 }
 
 RefPtr<CSSStyleValue> Parser::parse_font_language_override_value(TokenStream<ComponentValue>& tokens)

Libraries/LibWeb/CSS/Parser/ValueParsing.cpp

@@ -311,6 +311,55 @@ Optional<Ratio> Parser::parse_ratio(TokenStream<ComponentValue>& tokens)
     return Ratio { numerator };
 }
 
+// https://drafts.csswg.org/css-fonts-4/#family-name-syntax
+RefPtr<CSSStyleValue> Parser::parse_family_name_value(TokenStream<ComponentValue>& tokens)
+{
+    auto transaction = tokens.begin_transaction();
+    tokens.discard_whitespace();
+
+    // <family-name> = <string> | <custom-ident>+
+    Vector<String> parts;
+    while (tokens.has_next_token()) {
+        auto const& peek = tokens.next_token();
+
+        if (peek.is(Token::Type::String)) {
+            // `font-family: my cool "font";` is invalid.
+            if (!parts.is_empty())
+                return nullptr;
+            tokens.discard_a_token(); // String
+            tokens.discard_whitespace();
+            transaction.commit();
+            return StringStyleValue::create(peek.token().string());
+        }
+
+        if (peek.is(Token::Type::Ident)) {
+            auto ident = tokens.consume_a_token().token().ident();
+
+            // CSS-wide keywords are not allowed
+            if (is_css_wide_keyword(ident))
+                return nullptr;
+
+            // <generic-family> is a separate type from <family-name>, and so isn't allowed here.
+            auto maybe_keyword = keyword_from_string(ident);
+            if (maybe_keyword.has_value() && keyword_to_generic_font_family(maybe_keyword.value()).has_value()) {
+                return nullptr;
+            }
+
+            parts.append(ident.to_string());
+            tokens.discard_whitespace();
+            continue;
+        }
+
+        break;
+    }
+
+    if (parts.is_empty())
+        return nullptr;
+
+    transaction.commit();
+    return CustomIdentStyleValue::create(MUST(String::join(' ', parts)));
+}
+
 // https://www.w3.org/TR/css-syntax-3/#urange-syntax
 Optional<Gfx::UnicodeRange> Parser::parse_unicode_range(TokenStream<ComponentValue>& tokens)
 {