Ver código fonte

LibWeb: Streamline how inline CSS style declarations are constructed

When parsing the "style" attribute on elements, we'd previously ask the
CSS parser for a PropertyOwningCSSStyleDeclaration. Then we'd create a
new ElementCSSInlineStyleDeclaration and transfer the properties from
the first object to the second object.

This patch teaches the parser to make ElementCSSInlineStyleDeclaration
objects directly.
Andreas Kling 3 anos atrás
pai
commit
427beb97b5

+ 2 - 8
Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp

@@ -23,14 +23,8 @@ String PropertyOwningCSSStyleDeclaration::item(size_t index) const
     return CSS::string_from_property_id(m_properties[index].property_id);
     return CSS::string_from_property_id(m_properties[index].property_id);
 }
 }
 
 
-ElementInlineCSSStyleDeclaration::ElementInlineCSSStyleDeclaration(DOM::Element& element)
-    : PropertyOwningCSSStyleDeclaration({}, {})
-    , m_element(element.make_weak_ptr<DOM::Element>())
-{
-}
-
-ElementInlineCSSStyleDeclaration::ElementInlineCSSStyleDeclaration(DOM::Element& element, PropertyOwningCSSStyleDeclaration& declaration)
-    : PropertyOwningCSSStyleDeclaration(move(declaration.m_properties), move(declaration.m_custom_properties))
+ElementInlineCSSStyleDeclaration::ElementInlineCSSStyleDeclaration(DOM::Element& element, Vector<StyleProperty> properties, HashMap<String, StyleProperty> custom_properties)
+    : PropertyOwningCSSStyleDeclaration(move(properties), move(custom_properties))
     , m_element(element.make_weak_ptr<DOM::Element>())
     , m_element(element.make_weak_ptr<DOM::Element>())
 {
 {
 }
 }

+ 2 - 4
Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.h

@@ -86,16 +86,14 @@ private:
 
 
 class ElementInlineCSSStyleDeclaration final : public PropertyOwningCSSStyleDeclaration {
 class ElementInlineCSSStyleDeclaration final : public PropertyOwningCSSStyleDeclaration {
 public:
 public:
-    static NonnullRefPtr<ElementInlineCSSStyleDeclaration> create(DOM::Element& element) { return adopt_ref(*new ElementInlineCSSStyleDeclaration(element)); }
-    static NonnullRefPtr<ElementInlineCSSStyleDeclaration> create_and_take_properties_from(DOM::Element& element, PropertyOwningCSSStyleDeclaration& declaration) { return adopt_ref(*new ElementInlineCSSStyleDeclaration(element, declaration)); }
+    static NonnullRefPtr<ElementInlineCSSStyleDeclaration> create(DOM::Element& element, Vector<StyleProperty> properties, HashMap<String, StyleProperty> custom_properties) { return adopt_ref(*new ElementInlineCSSStyleDeclaration(element, move(properties), move(custom_properties))); }
     virtual ~ElementInlineCSSStyleDeclaration() override = default;
     virtual ~ElementInlineCSSStyleDeclaration() override = default;
 
 
     DOM::Element* element() { return m_element.ptr(); }
     DOM::Element* element() { return m_element.ptr(); }
     const DOM::Element* element() const { return m_element.ptr(); }
     const DOM::Element* element() const { return m_element.ptr(); }
 
 
 private:
 private:
-    explicit ElementInlineCSSStyleDeclaration(DOM::Element&);
-    explicit ElementInlineCSSStyleDeclaration(DOM::Element&, PropertyOwningCSSStyleDeclaration&);
+    explicit ElementInlineCSSStyleDeclaration(DOM::Element&, Vector<StyleProperty> properties, HashMap<String, StyleProperty> custom_properties);
 
 
     WeakPtr<DOM::Element> m_element;
     WeakPtr<DOM::Element> m_element;
 };
 };

+ 18 - 16
Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp

@@ -1997,10 +1997,11 @@ Vector<Vector<StyleComponentValueRule>> Parser::parse_a_comma_separated_list_of_
     return lists;
     return lists;
 }
 }
 
 
