LibWeb: Store custom properties in an OrderedHashMap

We are expected to preserve their order within a style declaration, so
let's do that. Passes 1 tracked WPT subtest.
This commit is contained in:
Sam Atkins 2025-09-18 11:44:56 +01:00 committed by Jelle Raaijmakers
parent 2674bd428e
commit 95aceb6ec9
11 changed files with 30 additions and 31 deletions

View file

@ -32,7 +32,7 @@ namespace Web::CSS {
GC_DEFINE_ALLOCATOR(CSSStyleProperties);
GC::Ref<CSSStyleProperties> CSSStyleProperties::create(JS::Realm& realm, Vector<StyleProperty> properties, HashMap<FlyString, StyleProperty> custom_properties)
GC::Ref<CSSStyleProperties> CSSStyleProperties::create(JS::Realm& realm, Vector<StyleProperty> properties, OrderedHashMap<FlyString, StyleProperty> custom_properties)
{
// https://drafts.csswg.org/cssom/#dom-cssstylerule-style
// The style attribute must return a CSSStyleProperties object for the style rule, with the following properties:
@ -54,10 +54,10 @@ GC::Ref<CSSStyleProperties> CSSStyleProperties::create_resolved_style(JS::Realm&
// parent CSS rule: Null.
// owner node: obj.
// AD-HOC: Rather than instantiate with a list of decls, they're generated on demand.
return realm.create<CSSStyleProperties>(realm, Computed::Yes, Readonly::Yes, Vector<StyleProperty> {}, HashMap<FlyString, StyleProperty> {}, move(element_reference));
return realm.create<CSSStyleProperties>(realm, Computed::Yes, Readonly::Yes, Vector<StyleProperty> {}, OrderedHashMap<FlyString, StyleProperty> {}, move(element_reference));
}
GC::Ref<CSSStyleProperties> CSSStyleProperties::create_element_inline_style(DOM::AbstractElement element_reference, Vector<StyleProperty> properties, HashMap<FlyString, StyleProperty> custom_properties)
GC::Ref<CSSStyleProperties> CSSStyleProperties::create_element_inline_style(DOM::AbstractElement element_reference, Vector<StyleProperty> properties, OrderedHashMap<FlyString, StyleProperty> custom_properties)
{
// https://drafts.csswg.org/cssom/#dom-elementcssinlinestyle-style
// The style attribute must return a CSS declaration block object whose readonly flag is unset, whose parent CSS
@ -66,7 +66,7 @@ GC::Ref<CSSStyleProperties> CSSStyleProperties::create_element_inline_style(DOM:
return realm.create<CSSStyleProperties>(realm, Computed::No, Readonly::No, convert_declarations_to_specified_order(properties), move(custom_properties), move(element_reference));
}
CSSStyleProperties::CSSStyleProperties(JS::Realm& realm, Computed computed, Readonly readonly, Vector<StyleProperty> properties, HashMap<FlyString, StyleProperty> custom_properties, Optional<DOM::AbstractElement> owner_node)
CSSStyleProperties::CSSStyleProperties(JS::Realm& realm, Computed computed, Readonly readonly, Vector<StyleProperty> properties, OrderedHashMap<FlyString, StyleProperty> custom_properties, Optional<DOM::AbstractElement> owner_node)
: CSSStyleDeclaration(realm, computed, readonly)
, m_properties(move(properties))
, m_custom_properties(move(custom_properties))
@ -1312,7 +1312,7 @@ void CSSStyleProperties::empty_the_declarations()
m_custom_properties.clear();
}
void CSSStyleProperties::set_the_declarations(Vector<StyleProperty> properties, HashMap<FlyString, StyleProperty> custom_properties)
void CSSStyleProperties::set_the_declarations(Vector<StyleProperty> properties, OrderedHashMap<FlyString, StyleProperty> custom_properties)
{
m_properties = convert_declarations_to_specified_order(properties);
m_custom_properties = move(custom_properties);

View file

@ -22,10 +22,10 @@ class WEB_API CSSStyleProperties
GC_DECLARE_ALLOCATOR(CSSStyleProperties);
public:
[[nodiscard]] static GC::Ref<CSSStyleProperties> create(JS::Realm&, Vector<StyleProperty>, HashMap<FlyString, StyleProperty> custom_properties);
[[nodiscard]] static GC::Ref<CSSStyleProperties> create(JS::Realm&, Vector<StyleProperty>, OrderedHashMap<FlyString, StyleProperty> custom_properties);
[[nodiscard]] static GC::Ref<CSSStyleProperties> create_resolved_style(JS::Realm&, Optional<DOM::AbstractElement>);
[[nodiscard]] static GC::Ref<CSSStyleProperties> create_element_inline_style(DOM::AbstractElement, Vector<StyleProperty>, HashMap<FlyString, StyleProperty> custom_properties);
[[nodiscard]] static GC::Ref<CSSStyleProperties> create_element_inline_style(DOM::AbstractElement, Vector<StyleProperty>, OrderedHashMap<FlyString, StyleProperty> custom_properties);
virtual ~CSSStyleProperties() override = default;
virtual void initialize(JS::Realm&) override;
@ -46,7 +46,7 @@ public:
virtual StringView get_property_priority(StringView property_name) const override;
Vector<StyleProperty> const& properties() const { return m_properties; }
HashMap<FlyString, StyleProperty> const& custom_properties() const { return m_custom_properties; }
OrderedHashMap<FlyString, StyleProperty> const& custom_properties() const { return m_custom_properties; }
size_t custom_property_count() const { return m_custom_properties.size(); }
@ -67,7 +67,7 @@ public:
virtual CSSStyleProperties& generated_style_properties_to_css_style_properties() override { return *this; }
private:
CSSStyleProperties(JS::Realm&, Computed, Readonly, Vector<StyleProperty> properties, HashMap<FlyString, StyleProperty> custom_properties, Optional<DOM::AbstractElement>);
CSSStyleProperties(JS::Realm&, Computed, Readonly, Vector<StyleProperty> properties, OrderedHashMap<FlyString, StyleProperty> custom_properties, Optional<DOM::AbstractElement>);
static Vector<StyleProperty> convert_declarations_to_specified_order(Vector<StyleProperty>&);
virtual void visit_edges(Cell::Visitor&) override;
@ -77,12 +77,12 @@ private:
bool set_a_css_declaration(PropertyID, NonnullRefPtr<StyleValue const>, Important);
void empty_the_declarations();
void set_the_declarations(Vector<StyleProperty> properties, HashMap<FlyString, StyleProperty> custom_properties);
void set_the_declarations(Vector<StyleProperty> properties, OrderedHashMap<FlyString, StyleProperty> custom_properties);
void invalidate_owners(DOM::StyleInvalidationReason);
Vector<StyleProperty> m_properties;
HashMap<FlyString, StyleProperty> m_custom_properties;
OrderedHashMap<FlyString, StyleProperty> m_custom_properties;
};
}

View file

@ -99,7 +99,7 @@ public:
struct PropertiesAndCustomProperties {
Vector<StyleProperty> properties;
HashMap<FlyString, StyleProperty> custom_properties;
OrderedHashMap<FlyString, StyleProperty> custom_properties;
};
PropertiesAndCustomProperties parse_as_property_declaration_block();
Vector<Descriptor> parse_as_descriptor_declaration_block(AtRuleID);

View file

@ -849,7 +849,7 @@ void StyleComputer::cascade_declarations(
}
}
static void cascade_custom_properties(DOM::AbstractElement abstract_element, Vector<MatchingRule const*> const& matching_rules, HashMap<FlyString, StyleProperty>& custom_properties)
static void cascade_custom_properties(DOM::AbstractElement abstract_element, Vector<MatchingRule const*> const& matching_rules, OrderedHashMap<FlyString, StyleProperty>& custom_properties)
{
size_t needed_capacity = 0;
for (auto const& matching_rule : matching_rules)
@ -2375,7 +2375,7 @@ GC::Ptr<ComputedProperties> StyleComputer::compute_style_impl(DOM::AbstractEleme
// Resolve all the CSS custom properties ("variables") for this element:
// FIXME: Also resolve !important custom properties, in a second cascade.
if (!abstract_element.pseudo_element().has_value() || pseudo_element_supports_property(*abstract_element.pseudo_element(), PropertyID::Custom)) {
HashMap<FlyString, StyleProperty> custom_properties;
OrderedHashMap<FlyString, StyleProperty> custom_properties;
for (auto& layer : matching_rule_set.author_rules) {
cascade_custom_properties(abstract_element, layer.rules, custom_properties);
}

View file

@ -88,12 +88,12 @@ GC::Ptr<CSS::ComputedProperties const> AbstractElement::computed_properties() co
return m_element->computed_properties(m_pseudo_element);
}
HashMap<FlyString, CSS::StyleProperty> const& AbstractElement::custom_properties() const
OrderedHashMap<FlyString, CSS::StyleProperty> const& AbstractElement::custom_properties() const
{
return m_element->custom_properties(m_pseudo_element);
}
void AbstractElement::set_custom_properties(HashMap<FlyString, CSS::StyleProperty>&& custom_properties)
void AbstractElement::set_custom_properties(OrderedHashMap<FlyString, CSS::StyleProperty>&& custom_properties)
{
m_element->set_custom_properties(m_pseudo_element, move(custom_properties));
}

View file

@ -35,8 +35,8 @@ public:
GC::Ptr<CSS::ComputedProperties const> computed_properties() const;
void set_custom_properties(HashMap<FlyString, CSS::StyleProperty>&& custom_properties);
[[nodiscard]] HashMap<FlyString, CSS::StyleProperty> const& custom_properties() const;
void set_custom_properties(OrderedHashMap<FlyString, CSS::StyleProperty>&& custom_properties);
[[nodiscard]] OrderedHashMap<FlyString, CSS::StyleProperty> const& custom_properties() const;
RefPtr<CSS::StyleValue const> get_custom_property(FlyString const& name) const;
GC::Ptr<CSS::CascadedProperties> cascaded_properties() const;

View file

@ -3041,7 +3041,7 @@ PseudoElement& Element::ensure_pseudo_element(CSS::PseudoElement type) const
return *(m_pseudo_element_data->get(type).value());
}
void Element::set_custom_properties(Optional<CSS::PseudoElement> pseudo_element, HashMap<FlyString, CSS::StyleProperty> custom_properties)
void Element::set_custom_properties(Optional<CSS::PseudoElement> pseudo_element, OrderedHashMap<FlyString, CSS::StyleProperty> custom_properties)
{
if (!pseudo_element.has_value()) {
m_custom_properties = move(custom_properties);
@ -3055,9 +3055,9 @@ void Element::set_custom_properties(Optional<CSS::PseudoElement> pseudo_element,
ensure_pseudo_element(pseudo_element.value()).set_custom_properties(move(custom_properties));
}
HashMap<FlyString, CSS::StyleProperty> const& Element::custom_properties(Optional<CSS::PseudoElement> pseudo_element) const
OrderedHashMap<FlyString, CSS::StyleProperty> const& Element::custom_properties(Optional<CSS::PseudoElement> pseudo_element) const
{
static HashMap<FlyString, CSS::StyleProperty> s_empty_custom_properties;
static OrderedHashMap<FlyString, CSS::StyleProperty> s_empty_custom_properties;
if (!pseudo_element.has_value())
return m_custom_properties;

View file

@ -257,8 +257,8 @@ public:
GC::Ptr<ShadowRoot const> shadow_root() const { return m_shadow_root; }
void set_shadow_root(GC::Ptr<ShadowRoot>);
void set_custom_properties(Optional<CSS::PseudoElement>, HashMap<FlyString, CSS::StyleProperty> custom_properties);
[[nodiscard]] HashMap<FlyString, CSS::StyleProperty> const& custom_properties(Optional<CSS::PseudoElement>) const;
void set_custom_properties(Optional<CSS::PseudoElement>, OrderedHashMap<FlyString, CSS::StyleProperty> custom_properties);
[[nodiscard]] OrderedHashMap<FlyString, CSS::StyleProperty> const& custom_properties(Optional<CSS::PseudoElement>) const;
bool style_uses_attr_css_function() const { return m_style_uses_attr_css_function; }
void set_style_uses_attr_css_function() { m_style_uses_attr_css_function = true; }
@ -586,7 +586,7 @@ private:
GC::Ptr<CSS::CascadedProperties> m_cascaded_properties;
GC::Ptr<CSS::ComputedProperties> m_computed_properties;
HashMap<FlyString, CSS::StyleProperty> m_custom_properties;
OrderedHashMap<FlyString, CSS::StyleProperty> m_custom_properties;
using PseudoElementData = HashMap<CSS::PseudoElement, GC::Ref<PseudoElement>>;
mutable OwnPtr<PseudoElementData> m_pseudo_element_data;

View file

@ -31,8 +31,8 @@ class WEB_API PseudoElement : public JS::Cell {
GC::Ptr<CSS::ComputedProperties> computed_properties() const { return m_computed_properties; }
void set_computed_properties(GC::Ptr<CSS::ComputedProperties> value) { m_computed_properties = value; }
HashMap<FlyString, CSS::StyleProperty> const& custom_properties() const { return m_custom_properties; }
void set_custom_properties(HashMap<FlyString, CSS::StyleProperty> value) { m_custom_properties = move(value); }
OrderedHashMap<FlyString, CSS::StyleProperty> const& custom_properties() const { return m_custom_properties; }
void set_custom_properties(OrderedHashMap<FlyString, CSS::StyleProperty> value) { m_custom_properties = move(value); }
bool has_non_empty_counters_set() const { return m_counters_set; }
Optional<CSS::CountersSet const&> counters_set() const;
@ -48,7 +48,7 @@ private:
GC::Ptr<Layout::NodeWithStyle> m_layout_node;
GC::Ptr<CSS::CascadedProperties> m_cascaded_properties;
GC::Ptr<CSS::ComputedProperties> m_computed_properties;
HashMap<FlyString, CSS::StyleProperty> m_custom_properties;
OrderedHashMap<FlyString, CSS::StyleProperty> m_custom_properties;
OwnPtr<CSS::CountersSet> m_counters_set;
CSSPixelPoint m_scroll_offset {};
};

View file

@ -325,7 +325,7 @@ void ConnectionFromClient::debug_request(u64 page_id, ByteString request, ByteSt
}
if (request == "dump-all-resolved-styles") {
auto dump_style = [](String const& title, Web::CSS::ComputedProperties const& style, HashMap<FlyString, Web::CSS::StyleProperty> const& custom_properties) {
auto dump_style = [](String const& title, Web::CSS::ComputedProperties const& style, OrderedHashMap<FlyString, Web::CSS::StyleProperty> const& custom_properties) {
dbgln("+ {}", title);
for (size_t i = to_underlying(Web::CSS::first_longhand_property_id); i < to_underlying(Web::CSS::last_longhand_property_id); ++i) {
dbgln("| {} = {}", Web::CSS::string_from_property_id(static_cast<Web::CSS::PropertyID>(i)), style.property(static_cast<Web::CSS::PropertyID>(i)).to_string(Web::CSS::SerializationMode::Normal));

View file

@ -2,10 +2,9 @@ Harness status: OK
Found 12 tests
11 Pass
1 Fail
12 Pass
Pass Trailing declarations
Fail Mixed declarations
Pass Mixed declarations
Pass CSSNestedDeclarations.style
Pass Nested group rule
Pass Nested @scope rule