From 95bd0602ba079e0bd7608e39bcbc3b0403ed89d1 Mon Sep 17 00:00:00 2001
From: Andreas Kling <andreas@ladybird.org>
Date: Sat, 7 Sep 2024 11:07:04 +0200
Subject: [PATCH] LibWeb: Keep custom properties from all cascade layers

Before this change, we were cascading custom properties for each layer,
and then replacing any previously cascaded properties for the element
with only the set from this latest layer.

The patch fixes the issue by making each pass of the custom property
cascade add to the same set, and then finally assigning that set of
properties to the element.
---
 .../css/custom-properties-from-all-layers.txt |  4 ++++
 .../custom-properties-from-all-layers.html    | 23 +++++++++++++++++++
 .../Libraries/LibWeb/CSS/StyleComputer.cpp    | 12 +++++-----
 3 files changed, 33 insertions(+), 6 deletions(-)
 create mode 100644 Tests/LibWeb/Text/expected/css/custom-properties-from-all-layers.txt
 create mode 100644 Tests/LibWeb/Text/input/css/custom-properties-from-all-layers.html

diff --git a/Tests/LibWeb/Text/expected/css/custom-properties-from-all-layers.txt b/Tests/LibWeb/Text/expected/css/custom-properties-from-all-layers.txt
new file mode 100644
index 00000000000..8b6f94330d6
--- /dev/null
+++ b/Tests/LibWeb/Text/expected/css/custom-properties-from-all-layers.txt
@@ -0,0 +1,4 @@
+   1px
+2px
+3px
+4px
diff --git a/Tests/LibWeb/Text/input/css/custom-properties-from-all-layers.html b/Tests/LibWeb/Text/input/css/custom-properties-from-all-layers.html
new file mode 100644
index 00000000000..c8251a21e0c
--- /dev/null
+++ b/Tests/LibWeb/Text/input/css/custom-properties-from-all-layers.html
@@ -0,0 +1,23 @@
+<style>
+:root { --top: 1px; }
+@layer foo { :root { --left: 2px; } }
+@layer bar { :root { --bottom: 3px; } }
+@layer baz { :root { --right: 4px; } }
+#testDiv {
+    padding-top: var(--top, 0);
+    padding-left: var(--left, 0);
+    padding-bottom: var(--bottom, 0);
+    padding-right: var(--right, 0);
+}
+</style><div id="testDiv"></div>
+<script src="../include.js"></script>
+<script>
+    test(() => {
+        const s = getComputedStyle(testDiv);
+        println(s.paddingTop);
+        println(s.paddingLeft);
+        println(s.paddingBottom);
+        println(s.paddingRight);
+        testDiv.remove();
+    });
+</script>
diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp
index 83d27dee5df..543aeb80c3b 100644
--- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp
+++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp
@@ -961,7 +961,7 @@ void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& e
     }
 }
 
-static void cascade_custom_properties(DOM::Element& element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element, Vector<MatchingRule> const& matching_rules)
+static void cascade_custom_properties(DOM::Element& element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element, Vector<MatchingRule> const& matching_rules, HashMap<FlyString, StyleProperty>& custom_properties)
 {
     size_t needed_capacity = 0;
     for (auto const& matching_rule : matching_rules)
@@ -972,8 +972,7 @@ static void cascade_custom_properties(DOM::Element& element, Optional<CSS::Selec
             needed_capacity += inline_style->custom_properties().size();
     }
 
-    HashMap<FlyString, StyleProperty> custom_properties;
-    custom_properties.ensure_capacity(needed_capacity);
+    custom_properties.ensure_capacity(custom_properties.size() + needed_capacity);
 
     for (auto const& matching_rule : matching_rules) {
         for (auto const& it : matching_rule.rule->declaration().custom_properties())
@@ -986,8 +985,6 @@ static void cascade_custom_properties(DOM::Element& element, Optional<CSS::Selec
                 custom_properties.set(it.key, it.value);
         }
     }
-
-    element.set_custom_properties(pseudo_element, move(custom_properties));
 }
 
 static NonnullRefPtr<CSSStyleValue const> interpolate_value(DOM::Element& element, CSSStyleValue const& from, CSSStyleValue const& to, float delta);
@@ -1778,9 +1775,12 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element
 
     // Then we resolve all the CSS custom properties ("variables") for this element:
     // FIXME: Also resolve !important custom properties, in a second cascade.
+
+    HashMap<FlyString, CSS::StyleProperty> custom_properties;
     for (auto& layer : matching_rule_set.author_rules) {
-        cascade_custom_properties(element, pseudo_element, layer.rules);
+        cascade_custom_properties(element, pseudo_element, layer.rules, custom_properties);
     }
+    element.set_custom_properties(pseudo_element, move(custom_properties));
 
     // Then we apply the declarations from the matched rules in cascade order: