From f0d198ca3f4866ccf406c3320d2fa5326bf6fe8c Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Tue, 18 Mar 2025 15:05:42 +0000 Subject: [PATCH] LibWeb/CSS: Move CSSStyleDeclaration subclasses' fields into it The spec defines several properties on the declaration which we previously made virtual or stored on subclasses. This commit moves these into CSSStyleDeclaration itself, and updates some of the naming. --- Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp | 73 ++++++++----------- Libraries/LibWeb/CSS/CSSStyleDeclaration.h | 65 ++++++++++++----- .../CSS/ResolvedCSSStyleDeclaration.cpp | 48 ++++++------ .../LibWeb/CSS/ResolvedCSSStyleDeclaration.h | 7 -- 4 files changed, 101 insertions(+), 92 deletions(-) diff --git a/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp b/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp index 459f0a72a60..0f97ffa29f5 100644 --- a/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp +++ b/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp @@ -25,8 +25,10 @@ GC_DEFINE_ALLOCATOR(CSSStyleDeclaration); GC_DEFINE_ALLOCATOR(PropertyOwningCSSStyleDeclaration); GC_DEFINE_ALLOCATOR(ElementInlineCSSStyleDeclaration); -CSSStyleDeclaration::CSSStyleDeclaration(JS::Realm& realm) +CSSStyleDeclaration::CSSStyleDeclaration(JS::Realm& realm, Computed computed, Readonly readonly) : PlatformObject(realm) + , m_computed(computed == Computed::Yes) + , m_readonly(readonly == Readonly::Yes) { m_legacy_platform_object_flags = LegacyPlatformObjectFlags { .supports_indexed_properties = true, @@ -39,27 +41,21 @@ void CSSStyleDeclaration::initialize(JS::Realm& realm) WEB_SET_PROTOTYPE_FOR_INTERFACE(CSSStyleDeclaration); } -GC::Ptr CSSStyleDeclaration::parent_rule() const -{ - return nullptr; -} - GC::Ref PropertyOwningCSSStyleDeclaration::create(JS::Realm& realm, Vector properties, HashMap custom_properties) { return realm.create(realm, move(properties), move(custom_properties)); } PropertyOwningCSSStyleDeclaration::PropertyOwningCSSStyleDeclaration(JS::Realm& realm, Vector properties, HashMap custom_properties) - : CSSStyleDeclaration(realm) + : CSSStyleDeclaration(realm, Computed::No, Readonly::No) , m_properties(move(properties)) , m_custom_properties(move(custom_properties)) { } -void PropertyOwningCSSStyleDeclaration::visit_edges(Cell::Visitor& visitor) +void PropertyOwningCSSStyleDeclaration::visit_edges(Visitor& visitor) { Base::visit_edges(visitor); - visitor.visit(m_parent_rule); for (auto& property : m_properties) { if (property.value->is_image()) property.value->as_image().visit_edges(visitor); @@ -81,14 +77,8 @@ GC::Ref ElementInlineCSSStyleDeclaration::crea ElementInlineCSSStyleDeclaration::ElementInlineCSSStyleDeclaration(DOM::Element& element, Vector properties, HashMap custom_properties) : PropertyOwningCSSStyleDeclaration(element.realm(), move(properties), move(custom_properties)) - , m_element(element.make_weak_ptr()) { -} - -void ElementInlineCSSStyleDeclaration::visit_edges(Cell::Visitor& visitor) -{ - Base::visit_edges(visitor); - visitor.visit(m_element); + set_owner_node(DOM::ElementReference { element }); } size_t PropertyOwningCSSStyleDeclaration::length() const @@ -132,8 +122,8 @@ WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::set_property(String return {}; // 5. Let component value list be the result of parsing value for property property. - auto component_value_list = is(this) - ? parse_css_value(CSS::Parser::ParsingParams { static_cast(*this).element()->document() }, value, property_id) + auto component_value_list = owner_node().has_value() + ? parse_css_value(CSS::Parser::ParsingParams { owner_node()->element().document() }, value, property_id) : parse_css_value(CSS::Parser::ParsingParams {}, value, property_id); // 6. If component value list is null, then return. @@ -223,21 +213,21 @@ WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::remove_property(S void ElementInlineCSSStyleDeclaration::update_style_attribute() { // 1. Assert: declaration block’s computed flag is unset. - // NOTE: Unnecessary, only relevant for ResolvedCSSStyleDeclaration. + VERIFY(!is_computed()); // 2. Let owner node be declaration block’s owner node. // 3. If owner node is null, then return. - if (!m_element) + if (!owner_node().has_value()) return; // 4. Set declaration block’s updating flag. - m_updating = true; + set_is_updating(true); // 5. Set an attribute value for owner node using "style" and the result of serializing declaration block. - MUST(m_element->set_attribute(HTML::AttributeNames::style, serialized())); + MUST(owner_node()->element().set_attribute(HTML::AttributeNames::style, serialized())); // 6. Unset declaration block’s updating flag. - m_updating = false; + set_is_updating(false); } // https://drafts.csswg.org/cssom/#set-a-css-declaration @@ -445,8 +435,8 @@ String CSSStyleDeclaration::get_property_value(StringView property_name) const auto maybe_custom_property = custom_property(FlyString::from_utf8_without_validation(property_name.bytes())); if (maybe_custom_property.has_value()) { return maybe_custom_property.value().value->to_string( - computed_flag() ? CSSStyleValue::SerializationMode::ResolvedValue - : CSSStyleValue::SerializationMode::Normal); + is_computed() ? CSSStyleValue::SerializationMode::ResolvedValue + : CSSStyleValue::SerializationMode::Normal); } return {}; } @@ -455,8 +445,8 @@ String CSSStyleDeclaration::get_property_value(StringView property_name) const if (!maybe_property.has_value()) return {}; return maybe_property->value->to_string( - computed_flag() ? CSSStyleValue::SerializationMode::ResolvedValue - : CSSStyleValue::SerializationMode::Normal); + is_computed() ? CSSStyleValue::SerializationMode::ResolvedValue + : CSSStyleValue::SerializationMode::Normal); } // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-getpropertypriority @@ -520,6 +510,14 @@ Optional CSSStyleDeclaration::item_value(size_t index) const return JS::PrimitiveString::create(vm(), value); } +void CSSStyleDeclaration::visit_edges(Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_parent_rule); + if (m_owner_node.has_value()) + m_owner_node->visit(visitor); +} + // https://www.w3.org/TR/cssom/#serialize-a-css-declaration static String serialize_a_css_declaration(CSS::PropertyID property, StringView value, Important important) { @@ -663,13 +661,14 @@ void PropertyOwningCSSStyleDeclaration::set_the_declarations(Vectordocument()), css_text, *m_element.ptr()); + auto style = parse_css_style_attribute(CSS::Parser::ParsingParams(element->element().document()), css_text, element->element()); set_the_declarations(style->properties(), style->custom_properties()); } @@ -677,8 +676,8 @@ void ElementInlineCSSStyleDeclaration::set_declarations_from_text(StringView css WebIDL::ExceptionOr ElementInlineCSSStyleDeclaration::set_css_text(StringView css_text) { // FIXME: What do we do if the element is null? - if (!m_element) { - dbgln("FIXME: Returning from ElementInlineCSSStyleDeclaration::set_css_text as m_element is null."); + if (!owner_node().has_value()) { + dbgln("FIXME: Returning from ElementInlineCSSStyleDeclaration::set_css_text as element is null."); return {}; } @@ -695,14 +694,4 @@ WebIDL::ExceptionOr ElementInlineCSSStyleDeclaration::set_css_text(StringV return {}; } -GC::Ptr PropertyOwningCSSStyleDeclaration::parent_rule() const -{ - return m_parent_rule; -} - -void PropertyOwningCSSStyleDeclaration::set_parent_rule(GC::Ref rule) -{ - m_parent_rule = rule; -} - } diff --git a/Libraries/LibWeb/CSS/CSSStyleDeclaration.h b/Libraries/LibWeb/CSS/CSSStyleDeclaration.h index e93db085ead..08b78acdd71 100644 --- a/Libraries/LibWeb/CSS/CSSStyleDeclaration.h +++ b/Libraries/LibWeb/CSS/CSSStyleDeclaration.h @@ -1,5 +1,6 @@ /* * Copyright (c) 2018-2023, Andreas Kling + * Copyright (c) 2024-2025, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -12,9 +13,11 @@ #include #include #include +#include namespace Web::CSS { +// https://drafts.csswg.org/cssom/#css-declaration-blocks class CSSStyleDeclaration : public Bindings::PlatformObject , public Bindings::GeneratedCSSStyleProperties { @@ -48,13 +51,36 @@ public: virtual String serialized() const = 0; - virtual GC::Ptr parent_rule() const; - // https://drafts.csswg.org/cssom/#cssstyledeclaration-computed-flag - [[nodiscard]] virtual bool computed_flag() const { return false; } + [[nodiscard]] bool is_computed() const { return m_computed; } + + // https://drafts.csswg.org/cssom/#cssstyledeclaration-readonly-flag + [[nodiscard]] bool is_readonly() const { return m_readonly; } + + // https://drafts.csswg.org/cssom/#cssstyledeclaration-parent-css-rule + GC::Ptr parent_rule() const { return m_parent_rule; } + void set_parent_rule(GC::Ptr parent) { m_parent_rule = parent; } + + // https://drafts.csswg.org/cssom/#cssstyledeclaration-owner-node + Optional owner_node() const { return m_owner_node; } + void set_owner_node(Optional owner_node) { m_owner_node = move(owner_node); } + + // https://drafts.csswg.org/cssom/#cssstyledeclaration-updating-flag + [[nodiscard]] bool is_updating() const { return m_updating; } + void set_is_updating(bool value) { m_updating = value; } protected: - explicit CSSStyleDeclaration(JS::Realm&); + enum class Computed : u8 { + No, + Yes, + }; + enum class Readonly : u8 { + No, + Yes, + }; + explicit CSSStyleDeclaration(JS::Realm&, Computed, Readonly); + + virtual void visit_edges(Visitor&) override; virtual CSSStyleDeclaration& generated_style_properties_to_css_style_declaration() override { return *this; } @@ -62,6 +88,21 @@ private: // ^PlatformObject virtual Optional item_value(size_t index) const override; Optional get_property_internal(PropertyID) const; + + // https://drafts.csswg.org/cssom/#cssstyledeclaration-parent-css-rule + GC::Ptr m_parent_rule { nullptr }; + + // https://drafts.csswg.org/cssom/#cssstyledeclaration-owner-node + Optional m_owner_node {}; + + // https://drafts.csswg.org/cssom/#cssstyledeclaration-computed-flag + bool m_computed { false }; + + // https://drafts.csswg.org/cssom/#cssstyledeclaration-readonly-flag + bool m_readonly { false }; + + // https://drafts.csswg.org/cssom/#cssstyledeclaration-updating-flag + bool m_updating { false }; }; class PropertyOwningCSSStyleDeclaration : public CSSStyleDeclaration { @@ -92,9 +133,6 @@ public: virtual String serialized() const final override; virtual WebIDL::ExceptionOr set_css_text(StringView) override; - virtual GC::Ptr parent_rule() const override; - void set_parent_rule(GC::Ref); - protected: PropertyOwningCSSStyleDeclaration(JS::Realm&, Vector, HashMap); @@ -108,7 +146,6 @@ private: virtual void visit_edges(Cell::Visitor&) override; - GC::Ptr m_parent_rule; Vector m_properties; HashMap m_custom_properties; }; @@ -122,11 +159,6 @@ public: virtual ~ElementInlineCSSStyleDeclaration() override = default; - DOM::Element* element() { return m_element.ptr(); } - const DOM::Element* element() const { return m_element.ptr(); } - - bool is_updating() const { return m_updating; } - void set_declarations_from_text(StringView); virtual WebIDL::ExceptionOr set_css_text(StringView) override; @@ -134,14 +166,7 @@ public: private: ElementInlineCSSStyleDeclaration(DOM::Element&, Vector properties, HashMap custom_properties); - virtual void visit_edges(Cell::Visitor&) override; - virtual void update_style_attribute() override; - - GC::Ptr m_element; - - // https://drafts.csswg.org/cssom/#cssstyledeclaration-updating-flag - bool m_updating { false }; }; } diff --git a/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp b/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp index ba618a9f4c7..cba2e8bf139 100644 --- a/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp +++ b/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp @@ -39,16 +39,9 @@ GC::Ref ResolvedCSSStyleDeclaration::create(DOM::El } ResolvedCSSStyleDeclaration::ResolvedCSSStyleDeclaration(DOM::Element& element, Optional pseudo_element) - : CSSStyleDeclaration(element.realm()) - , m_element(element) - , m_pseudo_element(move(pseudo_element)) + : CSSStyleDeclaration(element.realm(), Computed::Yes, Readonly::Yes) { -} - -void ResolvedCSSStyleDeclaration::visit_edges(Cell::Visitor& visitor) -{ - Base::visit_edges(visitor); - visitor.visit(m_element); + set_owner_node(DOM::ElementReference { element, pseudo_element }); } // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-length @@ -180,6 +173,9 @@ RefPtr ResolvedCSSStyleDeclaration::style_value_for_propert return {}; }; + auto& element = owner_node()->element(); + auto pseudo_element = owner_node()->pseudo_element(); + auto used_value_for_inset = [&layout_node, used_value_for_property](LengthPercentage const& start_side, LengthPercentage const& end_side, Function&& used_value_getter) -> Optional { if (!layout_node.is_positioned()) return {}; @@ -194,10 +190,10 @@ RefPtr ResolvedCSSStyleDeclaration::style_value_for_propert return used_value_for_property(move(used_value_getter)); }; - auto get_computed_value = [this](PropertyID property_id) -> auto const& { - if (m_pseudo_element.has_value()) - return m_element->pseudo_element_computed_properties(m_pseudo_element.value())->property(property_id); - return m_element->computed_properties()->property(property_id); + auto get_computed_value = [&element, pseudo_element](PropertyID property_id) -> auto const& { + if (pseudo_element.has_value()) + return element.pseudo_element_computed_properties(pseudo_element.value())->property(property_id); + return element.computed_properties()->property(property_id); }; // A limited number of properties have special rules for producing their "resolved value". @@ -557,15 +553,18 @@ RefPtr ResolvedCSSStyleDeclaration::style_value_for_propert Optional ResolvedCSSStyleDeclaration::property(PropertyID property_id) const { + auto& element = owner_node()->element(); + auto pseudo_element = owner_node()->pseudo_element(); + // https://www.w3.org/TR/cssom-1/#dom-window-getcomputedstyle // NOTE: This is a partial enforcement of step 5 ("If elt is connected, ...") - if (!m_element->is_connected()) + if (!element.is_connected()) return {}; auto get_layout_node = [&]() { - if (m_pseudo_element.has_value()) - return m_element->get_pseudo_element_node(m_pseudo_element.value()); - return m_element->layout_node(); + if (pseudo_element.has_value()) + return element.get_pseudo_element_node(pseudo_element.value()); + return element.layout_node(); }; Layout::NodeWithStyle* layout_node = get_layout_node(); @@ -574,15 +573,15 @@ Optional ResolvedCSSStyleDeclaration::property(PropertyID propert // We may legitimately have no layout node if we're not visible, but this protects against situations // where we're requesting the computed style before layout has happened. if (!layout_node || property_affects_layout(property_id)) { - const_cast(m_element->document()).update_layout(DOM::UpdateLayoutReason::ResolvedCSSStyleDeclarationProperty); + element.document().update_layout(DOM::UpdateLayoutReason::ResolvedCSSStyleDeclarationProperty); layout_node = get_layout_node(); } else { // FIXME: If we had a way to update style for a single element, this would be a good place to use it. - const_cast(m_element->document()).update_style(); + element.document().update_style(); } if (!layout_node) { - auto style = m_element->document().style_computer().compute_style(const_cast(*m_element), m_pseudo_element); + auto style = element.document().style_computer().compute_style(element, pseudo_element); // FIXME: This is a stopgap until we implement shorthand -> longhand conversion. auto const* value = style->maybe_null_property(property_id); @@ -607,11 +606,14 @@ Optional ResolvedCSSStyleDeclaration::property(PropertyID propert Optional ResolvedCSSStyleDeclaration::custom_property(FlyString const& name) const { - const_cast(m_element->document()).update_style(); + auto& element = owner_node()->element(); + auto pseudo_element = owner_node()->pseudo_element(); - auto const* element_to_check = m_element.ptr(); + element.document().update_style(); + + auto const* element_to_check = &element; while (element_to_check) { - if (auto property = element_to_check->custom_properties(m_pseudo_element).get(name); property.has_value()) + if (auto property = element_to_check->custom_properties(pseudo_element).get(name); property.has_value()) return *property; element_to_check = element_to_check->parent_element(); diff --git a/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h b/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h index beb40f92cf2..7743ab02751 100644 --- a/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h +++ b/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.h @@ -36,14 +36,7 @@ public: private: explicit ResolvedCSSStyleDeclaration(DOM::Element&, Optional); - virtual void visit_edges(Cell::Visitor&) override; - - virtual bool computed_flag() const override { return true; } - RefPtr style_value_for_property(Layout::NodeWithStyle const&, PropertyID) const; - - GC::Ref m_element; - Optional m_pseudo_element; }; }