From 9735f474a224177ef0a3ecd0ea5e88e89ca9d8a1 Mon Sep 17 00:00:00 2001
From: Sam Atkins <atkinssj@serenityos.org>
Date: Mon, 4 Sep 2023 11:34:22 +0100
Subject: [PATCH] LibWeb: Extract attr() substitution into its own method

No functional changes.
---
 .../Libraries/LibWeb/CSS/Parser/Parser.cpp    | 73 ++++++++++---------
 Userland/Libraries/LibWeb/CSS/Parser/Parser.h |  1 +
 2 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
index aeeb719822..22c4634c43 100644
--- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
+++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp
@@ -6670,40 +6670,9 @@ bool Parser::expand_unresolved_values(DOM::Element& element, StringView property
         auto const& value = source.next_token();
         if (value.is_function()) {
             if (value.function().name().equals_ignoring_ascii_case("attr"sv)) {
-                // https://drafts.csswg.org/css-values-5/#attr-substitution
-                TokenStream attr_contents { value.function().values() };
-                attr_contents.skip_whitespace();
-                if (!attr_contents.has_next_token())
+                if (!substitute_attr_function(element, property_name, value.function(), dest))
                     return false;
-
-                auto const& attr_name_token = attr_contents.next_token();
-                if (!attr_name_token.is(Token::Type::Ident))
-                    return false;
-                auto attr_name = attr_name_token.token().ident();
-
-                auto attr_value = element.get_attribute(attr_name);
-                // 1. If the attr() function has a substitution value, replace the attr() function by the substitution value.
-                if (!attr_value.is_null()) {
-                    // FIXME: attr() should also accept an optional type argument, not just strings.
-                    dest.empend(Token::of_string(FlyString::from_deprecated_fly_string(attr_value).release_value_but_fixme_should_propagate_errors()));
-                    continue;
-                }
-
-                // 2. Otherwise, if the attr() function has a fallback value as its last argument, replace the attr() function by the fallback value.
-                //    If there are any var() or attr() references in the fallback, substitute them as well.
-                attr_contents.skip_whitespace();
-                if (attr_contents.has_next_token()) {
-                    auto const& comma_token = attr_contents.next_token();
-                    if (!comma_token.is(Token::Type::Comma))
-                        return false;
-                    attr_contents.skip_whitespace();
-                    if (!expand_unresolved_values(element, property_name, attr_contents, dest))
-                        return false;
-                    continue;
-                }
-
-                // 3. Otherwise, the property containing the attr() function is invalid at computed-value time.
-                return false;
+                continue;
             }
 
             if (auto maybe_calc_value = parse_calculated_value(value); maybe_calc_value && maybe_calc_value->is_calculated()) {
@@ -6765,4 +6734,42 @@ bool Parser::expand_unresolved_values(DOM::Element& element, StringView property
     return true;
 }
 
+// https://drafts.csswg.org/css-values-5/#attr-substitution
+bool Parser::substitute_attr_function(DOM::Element& element, StringView property_name, Function const& attr_function, Vector<ComponentValue>& dest)
+{
+    TokenStream attr_contents { attr_function.values() };
+    attr_contents.skip_whitespace();
+    if (!attr_contents.has_next_token())
+        return false;
+
+    auto const& attr_name_token = attr_contents.next_token();
+    if (!attr_name_token.is(Token::Type::Ident))
+        return false;
+    auto attr_name = attr_name_token.token().ident();
+
+    auto attr_value = element.get_attribute(attr_name);
+    // 1. If the attr() function has a substitution value, replace the attr() function by the substitution value.
+    if (!attr_value.is_null()) {
+        // FIXME: attr() should also accept an optional type argument, not just strings.
+        dest.empend(Token::of_string(FlyString::from_deprecated_fly_string(attr_value).release_value_but_fixme_should_propagate_errors()));
+        return true;
+    }
+
+    // 2. Otherwise, if the attr() function has a fallback value as its last argument, replace the attr() function by the fallback value.
+    //    If there are any var() or attr() references in the fallback, substitute them as well.
+    attr_contents.skip_whitespace();
+    if (attr_contents.has_next_token()) {
+        auto const& comma_token = attr_contents.next_token();
+        if (!comma_token.is(Token::Type::Comma))
+            return false;
+        attr_contents.skip_whitespace();
+        if (!expand_unresolved_values(element, property_name, attr_contents, dest))
+            return false;
+        return true;
+    }
+
+    // 3. Otherwise, the property containing the attr() function is invalid at computed-value time.
+    return false;
+}
+
 }
diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.h b/Userland/Libraries/LibWeb/CSS/Parser/Parser.h
index ca18d197bf..06688f9a29 100644
--- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.h
+++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.h
@@ -289,6 +289,7 @@ private:
     NonnullRefPtr<StyleValue> resolve_unresolved_style_value(DOM::Element&, Optional<Selector::PseudoElement>, PropertyID, UnresolvedStyleValue const&);
     bool expand_variables(DOM::Element&, Optional<Selector::PseudoElement>, StringView property_name, HashMap<FlyString, NonnullRefPtr<PropertyDependencyNode>>& dependencies, TokenStream<ComponentValue>& source, Vector<ComponentValue>& dest);
     bool expand_unresolved_values(DOM::Element&, StringView property_name, TokenStream<ComponentValue>& source, Vector<ComponentValue>& dest);
+    bool substitute_attr_function(DOM::Element& element, StringView property_name, Function const& attr_function, Vector<ComponentValue>& dest);
 
     static bool has_ignored_vendor_prefix(StringView);