From 687d32b712f96c49c8b48a668cff4c37e42ebe1c Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Mon, 17 Mar 2025 13:04:41 +0000 Subject: [PATCH] LibWeb: Remove ElementInlineCSSStyleDeclaration entirely All of its behavior has now been moved up into its parent classes. --- Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp | 76 +++++++------------ Libraries/LibWeb/CSS/CSSStyleDeclaration.h | 32 ++------ Libraries/LibWeb/DOM/Element.cpp | 4 +- Libraries/LibWeb/DOM/Element.h | 6 +- Libraries/LibWeb/Forward.h | 1 - .../wpt-import/css/css-nesting/cssom.txt | 5 +- .../nested-declarations-cssom-whitespace.txt | 5 +- 7 files changed, 46 insertions(+), 83 deletions(-) diff --git a/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp b/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp index 4cd7de609cb..1bec03e8f17 100644 --- a/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp +++ b/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp @@ -23,7 +23,6 @@ namespace Web::CSS { GC_DEFINE_ALLOCATOR(CSSStyleDeclaration); GC_DEFINE_ALLOCATOR(PropertyOwningCSSStyleDeclaration); -GC_DEFINE_ALLOCATOR(ElementInlineCSSStyleDeclaration); CSSStyleDeclaration::CSSStyleDeclaration(JS::Realm& realm, Computed computed, Readonly readonly) : PlatformObject(realm) @@ -43,14 +42,21 @@ void CSSStyleDeclaration::initialize(JS::Realm& realm) GC::Ref PropertyOwningCSSStyleDeclaration::create(JS::Realm& realm, Vector properties, HashMap custom_properties) { - return realm.create(realm, move(properties), move(custom_properties)); + return realm.create(realm, nullptr, move(properties), move(custom_properties)); } -PropertyOwningCSSStyleDeclaration::PropertyOwningCSSStyleDeclaration(JS::Realm& realm, Vector properties, HashMap custom_properties) +GC::Ref PropertyOwningCSSStyleDeclaration::create_element_inline_style(JS::Realm& realm, GC::Ref element, Vector properties, HashMap custom_properties) +{ + return realm.create(realm, element, move(properties), move(custom_properties)); +} + +PropertyOwningCSSStyleDeclaration::PropertyOwningCSSStyleDeclaration(JS::Realm& realm, GC::Ptr element, Vector properties, HashMap custom_properties) : CSSStyleDeclaration(realm, Computed::No, Readonly::No) , m_properties(move(properties)) , m_custom_properties(move(custom_properties)) { + if (element) + set_owner_node(DOM::ElementReference { *element }); } void PropertyOwningCSSStyleDeclaration::visit_edges(Visitor& visitor) @@ -69,18 +75,6 @@ String PropertyOwningCSSStyleDeclaration::item(size_t index) const return CSS::string_from_property_id(m_properties[index].property_id).to_string(); } -GC::Ref ElementInlineCSSStyleDeclaration::create(DOM::Element& element, Vector properties, HashMap custom_properties) -{ - auto& realm = element.realm(); - return realm.create(element, move(properties), move(custom_properties)); -} - -ElementInlineCSSStyleDeclaration::ElementInlineCSSStyleDeclaration(DOM::Element& element, Vector properties, HashMap custom_properties) - : PropertyOwningCSSStyleDeclaration(element.realm(), move(properties), move(custom_properties)) -{ - set_owner_node(DOM::ElementReference { element }); -} - size_t PropertyOwningCSSStyleDeclaration::length() const { return m_properties.size(); @@ -210,7 +204,7 @@ WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::remove_property(S } // https://drafts.csswg.org/cssom/#update-style-attribute-for -void ElementInlineCSSStyleDeclaration::update_style_attribute() +void CSSStyleDeclaration::update_style_attribute() { // 1. Assert: declaration block’s computed flag is unset. VERIFY(!is_computed()); @@ -640,9 +634,21 @@ String PropertyOwningCSSStyleDeclaration::serialized() const return MUST(builder.to_string()); } +// https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-csstext WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::set_css_text(StringView css_text) { - dbgln("(STUBBED) PropertyOwningCSSStyleDeclaration::set_css_text(css_text='{}')", css_text); + // 1. If the readonly flag is set, then throw a NoModificationAllowedError exception. + if (is_readonly()) { + return WebIDL::NoModificationAllowedError::create(realm(), "Cannot modify properties: CSSStyleDeclaration is read-only."_string); + } + + // 2. Empty the declarations. + // 3. Parse the given value and, if the return value is not the empty list, insert the items in the list into the declarations, in specified order. + set_declarations_from_text(css_text); + + // 4. Update style attribute for the CSS declaration block. + update_style_attribute(); + return {}; } @@ -658,40 +664,14 @@ void PropertyOwningCSSStyleDeclaration::set_the_declarations(Vectorelement().document()), css_text); + auto parsing_params = owner_node().has_value() + ? Parser::ParsingParams(owner_node()->element().document()) + : Parser::ParsingParams(); + auto style = parse_css_style_attribute(parsing_params, css_text); set_the_declarations(style.properties, style.custom_properties); } -// https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-csstext -WebIDL::ExceptionOr ElementInlineCSSStyleDeclaration::set_css_text(StringView css_text) -{ - // FIXME: What do we do if the element is null? - if (!owner_node().has_value()) { - dbgln("FIXME: Returning from ElementInlineCSSStyleDeclaration::set_css_text as element is null."); - return {}; - } - - // 1. If the computed flag is set, then throw a NoModificationAllowedError exception. - // NOTE: See ResolvedCSSStyleDeclaration. - - // 2. Empty the declarations. - // 3. Parse the given value and, if the return value is not the empty list, insert the items in the list into the declarations, in specified order. - set_declarations_from_text(css_text); - - // 4. Update style attribute for the CSS declaration block. - update_style_attribute(); - - return {}; -} - } diff --git a/Libraries/LibWeb/CSS/CSSStyleDeclaration.h b/Libraries/LibWeb/CSS/CSSStyleDeclaration.h index 08b78acdd71..0092fa13cc6 100644 --- a/Libraries/LibWeb/CSS/CSSStyleDeclaration.h +++ b/Libraries/LibWeb/CSS/CSSStyleDeclaration.h @@ -84,6 +84,8 @@ protected: virtual CSSStyleDeclaration& generated_style_properties_to_css_style_declaration() override { return *this; } + void update_style_attribute(); + private: // ^PlatformObject virtual Optional item_value(size_t index) const override; @@ -109,12 +111,13 @@ class PropertyOwningCSSStyleDeclaration : public CSSStyleDeclaration { WEB_PLATFORM_OBJECT(PropertyOwningCSSStyleDeclaration, CSSStyleDeclaration); GC_DECLARE_ALLOCATOR(PropertyOwningCSSStyleDeclaration); - friend class ElementInlineCSSStyleDeclaration; - public: [[nodiscard]] static GC::Ref create(JS::Realm&, Vector, HashMap custom_properties); + [[nodiscard]] static GC::Ref + create_element_inline_style(JS::Realm&, GC::Ref, Vector, HashMap custom_properties); + virtual ~PropertyOwningCSSStyleDeclaration() override = default; virtual size_t length() const override; @@ -133,10 +136,10 @@ public: virtual String serialized() const final override; virtual WebIDL::ExceptionOr set_css_text(StringView) override; -protected: - PropertyOwningCSSStyleDeclaration(JS::Realm&, Vector, HashMap); + void set_declarations_from_text(StringView); - virtual void update_style_attribute() { } +protected: + PropertyOwningCSSStyleDeclaration(JS::Realm&, GC::Ptr owner_node, Vector, HashMap); void empty_the_declarations(); void set_the_declarations(Vector properties, HashMap custom_properties); @@ -150,23 +153,4 @@ private: HashMap m_custom_properties; }; -class ElementInlineCSSStyleDeclaration final : public PropertyOwningCSSStyleDeclaration { - WEB_PLATFORM_OBJECT(ElementInlineCSSStyleDeclaration, PropertyOwningCSSStyleDeclaration); - GC_DECLARE_ALLOCATOR(ElementInlineCSSStyleDeclaration); - -public: - [[nodiscard]] static GC::Ref create(DOM::Element&, Vector, HashMap custom_properties); - - virtual ~ElementInlineCSSStyleDeclaration() override = default; - - void set_declarations_from_text(StringView); - - virtual WebIDL::ExceptionOr set_css_text(StringView) override; - -private: - ElementInlineCSSStyleDeclaration(DOM::Element&, Vector properties, HashMap custom_properties); - - virtual void update_style_attribute() override; -}; - } diff --git a/Libraries/LibWeb/DOM/Element.cpp b/Libraries/LibWeb/DOM/Element.cpp index 3479b2a0733..10f137cd7f8 100644 --- a/Libraries/LibWeb/DOM/Element.cpp +++ b/Libraries/LibWeb/DOM/Element.cpp @@ -921,7 +921,7 @@ void Element::set_shadow_root(GC::Ptr shadow_root) CSS::CSSStyleDeclaration* Element::style_for_bindings() { if (!m_inline_style) - m_inline_style = CSS::ElementInlineCSSStyleDeclaration::create(*this, {}, {}); + m_inline_style = CSS::PropertyOwningCSSStyleDeclaration::create_element_inline_style(realm(), *this, {}, {}); return m_inline_style; } @@ -3524,7 +3524,7 @@ void Element::attribute_changed(FlyString const& local_name, Optional co if (m_inline_style && m_inline_style->is_updating()) return; if (!m_inline_style) - m_inline_style = CSS::ElementInlineCSSStyleDeclaration::create(*this, {}, {}); + m_inline_style = CSS::PropertyOwningCSSStyleDeclaration::create_element_inline_style(realm(), *this, {}, {}); m_inline_style->set_declarations_from_text(*value); set_needs_style_update(true); } diff --git a/Libraries/LibWeb/DOM/Element.h b/Libraries/LibWeb/DOM/Element.h index 29bc2052a74..d2fba9fe53e 100644 --- a/Libraries/LibWeb/DOM/Element.h +++ b/Libraries/LibWeb/DOM/Element.h @@ -211,8 +211,8 @@ public: void reset_animated_css_properties(); - GC::Ptr inline_style() { return m_inline_style; } - GC::Ptr inline_style() const { return m_inline_style; } + GC::Ptr inline_style() { return m_inline_style; } + GC::Ptr inline_style() const { return m_inline_style; } CSS::CSSStyleDeclaration* style_for_bindings(); @@ -499,7 +499,7 @@ private: FlyString m_html_uppercased_qualified_name; GC::Ptr m_attributes; - GC::Ptr m_inline_style; + GC::Ptr m_inline_style; GC::Ptr m_class_list; GC::Ptr m_shadow_root; diff --git a/Libraries/LibWeb/Forward.h b/Libraries/LibWeb/Forward.h index a0b41e4c5af..b3f539a1089 100644 --- a/Libraries/LibWeb/Forward.h +++ b/Libraries/LibWeb/Forward.h @@ -189,7 +189,6 @@ class Display; class DisplayStyleValue; class EasingStyleValue; class EdgeStyleValue; -class ElementInlineCSSStyleDeclaration; class ExplicitGridTrack; class FilterValueListStyleValue; class FitContentStyleValue; diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/css-nesting/cssom.txt b/Tests/LibWeb/Text/expected/wpt-import/css/css-nesting/cssom.txt index 75f7d9a0f11..8c2bfc74d14 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/css-nesting/cssom.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/css-nesting/cssom.txt @@ -2,8 +2,7 @@ Harness status: OK Found 13 tests -12 Pass -1 Fail +13 Pass Pass CSSStyleRule is a CSSGroupingRule Pass Simple CSSOM manipulation of subrules Pass Simple CSSOM manipulation of subrules 1 @@ -13,7 +12,7 @@ Pass Simple CSSOM manipulation of subrules 4 Pass Simple CSSOM manipulation of subrules 5 Pass Simple CSSOM manipulation of subrules 6 Pass Simple CSSOM manipulation of subrules 7 -Fail Simple CSSOM manipulation of subrules 8 +Pass Simple CSSOM manipulation of subrules 8 Pass Simple CSSOM manipulation of subrules 9 Pass Simple CSSOM manipulation of subrules 10 Pass Mutating the selectorText of outer rule invalidates inner rules \ No newline at end of file diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/css-nesting/nested-declarations-cssom-whitespace.txt b/Tests/LibWeb/Text/expected/wpt-import/css/css-nesting/nested-declarations-cssom-whitespace.txt index 828018cd051..2d94c4eb749 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/css-nesting/nested-declarations-cssom-whitespace.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/css-nesting/nested-declarations-cssom-whitespace.txt @@ -2,6 +2,7 @@ Harness status: OK Found 2 tests -2 Fail -Fail Empty CSSNestedDeclarations do not affect outer serialization +1 Pass +1 Fail +Pass Empty CSSNestedDeclarations do not affect outer serialization Fail Empty CSSNestedDeclarations do not affect outer serialization (nested grouping rule) \ No newline at end of file