-RefPtr<PropertyOwningCSSStyleDeclaration> Parser::parse_as_style_attribute()
+RefPtr<ElementInlineCSSStyleDeclaration> Parser::parse_as_style_attribute(DOM::Element& element)
 {
 {
     auto declarations_and_at_rules = parse_a_list_of_declarations(m_token_stream);
     auto declarations_and_at_rules = parse_a_list_of_declarations(m_token_stream);
-    return convert_to_style_declaration(declarations_and_at_rules);
+    auto [properties, custom_properties] = extract_properties(declarations_and_at_rules);
+    return ElementInlineCSSStyleDeclaration::create(element, move(properties), move(custom_properties));
 }
 }
 
 
 Optional<AK::URL> Parser::parse_url_function(StyleComponentValueRule const& component_value, AllowedDataUrlType allowed_data_url_type)
 Optional<AK::URL> Parser::parse_url_function(StyleComponentValueRule const& component_value, AllowedDataUrlType allowed_data_url_type)
@@ -2169,30 +2170,32 @@ RefPtr<CSSRule> Parser::convert_to_rule(NonnullRefPtr<StyleRule> rule)
     return {};
     return {};
 }
 }
 
 
-RefPtr<PropertyOwningCSSStyleDeclaration> Parser::convert_to_style_declaration(Vector<DeclarationOrAtRule> declarations_and_at_rules)
+auto Parser::extract_properties(Vector<DeclarationOrAtRule> const& declarations_and_at_rules) -> PropertiesAndCustomProperties
 {
 {
-    Vector<StyleProperty> properties;
-    HashMap<String, StyleProperty> custom_properties;
-
+    PropertiesAndCustomProperties result;
     for (auto& declaration_or_at_rule : declarations_and_at_rules) {
     for (auto& declaration_or_at_rule : declarations_and_at_rules) {
         if (declaration_or_at_rule.is_at_rule()) {
         if (declaration_or_at_rule.is_at_rule()) {
             dbgln_if(CSS_PARSER_DEBUG, "!!! CSS at-rule is not allowed here!");
             dbgln_if(CSS_PARSER_DEBUG, "!!! CSS at-rule is not allowed here!");
             continue;
             continue;
         }
         }
 
 
-        auto& declaration = declaration_or_at_rule.m_declaration;
+        auto const& declaration = declaration_or_at_rule.declaration();
 
 
-        auto maybe_property = convert_to_style_property(declaration);
-        if (maybe_property.has_value()) {
-            auto property = maybe_property.value();
+        if (auto maybe_property = convert_to_style_property(declaration); maybe_property.has_value()) {
+            auto property = maybe_property.release_value();
             if (property.property_id == PropertyID::Custom) {
             if (property.property_id == PropertyID::Custom) {
-                custom_properties.set(property.custom_name, property);
+                result.custom_properties.set(property.custom_name, property);
             } else {
             } else {
-                properties.append(property);
+                result.properties.append(move(property));
             }
             }
         }
         }
     }
     }
+    return result;
+}
 
 
+RefPtr<PropertyOwningCSSStyleDeclaration> Parser::convert_to_style_declaration(Vector<DeclarationOrAtRule> declarations_and_at_rules)
+{
+    auto [properties, custom_properties] = extract_properties(declarations_and_at_rules);
     return PropertyOwningCSSStyleDeclaration::create(move(properties), move(custom_properties));
     return PropertyOwningCSSStyleDeclaration::create(move(properties), move(custom_properties));
 }
 }
 
 
@@ -5206,13 +5209,12 @@ RefPtr<CSS::CSSStyleSheet> parse_css(CSS::ParsingContext const& context, StringV
     return parser.parse_as_stylesheet();
     return parser.parse_as_stylesheet();
 }
 }
 
 
-RefPtr<CSS::PropertyOwningCSSStyleDeclaration> parse_css_style_attribute(CSS::ParsingContext const& context, StringView css)
+RefPtr<CSS::ElementInlineCSSStyleDeclaration> parse_css_style_attribute(CSS::ParsingContext const& context, StringView css, DOM::Element& element)
 {
 {
-    // FIXME: We could save some effort by creating an ElementInlineCSSStyleDeclaration right here.
     if (css.is_empty())
     if (css.is_empty())
-        return CSS::PropertyOwningCSSStyleDeclaration::create({}, {});
+        return CSS::ElementInlineCSSStyleDeclaration::create(element, {}, {});
     CSS::Parser parser(context, css);
     CSS::Parser parser(context, css);
-    return parser.parse_as_style_attribute();
+    return parser.parse_as_style_attribute(element);
 }
 }
 
 
 RefPtr<CSS::StyleValue> parse_css_value(CSS::ParsingContext const& context, StringView string, CSS::PropertyID property_id)
 RefPtr<CSS::StyleValue> parse_css_value(CSS::ParsingContext const& context, StringView string, CSS::PropertyID property_id)

