LibWeb: Fix underinvalidation when inline style has custom properties

We have an optimization that allows us to invalidate only the style of
the element itself and mark descendants for inherited properties update
when the "style" attribute changes (unless there are any CSS rules that
use the "style" attribute, then we also invalidate all descendants that
might be affected by those rules). This optimization was not taking into
account that when the inline style has custom properties, we also need
to invalidate all descendants whose style might be affected by them.

This change fixes this bug by saving a flag in Element that indicates
whether its style depends on any custom properties and then invalidating
all descendants with this flag set when the "style" attribute changes.
Unlike font relative lengths invalidation, for elements that depend on
custom properties, we need to actually recompute the style, instead of
individual properties, because values without expanded custom properties
are gone after cascading, and it has to be done again.

The test added for this change is a version of an existing test we had
restructured such that it doesn't trigger aggressive style invalidation
caused by DOM structured changes until the last moment when test results
are printed.

Libraries/LibWeb/CSS/Parser/Parser.cpp

@@ -9569,6 +9569,14 @@ NonnullRefPtr<CSSStyleValue> Parser::resolve_unresolved_style_value(DOM::Element
     Vector<ComponentValue> values_with_variables_expanded;
 
     HashMap<FlyString, NonnullRefPtr<PropertyDependencyNode>> dependencies;
+    ScopeGuard mark_element_if_uses_custom_properties = [&] {
+        for (auto const& name : dependencies.keys()) {
+            if (is_a_custom_property_name_string(name)) {
+                element.set_style_uses_css_custom_properties(true);
+                return;
+            }
+        }
+    };
     if (!expand_variables(element, pseudo_element, string_from_property_id(property_id), dependencies, unresolved_values_without_variables_expanded, values_with_variables_expanded))
         return CSSKeywordValue::create(Keyword::Unset);
 

Libraries/LibWeb/DOM/Element.cpp

@@ -1986,13 +1986,14 @@ ErrorOr<void> Element::scroll_into_view(Optional<Variant<bool, ScrollIntoViewOpt
 void Element::invalidate_style_after_attribute_change(FlyString const& attribute_name, Optional<String> const& old_value, Optional<String> const& new_value)
 {
     Vector<CSS::InvalidationSet::Property, 1> changed_properties;
-    ForceSelfStyleInvalidation force_self_invalidation = ForceSelfStyleInvalidation::No;
+    StyleInvalidationOptions style_invalidation_options;
     if (is_presentational_hint(attribute_name)) {
-        force_self_invalidation = ForceSelfStyleInvalidation::Yes;
+        style_invalidation_options.invalidate_self = true;
     }
 
     if (attribute_name == HTML::AttributeNames::style) {
-        force_self_invalidation = ForceSelfStyleInvalidation::Yes;
+        style_invalidation_options.invalidate_self = true;
+        style_invalidation_options.invalidate_elements_that_use_css_custom_properties = true;
     } else if (attribute_name == HTML::AttributeNames::class_) {
         Vector<StringView> old_classes;
         Vector<StringView> new_classes;
@@ -2025,7 +2026,7 @@ void Element::invalidate_style_after_attribute_change(FlyString const& attribute
     }
 
     changed_properties.append({ .type = CSS::InvalidationSet::Property::Type::Attribute, .value = attribute_name });
-    invalidate_style(StyleInvalidationReason::ElementAttributeChange, changed_properties, force_self_invalidation);
+    invalidate_style(StyleInvalidationReason::ElementAttributeChange, changed_properties, style_invalidation_options);
 }
 
 bool Element::is_hidden() const

Libraries/LibWeb/DOM/Element.h

@@ -245,6 +245,9 @@ public:
     void set_custom_properties(Optional<CSS::Selector::PseudoElement::Type>, HashMap<FlyString, CSS::StyleProperty> custom_properties);
     [[nodiscard]] HashMap<FlyString, CSS::StyleProperty> const& custom_properties(Optional<CSS::Selector::PseudoElement::Type>) const;
 
+    bool style_uses_css_custom_properties() const { return m_style_uses_css_custom_properties; }
+    void set_style_uses_css_custom_properties(bool value) { m_style_uses_css_custom_properties = value; }
+
     // NOTE: The function is wrapped in a GC::HeapFunction immediately.
     HTML::TaskID queue_an_element_task(HTML::Task::Source, Function<void()>);
 
@@ -496,6 +499,7 @@ private:
     Array<CSSPixelPoint, 3> m_scroll_offset;
 
     bool m_in_top_layer { false };
+    bool m_style_uses_css_custom_properties { false };
 
     OwnPtr<CSS::CountersSet> m_counters_set;
 

Libraries/LibWeb/DOM/Node.cpp

@@ -462,7 +462,7 @@ void Node::invalidate_style(StyleInvalidationReason reason)
     document().schedule_style_update();
 }
 
-void Node::invalidate_style(StyleInvalidationReason reason, Vector<CSS::InvalidationSet::Property> const& properties, ForceSelfStyleInvalidation force_self_invalidation)
+void Node::invalidate_style(StyleInvalidationReason reason, Vector<CSS::InvalidationSet::Property> const& properties, StyleInvalidationOptions options)
 {
     if (is_character_data())
         return;
@@ -477,7 +477,7 @@ void Node::invalidate_style(StyleInvalidationReason reason, Vector<CSS::Invalida
     }
 
     auto invalidation_set = document().style_computer().invalidation_set_for_properties(properties);
-    if (force_self_invalidation == ForceSelfStyleInvalidation::Yes)
+    if (options.invalidate_self)
         invalidation_set.set_needs_invalidate_self();
     if (invalidation_set.is_empty())
         return;
@@ -500,7 +500,14 @@ void Node::invalidate_style(StyleInvalidationReason reason, Vector<CSS::Invalida
             if (!node.is_element())
                 return TraversalDecision::Continue;
             auto& element = static_cast<Element&>(node);
-            bool needs_style_recalculation = invalidation_set.needs_invalidate_whole_subtree() || element_has_properties_from_invalidation_set(element);
+            bool needs_style_recalculation = false;
+            if (invalidation_set.needs_invalidate_whole_subtree()) {
+                needs_style_recalculation = true;
+            } else if (element_has_properties_from_invalidation_set(element)) {
+                needs_style_recalculation = true;
+            } else if (options.invalidate_elements_that_use_css_custom_properties && element.style_uses_css_custom_properties()) {
+                needs_style_recalculation = true;
+            }
             if (needs_style_recalculation) {
                 element.set_needs_style_update(true);
             } else {

Libraries/LibWeb/DOM/Node.h

@@ -301,12 +301,12 @@ public:
     [[nodiscard]] bool entire_subtree_needs_style_update() const { return m_entire_subtree_needs_style_update; }
     void set_entire_subtree_needs_style_update(bool b) { m_entire_subtree_needs_style_update = b; }
 
-    enum class ForceSelfStyleInvalidation : bool {
-        Yes,
-        No
-    };
     void invalidate_style(StyleInvalidationReason);
-    void invalidate_style(StyleInvalidationReason, Vector<CSS::InvalidationSet::Property> const&, ForceSelfStyleInvalidation = ForceSelfStyleInvalidation::No);
+    struct StyleInvalidationOptions {
+        bool invalidate_self { false };
+        bool invalidate_elements_that_use_css_custom_properties { false };
+    };
+    void invalidate_style(StyleInvalidationReason, Vector<CSS::InvalidationSet::Property> const&, StyleInvalidationOptions);
 
     void set_document(Badge<Document>, Document&);
 

Tests/LibWeb/Text/expected/css/CSSStyleDeclaration-custom-properties-2.txt

@@ -0,0 +1,4 @@
+style.getPropertyValue(--redcolor)=red
+style.getPropertyPriority(--redcolor)=
+rgb(255, 0, 0)
+rgba(0, 0, 0, 0)

Tests/LibWeb/Text/input/css/CSSStyleDeclaration-custom-properties-2.html

@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<script src="../include.js"></script>
+<body></body>
+<script>
+    test(() => {
+        const div = document.createElement('div');
+        document.body.appendChild(div);
+        const nested = document.createElement('div');
+        div.appendChild(nested);
+
+        div.style.setProperty("--redcolor", "red");
+        println(`style.getPropertyValue(--redcolor)=${div.style.getPropertyValue("--redcolor")}`);
+        println(`style.getPropertyPriority(--redcolor)=${div.style.getPropertyPriority("--redcolor")}`);
+
+        nested.style["backgroundColor"] = "var(--redcolor)";
+        nested.style["width"] = "100px";
+        nested.style["height"] = "100px";
+ 
+        const bgColorBefore = getComputedStyle(nested).backgroundColor;
+        div.style.removeProperty("--redcolor");
+        const bgColorAfter = getComputedStyle(nested).backgroundColor;
+
+        println(bgColorBefore);
+        println(bgColorAfter);
+    });
+</script>