mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-07-30 12:49:19 +00:00
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.
This commit is contained in:
Callum Law
Sam Atkins
parent
048a0c9106
commit
3421cd76fa
Notes:
github-actions[bot]
2025-06-09 09:44:59 +00:00
Author: https://github.com/Calme1709
Commit: 3421cd76fa
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/4990
Reviewed-by: https://github.com/AtkinsSJ ✅
Reviewed-by: https://github.com/konradekk
7 changed files with 84 additions and 50 deletions
|
|
@ -41,7 +41,7 @@ GC::Ref<CSSStyleProperties> 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<CSSStyleProperties>(realm, Computed::No, Readonly::No, move(properties), move(custom_properties), OptionalNone {});
|
||||
return realm.create<CSSStyleProperties>(realm, Computed::No, Readonly::No, convert_declarations_to_specified_order(properties), move(custom_properties), OptionalNone {});
|
||||
}
|
||||
|
||||
GC::Ref<CSSStyleProperties> CSSStyleProperties::create_resolved_style(DOM::ElementReference element_reference)
|
||||
|
|
@ -64,7 +64,7 @@ GC::Ref<CSSStyleProperties> 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<CSSStyleProperties>(realm, Computed::No, Readonly::No, move(properties), move(custom_properties), move(element_reference));
|
||||
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::ElementReference> 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<StyleProperty> CSSStyleProperties::convert_declarations_to_specified_order(Vector<StyleProperty>& 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<StyleProperty> 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<StyleProperty> properties, HashMap<FlyString, StyleProperty> custom_properties)
|
||||
{
|
||||
m_properties = move(properties);
|
||||
m_properties = convert_declarations_to_specified_order(properties);
|
||||
m_custom_properties = move(custom_properties);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -63,6 +63,7 @@ public:
|
|||
|
||||
private:
|
||||
CSSStyleProperties(JS::Realm&, Computed, Readonly, Vector<StyleProperty> properties, HashMap<FlyString, StyleProperty> custom_properties, Optional<DOM::ElementReference>);
|
||||
static Vector<StyleProperty> convert_declarations_to_specified_order(Vector<StyleProperty>&);
|
||||
|
||||
virtual void visit_edges(Cell::Visitor&) override;
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
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.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue