From 3421cd76faf3e8803246ff7a981a79bfed3bddb6 Mon Sep 17 00:00:00 2001 From: Callum Law Date: Tue, 3 Jun 2025 20:16:14 +1200 Subject: [PATCH] LibWeb: Store CSSStyleProperties properties in "specified order" The spec requires us to store properties in their shorthand-expanded form and in the "specified" order, removing duplicates prefering based on "cascading order". We already enforced this in `set_property` but not at creation time (e.g. in stylesheets) or in `set_css_text` (e.g. updating style attribute). This commit enforces the spec requirements at all the relevant points. We no longer include logical properties in the return value of `getComputedStyle` as they are mapped to their physical equivalents in `StyleComputer::for_each_property_expanding_shorthands`, but resolving that requires a relatively large rework of how we handle logical properties, (namely moving when we map them to their physical equivalents from parse time to style computation time). This also exposes two false positive tests in wpt-import/css/cssom/border-shorthand-serialization.html related to us not yet supporting the border-image css property. --- Libraries/LibWeb/CSS/CSSStyleProperties.cpp | 37 ++++++++++++++-- Libraries/LibWeb/CSS/CSSStyleProperties.h | 1 + .../css/css-cascade/all-prop-revert-layer.txt | 44 ++++++++++--------- .../parsing/grid-shorthand-serialization.txt | 28 ++++++------ .../cssom/border-shorthand-serialization.txt | 10 ++--- .../css/cssom/flex-serialization.txt | 6 +-- .../wpt-import/css/cssom/shorthand-values.txt | 8 ++-- 7 files changed, 84 insertions(+), 50 deletions(-) diff --git a/Libraries/LibWeb/CSS/CSSStyleProperties.cpp b/Libraries/LibWeb/CSS/CSSStyleProperties.cpp index a721b6ee43e..0c60ffdac1f 100644 --- a/Libraries/LibWeb/CSS/CSSStyleProperties.cpp +++ b/Libraries/LibWeb/CSS/CSSStyleProperties.cpp @@ -41,7 +41,7 @@ GC::Ref CSSStyleProperties::create(JS::Realm& realm, Vector< // declarations: The declared declarations in the rule, in specified order. // parent CSS rule: The context object. // owner node: Null. - return realm.create(realm, Computed::No, Readonly::No, move(properties), move(custom_properties), OptionalNone {}); + return realm.create(realm, Computed::No, Readonly::No, convert_declarations_to_specified_order(properties), move(custom_properties), OptionalNone {}); } GC::Ref CSSStyleProperties::create_resolved_style(DOM::ElementReference element_reference) @@ -64,7 +64,7 @@ GC::Ref CSSStyleProperties::create_element_inline_style(DOM: // The style attribute must return a CSS declaration block object whose readonly flag is unset, whose parent CSS // rule is null, and whose owner node is the context object. auto& realm = element_reference.element().realm(); - return realm.create(realm, Computed::No, Readonly::No, move(properties), move(custom_properties), move(element_reference)); + return realm.create(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 properties, HashMap custom_properties, Optional owner_node) @@ -75,6 +75,37 @@ CSSStyleProperties::CSSStyleProperties(JS::Realm& realm, Computed computed, Read set_owner_node(move(owner_node)); } +// https://drafts.csswg.org/cssom/#concept-declarations-specified-order +Vector CSSStyleProperties::convert_declarations_to_specified_order(Vector& declarations) +{ + // The specified order for declarations is the same as specified, but with shorthand properties expanded into their + // longhand properties, in canonical order. If a property is specified more than once (after shorthand expansion), only + // the one with greatest cascading order must be represented, at the same relative position as it was specified. + Vector specified_order_declarations; + + for (auto declaration : declarations) { + StyleComputer::for_each_property_expanding_shorthands(declaration.property_id, declaration.value, [&](CSS::PropertyID longhand_id, CSS::CSSStyleValue const& longhand_property_value) { + auto existing_entry_index = specified_order_declarations.find_first_index_if([&](StyleProperty const& existing_declaration) { return existing_declaration.property_id == longhand_id; }); + + if (existing_entry_index.has_value()) { + // If there is an existing entry for this property and it is a higher cascading order than the current entry, skip the current entry. + if (specified_order_declarations[existing_entry_index.value()].important == Important::Yes && declaration.important == Important::No) + return; + + // Otherwise the existing entry has a lower cascading order and is removed. + specified_order_declarations.remove(existing_entry_index.value()); + } + + specified_order_declarations.append(StyleProperty { + .important = declaration.important, + .property_id = longhand_id, + .value = longhand_property_value }); + }); + } + + return specified_order_declarations; +} + void CSSStyleProperties::initialize(JS::Realm& realm) { WEB_SET_PROTOTYPE_FOR_INTERFACE(CSSStyleProperties); @@ -1412,7 +1443,7 @@ void CSSStyleProperties::empty_the_declarations() void CSSStyleProperties::set_the_declarations(Vector properties, HashMap custom_properties) { - m_properties = move(properties); + m_properties = convert_declarations_to_specified_order(properties); m_custom_properties = move(custom_properties); } diff --git a/Libraries/LibWeb/CSS/CSSStyleProperties.h b/Libraries/LibWeb/CSS/CSSStyleProperties.h index 628b56e793e..51c6e0b1da7 100644 --- a/Libraries/LibWeb/CSS/CSSStyleProperties.h +++ b/Libraries/LibWeb/CSS/CSSStyleProperties.h @@ -63,6 +63,7 @@ public: private: CSSStyleProperties(JS::Realm&, Computed, Readonly, Vector properties, HashMap custom_properties, Optional); + static Vector convert_declarations_to_specified_order(Vector&); virtual void visit_edges(Cell::Visitor&) override; diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/css-cascade/all-prop-revert-layer.txt b/Tests/LibWeb/Text/expected/wpt-import/css/css-cascade/all-prop-revert-layer.txt index ac9664b58da..32fbb15ecca 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/css-cascade/all-prop-revert-layer.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/css-cascade/all-prop-revert-layer.txt @@ -1,9 +1,9 @@ Harness status: OK -Found 201 tests +Found 203 tests -191 Pass -10 Fail +199 Pass +4 Fail Pass accent-color Pass border-collapse Pass border-spacing @@ -58,8 +58,10 @@ Pass text-indent Pass text-justify Pass text-shadow Pass text-transform +Pass text-wrap-mode Pass text-wrap-style Pass visibility +Pass white-space-collapse Pass word-break Pass word-spacing Pass word-wrap @@ -82,9 +84,26 @@ Pass background-blend-mode Pass background-clip Pass background-color Pass background-origin +Pass background-position-x +Pass background-position-y Pass background-repeat Pass background-size -Fail block-size +Pass border-bottom-color +Pass border-bottom-left-radius +Pass border-bottom-right-radius +Pass border-bottom-style +Pass border-bottom-width +Pass border-left-color +Pass border-left-style +Pass border-left-width +Pass border-right-color +Pass border-right-style +Pass border-right-width +Pass border-top-color +Pass border-top-left-radius +Pass border-top-right-radius +Pass border-top-style +Pass border-top-width Pass bottom Pass box-shadow Pass box-sizing @@ -121,33 +140,20 @@ Pass grid-template-areas Pass grid-template-columns Pass grid-template-rows Fail height -Fail inline-size -Pass inset-block-end -Pass inset-block-start -Pass inset-inline-end -Pass inset-inline-start Pass isolation Pass justify-content Pass justify-items Pass justify-self Pass left -Pass margin-block-end -Pass margin-block-start Pass margin-bottom -Pass margin-inline-end -Pass margin-inline-start Pass margin-left Pass margin-right Pass margin-top Pass mask-image Pass mask-type -Fail max-block-size Pass max-height -Fail max-inline-size Pass max-width -Fail min-block-size Pass min-height -Fail min-inline-size Pass min-width Pass mix-blend-mode Pass object-fit @@ -160,11 +166,7 @@ Pass outline-style Pass outline-width Pass overflow-x Pass overflow-y -Pass padding-block-end -Pass padding-block-start Pass padding-bottom -Pass padding-inline-end -Pass padding-inline-start Pass padding-left Pass padding-right Pass padding-top diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/css-grid/parsing/grid-shorthand-serialization.txt b/Tests/LibWeb/Text/expected/wpt-import/css/css-grid/parsing/grid-shorthand-serialization.txt index 5e239741841..f10b9b76f3a 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/css-grid/parsing/grid-shorthand-serialization.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/css-grid/parsing/grid-shorthand-serialization.txt @@ -2,8 +2,8 @@ Harness status: OK Found 121 tests -35 Pass -86 Fail +47 Pass +74 Fail Fail e.style.cssText = grid: auto-flow auto / 100px 100px should set grid Fail e.style.cssText = grid: auto-flow auto / 100px 100px should set grid-template-areas Pass e.style.cssText = grid: auto-flow auto / 100px 100px; grid-template-areas: "one two" "three four" should set grid @@ -17,24 +17,24 @@ Fail cssText ('grid: 30px 40px / 50px 60px; grid-auto-flow: column') must contai Fail cssText ('grid: 30px 40px / 50px 60px; grid-auto-flow: column') must contain 'grid-auto-rows: auto;' in its serialization Fail cssText ('grid: 30px 40px / 50px 60px; grid-auto-flow: column') must contain 'grid-auto-columns: auto;' in its serialization Pass cssText ('grid: 30px 40px / 50px 60px; grid-auto-flow: column') must contain 'grid-auto-flow: column;' in its serialization -Fail e.style.cssText = grid: auto-flow / 10px; grid-template-rows: 20px should set grid -Fail e.style.cssText = grid: auto-flow / 10px; grid-template-rows: 20px should set grid-template -Fail cssText ('grid: auto-flow / 10px; grid-template-rows: 20px') must contain 'grid: 20px / 10px;' in its serialization -Fail e.style.cssText = grid: auto-flow / 10px; grid-template-rows: repeat(2, 20px) should set grid -Fail e.style.cssText = grid: auto-flow / 10px; grid-template-rows: repeat(2, 20px) should set grid-template-rows -Fail e.style.cssText = grid: auto-flow / 10px; grid-template-rows: repeat(2, 20px) repeat(3, 30px) should set grid -Fail e.style.cssText = grid: auto-flow / 10px; grid-template-rows: repeat(2, 20px) repeat(3, 30px) should set grid-template-rows +Pass e.style.cssText = grid: auto-flow / 10px; grid-template-rows: 20px should set grid +Pass e.style.cssText = grid: auto-flow / 10px; grid-template-rows: 20px should set grid-template +Pass cssText ('grid: auto-flow / 10px; grid-template-rows: 20px') must contain 'grid: 20px / 10px;' in its serialization +Pass e.style.cssText = grid: auto-flow / 10px; grid-template-rows: repeat(2, 20px) should set grid +Pass e.style.cssText = grid: auto-flow / 10px; grid-template-rows: repeat(2, 20px) should set grid-template-rows +Pass e.style.cssText = grid: auto-flow / 10px; grid-template-rows: repeat(2, 20px) repeat(3, 30px) should set grid +Pass e.style.cssText = grid: auto-flow / 10px; grid-template-rows: repeat(2, 20px) repeat(3, 30px) should set grid-template-rows Fail e.style.cssText = grid: auto-flow / 10px; grid-template-rows: repeat(auto-fill, 20px) should set grid Fail e.style.cssText = grid: auto-flow / 10px; grid-template-rows: repeat(auto-fill, 20px) should set grid-template-rows Fail e.style.cssText = grid: auto-flow / 10px; grid-template-rows: repeat(auto-fit, 20px) should set grid Fail e.style.cssText = grid: auto-flow / 10px; grid-template-rows: repeat(auto-fit, 20px) should set grid-template-rows -Fail e.style.cssText = grid: 10px / auto; grid-template-columns: 20px should set grid -Fail e.style.cssText = grid: 10px / auto; grid-template-columns: 20px should set grid-template -Fail cssText ('grid: 10px / auto; grid-template-columns: 20px') must contain 'grid: 10px / 20px;' in its serialization +Pass e.style.cssText = grid: 10px / auto; grid-template-columns: 20px should set grid +Pass e.style.cssText = grid: 10px / auto; grid-template-columns: 20px should set grid-template +Pass cssText ('grid: 10px / auto; grid-template-columns: 20px') must contain 'grid: 10px / 20px;' in its serialization Fail e.style.cssText = grid: auto-flow / 10px; grid-template-columns: repeat(2, 20px) should set grid -Fail e.style.cssText = grid: auto-flow / 10px; grid-template-columns: repeat(2, 20px) should set grid-template-columns +Pass e.style.cssText = grid: auto-flow / 10px; grid-template-columns: repeat(2, 20px) should set grid-template-columns Fail e.style.cssText = grid: auto-flow / 10px; grid-template-columns: repeat(2, 20px) repeat(3, 30px) should set grid -Fail e.style.cssText = grid: auto-flow / 10px; grid-template-columns: repeat(2, 20px) repeat(3, 30px) should set grid-template-columns +Pass e.style.cssText = grid: auto-flow / 10px; grid-template-columns: repeat(2, 20px) repeat(3, 30px) should set grid-template-columns Fail e.style.cssText = grid: auto-flow / 10px; grid-template-columns: repeat(auto-fill, 20px) should set grid Fail e.style.cssText = grid: auto-flow / 10px; grid-template-columns: repeat(auto-fill, 20px) should set grid-template-columns Fail e.style.cssText = grid: auto-flow / 10px; grid-template-columns: repeat(auto-fit, 20px) should set grid diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/cssom/border-shorthand-serialization.txt b/Tests/LibWeb/Text/expected/wpt-import/css/cssom/border-shorthand-serialization.txt index 079b8f7bb94..80285c69934 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/cssom/border-shorthand-serialization.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/cssom/border-shorthand-serialization.txt @@ -2,8 +2,8 @@ Harness status: OK Found 3 tests -2 Pass -1 Fail -Pass Declaration with border longhands is not serialized to a border shorthand declaration. -Pass Declaration with border longhands and border-image is not serialized to a border shorthand declaration. -Fail Border shorthand is serialized correctly if all border-image-* are set to their initial specified values. \ No newline at end of file +1 Pass +2 Fail +Fail Declaration with border longhands is not serialized to a border shorthand declaration. +Fail Declaration with border longhands and border-image is not serialized to a border shorthand declaration. +Pass Border shorthand is serialized correctly if all border-image-* are set to their initial specified values. \ No newline at end of file diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/cssom/flex-serialization.txt b/Tests/LibWeb/Text/expected/wpt-import/css/cssom/flex-serialization.txt index 17b3336f783..932fcb30c27 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/cssom/flex-serialization.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/cssom/flex-serialization.txt @@ -2,10 +2,10 @@ Harness status: OK Found 5 tests -1 Pass -4 Fail +2 Pass +3 Fail Pass Single value flex with CSS-wide keyword should serialize correctly. Fail Single value flex with non-CSS-wide value should serialize correctly. -Fail Multiple values flex with CSS-wide keyword should serialize correctly. +Pass Multiple values flex with CSS-wide keyword should serialize correctly. Fail Multiple values flex with CSS-wide keywords and non-CSS-wide value should serialize correctly. Fail Multiple values flex with CSS-wide and two non-CSS-wide-keyword values should serialize correctly. \ No newline at end of file diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/cssom/shorthand-values.txt b/Tests/LibWeb/Text/expected/wpt-import/css/cssom/shorthand-values.txt index da20b938dd2..cdccee56f3f 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/cssom/shorthand-values.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/cssom/shorthand-values.txt @@ -2,9 +2,9 @@ Harness status: OK Found 20 tests -13 Pass -7 Fail -Fail The serialization of border: 1px; border-top: 1px; should be canonical. +15 Pass +5 Fail +Pass The serialization of border: 1px; border-top: 1px; should be canonical. Pass The serialization of border: 1px solid red; should be canonical. Pass The serialization of border: 1px red; should be canonical. Pass The serialization of border: red; should be canonical. @@ -14,7 +14,7 @@ Fail The serialization of border-top: 1px; border-right: 2px; border-bottom: 3px Fail The serialization of border: 1px; border-top: 2px; should be canonical. Fail The serialization of border: 1px; border-top: 1px !important; should be canonical. Fail The serialization of border: 1px; border-top-color: red; should be canonical. -Fail The serialization of border: solid; border-style: dotted should be canonical. +Pass The serialization of border: solid; border-style: dotted should be canonical. Pass The serialization of border-width: 1px; should be canonical. Pass The serialization of overflow-x: scroll; overflow-y: hidden; should be canonical. Pass The serialization of overflow-x: scroll; overflow-y: scroll; should be canonical.