+ 9 - 2
Userland/Libraries/LibWeb/CSS/Parser/Parser.h

@@ -105,7 +105,7 @@ public:
     Vector<StyleComponentValueRule> parse_as_list_of_component_values();
     Vector<StyleComponentValueRule> parse_as_list_of_component_values();
     Vector<Vector<StyleComponentValueRule>> parse_as_comma_separated_list_of_component_values();
     Vector<Vector<StyleComponentValueRule>> parse_as_comma_separated_list_of_component_values();
 
 
-    RefPtr<PropertyOwningCSSStyleDeclaration> parse_as_style_attribute();
+    RefPtr<ElementInlineCSSStyleDeclaration> parse_as_style_attribute(DOM::Element&);
 
 
     enum class SelectorParsingMode {
     enum class SelectorParsingMode {
         Standard,
         Standard,
@@ -343,6 +343,13 @@ private:
     static bool has_ignored_vendor_prefix(StringView);
     static bool has_ignored_vendor_prefix(StringView);
     static bool is_builtin(StringView);
     static bool is_builtin(StringView);
 
 
+    struct PropertiesAndCustomProperties {
+        Vector<StyleProperty> properties;
+        HashMap<String, StyleProperty> custom_properties;
+    };
+
+    PropertiesAndCustomProperties extract_properties(Vector<DeclarationOrAtRule> const&);
+
     ParsingContext m_context;
     ParsingContext m_context;
 
 
     Tokenizer m_tokenizer;
     Tokenizer m_tokenizer;
@@ -355,7 +362,7 @@ private:
 namespace Web {
 namespace Web {
 
 
 RefPtr<CSS::CSSStyleSheet> parse_css(CSS::ParsingContext const&, StringView);
 RefPtr<CSS::CSSStyleSheet> parse_css(CSS::ParsingContext const&, StringView);
-RefPtr<CSS::PropertyOwningCSSStyleDeclaration> parse_css_style_attribute(CSS::ParsingContext const&, StringView);
+RefPtr<CSS::ElementInlineCSSStyleDeclaration> parse_css_style_attribute(CSS::ParsingContext const&, StringView, DOM::Element&);
 RefPtr<CSS::StyleValue> parse_css_value(CSS::ParsingContext const&, StringView, CSS::PropertyID property_id = CSS::PropertyID::Invalid);
 RefPtr<CSS::StyleValue> parse_css_value(CSS::ParsingContext const&, StringView, CSS::PropertyID property_id = CSS::PropertyID::Invalid);
 Optional<CSS::SelectorList> parse_selector(CSS::ParsingContext const&, StringView);
 Optional<CSS::SelectorList> parse_selector(CSS::ParsingContext const&, StringView);
 RefPtr<CSS::CSSRule> parse_css_rule(CSS::ParsingContext const&, StringView);
 RefPtr<CSS::CSSRule> parse_css_rule(CSS::ParsingContext const&, StringView);

+ 3 - 6
Userland/Libraries/LibWeb/DOM/Element.cpp

@@ -255,11 +255,8 @@ void Element::parse_attribute(const FlyString& name, const String& value)
         if (m_class_list)
         if (m_class_list)
             m_class_list->associated_attribute_changed(value);
             m_class_list->associated_attribute_changed(value);
     } else if (name == HTML::AttributeNames::style) {
     } else if (name == HTML::AttributeNames::style) {
-        auto parsed_style = parse_css_style_attribute(CSS::ParsingContext(document()), value);
-        if (!parsed_style.is_null()) {
-            m_inline_style = CSS::ElementInlineCSSStyleDeclaration::create_and_take_properties_from(*this, parsed_style.release_nonnull());
-            set_needs_style_update(true);
-        }
+        m_inline_style = parse_css_style_attribute(CSS::ParsingContext(document()), value, *this);
+        set_needs_style_update(true);
     }
     }
 }
 }
 
 
@@ -452,7 +449,7 @@ void Element::set_shadow_root(RefPtr<ShadowRoot> shadow_root)
 NonnullRefPtr<CSS::CSSStyleDeclaration> Element::style_for_bindings()
 NonnullRefPtr<CSS::CSSStyleDeclaration> Element::style_for_bindings()
 {
 {
     if (!m_inline_style)
     if (!m_inline_style)
-        m_inline_style = CSS::ElementInlineCSSStyleDeclaration::create(*this);
+        m_inline_style = CSS::ElementInlineCSSStyleDeclaration::create(*this, {}, {});
     return *m_inline_style;
     return *m_inline_style;
 }
 }