From b1a1f73d90f6477f113c5dfd651ec08419cc40bd Mon Sep 17 00:00:00 2001 From: ldm0 Date: Wed, 7 Oct 2026 05:01:30 +0800 Subject: [PATCH] refactor(cssom): share public computed property enumeration --- .../css_runtime/typed_om/map.rs | 13 +-- .../css_runtime/typed_om/mod.rs | 4 +- moli-renderer-v8/src/css_style.rs | 8 ++ moli-renderer-v8/src/detached_css_style.rs | 8 +- moli-renderer-v8/src/native_bridge/element.rs | 14 ++-- .../src/native_bridge/element/styles.rs | 8 +- .../script_vm/tests/dom_xhr/computed_style.rs | 1 + .../css_computed_map_enumeration.js | 84 +++++++++++++++++++ .../css_computed_map_enumeration.rs | 39 +++++++++ .../src/style_engine/property_metadata.rs | 56 ++++++++----- 10 files changed, 180 insertions(+), 55 deletions(-) create mode 100644 moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/css_computed_map_enumeration.js create mode 100644 moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/css_computed_map_enumeration.rs diff --git a/moli-renderer-v8/src/context_bootstrap/css_runtime/typed_om/map.rs b/moli-renderer-v8/src/context_bootstrap/css_runtime/typed_om/map.rs index d3cf2284bf..6878324b76 100644 --- a/moli-renderer-v8/src/context_bootstrap/css_runtime/typed_om/map.rs +++ b/moli-renderer-v8/src/context_bootstrap/css_runtime/typed_om/map.rs @@ -366,20 +366,9 @@ fn computed_style_property_names<'s>( scope: &mut v8::PinScope<'s, '_>, style: v8::Local<'s, v8::Object>, ) -> Vec { + // Computed CSSStyleDeclaration and Typed OM enumerate one public property list. let mut names = native_bridge::element::computed_style_property_names_from_object(scope, style) .unwrap_or_default(); - for property in css_style_declaration_standard_property_names() { - if native_bridge::element::computed_style_property_is_shorthand(property) - || names.iter().any(|name| name == property) - { - continue; - } - if native_bridge::element::computed_style_property_value_from_object(scope, style, property) - .is_some_and(|value| !value.is_empty()) - { - names.push((*property).to_owned()); - } - } names.sort_by(|left, right| { typed_om_property_sort_category(left) .cmp(&typed_om_property_sort_category(right)) diff --git a/moli-renderer-v8/src/context_bootstrap/css_runtime/typed_om/mod.rs b/moli-renderer-v8/src/context_bootstrap/css_runtime/typed_om/mod.rs index fa0cc5bdac..233a322306 100644 --- a/moli-renderer-v8/src/context_bootstrap/css_runtime/typed_om/mod.rs +++ b/moli-renderer-v8/src/context_bootstrap/css_runtime/typed_om/mod.rs @@ -1,8 +1,6 @@ use crate::{ css_style::top_level_comma_separated_component_values, - detached_css_style::{ - css_style_declaration_exposes_property_name, css_style_declaration_standard_property_names, - }, + detached_css_style::css_style_declaration_exposes_property_name, native_bridge, util::{get_private_object, get_private_value, set_private_value, throw_type_error, v8_string}, webidl, window_host, diff --git a/moli-renderer-v8/src/css_style.rs b/moli-renderer-v8/src/css_style.rs index c3632e33c9..6e0d647890 100644 --- a/moli-renderer-v8/src/css_style.rs +++ b/moli-renderer-v8/src/css_style.rs @@ -12,6 +12,14 @@ pub(crate) use moli_css_parse::{ canonical_style_property_name, escape_top_level_semicolons, serialize_style_property_name, }; +/// Filter Stylo's names to the public CSS property surface. Accessor installation +/// and computed declaration enumeration must use the same exposure policy. +pub(crate) fn stylo_property_is_chromium_exposed(name: &str) -> bool { + !name.starts_with("-moz-") + && !name.starts_with("-x-") + && !matches!(name, "mask-position-x" | "mask-position-y") +} + #[derive(webidl::WebIdlArgs)] #[webidl(prefix = "CSSStyleDeclaration.item")] pub(crate) struct CssStyleDeclarationItemArgs { diff --git a/moli-renderer-v8/src/detached_css_style.rs b/moli-renderer-v8/src/detached_css_style.rs index 82af129141..953032e3b7 100644 --- a/moli-renderer-v8/src/detached_css_style.rs +++ b/moli-renderer-v8/src/detached_css_style.rs @@ -559,19 +559,13 @@ static CSS_STYLE_DECLARATION_STANDARD_PROPERTY_NAMES: LazyLock let mut names = Vec::from(LIGHTWEIGHT_STYLE_PROPERTIES); let mut seen = names.iter().copied().collect::>(); for name in moli_css_parse::stylo_enabled_style_rule_property_names() { - if stylo_property_is_chromium_exposed(name) && seen.insert(name) { + if crate::css_style::stylo_property_is_chromium_exposed(name) && seen.insert(name) { names.push(name); } } names }); -fn stylo_property_is_chromium_exposed(name: &str) -> bool { - !name.starts_with("-moz-") - && !name.starts_with("-x-") - && !matches!(name, "mask-position-x" | "mask-position-y") -} - pub(crate) fn css_style_declaration_exposes_property_name(property: &str) -> bool { CSS_STYLE_DECLARATION_EXPOSED_PROPERTY_NAMES.contains(property) } diff --git a/moli-renderer-v8/src/native_bridge/element.rs b/moli-renderer-v8/src/native_bridge/element.rs index c1957e8b3a..4da669b41b 100644 --- a/moli-renderer-v8/src/native_bridge/element.rs +++ b/moli-renderer-v8/src/native_bridge/element.rs @@ -635,13 +635,13 @@ pub(super) use styles::{ build_style_wrapper_template, node_style_getter_function, node_style_setter_function, }; pub(crate) use styles::{ - computed_style_property_is_shorthand, computed_style_property_names_from_object, - computed_style_property_value_from_object, cssom_style_entry_is_pdb_supplemental_side_entry, - cssom_style_property_affected_names_with_pdb, is_live_style_declaration_object, - live_style_named_property_value, set_live_style_named_property_value, - style_css_text_getter_callback, style_css_text_setter_callback, - style_get_property_priority_callback, style_get_property_value_callback, style_item_callback, - style_length_getter_callback, style_remove_property_callback, style_set_property_callback, + computed_style_property_names_from_object, computed_style_property_value_from_object, + cssom_style_entry_is_pdb_supplemental_side_entry, cssom_style_property_affected_names_with_pdb, + is_live_style_declaration_object, live_style_named_property_value, + set_live_style_named_property_value, style_css_text_getter_callback, + style_css_text_setter_callback, style_get_property_priority_callback, + style_get_property_value_callback, style_item_callback, style_length_getter_callback, + style_remove_property_callback, style_set_property_callback, }; pub(crate) use stylesheets::{ detach_cached_style_sheet_for_element, detach_cached_style_sheet_if_live_stylesheet_changed, diff --git a/moli-renderer-v8/src/native_bridge/element/styles.rs b/moli-renderer-v8/src/native_bridge/element/styles.rs index f977d46f42..ef08baa1ca 100644 --- a/moli-renderer-v8/src/native_bridge/element/styles.rs +++ b/moli-renderer-v8/src/native_bridge/element/styles.rs @@ -20,8 +20,8 @@ pub(crate) use accessors::{ pub(in crate::native_bridge::element) use declaration::style_base_url; pub(crate) use declaration::{ComputedStyleRead, StyleObservation}; use declaration::{ - StyleComputationContext, shorthand_longhands, style_property_count_with_context, - style_property_value_with_context, style_runtime_and_handle_from_object, + StyleComputationContext, style_property_count_with_context, style_property_value_with_context, + style_runtime_and_handle_from_object, }; pub(crate) use declaration::{ StyleMode, active_css_animation_transform_value, css_animation_start_applies, @@ -81,10 +81,6 @@ pub(crate) fn is_live_style_declaration_object( style_runtime_and_handle_from_object(scope, style).is_ok() } -pub(crate) fn computed_style_property_is_shorthand(property: &str) -> bool { - shorthand_longhands(property).is_some() -} - fn style_object_forces_empty_computed<'s>( scope: &mut v8::PinScope<'s, '_>, style: v8::Local<'s, v8::Object>, diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style.rs index fa40bf0f74..d23e1c5673 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style.rs @@ -58,6 +58,7 @@ mod advanced_style_values; mod computed_style_access; mod content_and_invalidation; mod cross_document_and_animations; +mod css_computed_map_enumeration; mod nested_document_invalidation; mod properties_and_selectors; mod stylesheet_and_document_lifecycle; diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/css_computed_map_enumeration.js b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/css_computed_map_enumeration.js new file mode 100644 index 0000000000..783493c2d1 --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/css_computed_map_enumeration.js @@ -0,0 +1,84 @@ +(() => { + const check = (ok, message) => { if (!ok) throw new Error(message); }; + const same = (actual, expected, message) => { + check(actual.length === expected.length && actual.every((value, i) => value === expected[i]), message); + }; + const category = name => name.startsWith('--') ? 2 : name.startsWith('-') ? 1 : 0; + const compare = (a, b) => category(a) - category(b) || (a < b ? -1 : a > b ? 1 : 0); + const parent = document.createElement('div'); + parent.style.setProperty('--inherited', 'parent'); + const element = document.createElement('div'); + element.style.cssText = 'width:25%;overflow-wrap:anywhere;transform:translateX(1px);mask-position:10% 20%;--z:Z;--A:A;--a:a;transition-property:none;transition-duration:1s,2s;'; + parent.append(element); document.body.append(parent); + const map = element.computedStyleMap(); + const names = [...getComputedStyle(element)].sort(compare); + check(names.length > 250, 'complete computed declaration'); + same([...map.keys()], names, 'keys enumerate the computed declaration'); + check(map.size === names.length && new Set(names).size === names.length, 'unique size'); + for (const name of ['font-size', 'direction', 'inline-size', 'mask-position', 'unicode-bidi', 'zoom']) { + check(names.includes(name), 'enabled longhand: ' + name); + } + check(String(map.get('mask-position')) === '10% 20%', 'public mask position value'); + for (const name of ['mask-position-x', 'mask-position-y', '-moz-default-appearance', '-moz-min-font-size-ratio', '-x-text-scale']) { + check(!names.includes(name) && ![...map.keys()].includes(name), 'internal property excluded: ' + name); + } + for (const name of ['background', 'border', 'gap', 'overflow', 'word-wrap', '-webkit-transform']) { + check(!names.includes(name) && ![...map.keys()].includes(name), 'query-only property: ' + name); + check(map.has(name) && map.getAll(name).length > 0, 'query remains supported: ' + name); + } + check(String(map.get('word-wrap')) === String(map.get('overflow-wrap')), 'alias query'); + same([...map.keys()].filter(name => name.startsWith('--')), ['--A', '--a', '--inherited', '--z'], 'custom properties preserve case'); + const entries = [...map.entries()]; + same(entries.map(entry => entry[0]), names, 'entries order'); + same([...map].map(entry => entry[0]), names, 'default iterator order'); + const values = [...map.values()]; + check(values.length === names.length, 'values cardinality'); + entries.forEach(([name, list], i) => { + check(list.length > 0 && list.every(value => value instanceof CSSStyleValue), 'typed entry: ' + name); + same(list.map(String), map.getAll(name).map(String), 'getAll agrees: ' + name); + same(values[i].map(String), list.map(String), 'values agree: ' + name); + }); + const receiver = {}; + const visited = []; + map.forEach(function (list, name, owner) { + check(this === receiver && owner === map && list.length > 0, 'forEach arguments'); + visited.push(name); + }, receiver); + same(visited, names, 'forEach order'); + element.style.setProperty('--B', 'added'); + check(map.size === names.length + 1, 'held map observes added declaration'); + same([...map.keys()].filter(name => name.startsWith('--')), ['--A', '--B', '--a', '--inherited', '--z'], 'new declaration ordered'); + element.style.removeProperty('--B'); + parent.style.removeProperty('--inherited'); + check(map.size === names.length - 1 && ![...map.keys()].includes('--inherited'), 'inherited removal updates enumeration'); + const currentNames = [...map.keys()]; + const nativeComputedStyle = globalThis.getComputedStyle; + const nativeItem = CSSStyleDeclaration.prototype.item; + const nativeGet = CSSStyleDeclaration.prototype.getPropertyValue; + try { + globalThis.getComputedStyle = CSSStyleDeclaration.prototype.item = CSSStyleDeclaration.prototype.getPropertyValue = () => { + throw new Error('author-overridden CSSOM method'); + }; + same([...map.keys()], currentNames, 'enumeration uses native declaration'); + check(map.size === currentNames.length, 'size uses native declaration'); + same([...map.entries()].map(entry => entry[0]), currentNames, 'entry values bypass public CSSOM'); + } finally { + globalThis.getComputedStyle = nativeComputedStyle; + CSSStyleDeclaration.prototype.item = nativeItem; + CSSStyleDeclaration.prototype.getPropertyValue = nativeGet; + } + const other = document.getElementById('child').contentWindow; + const child = other.document.createElement('div'); + child.style.setProperty('--child', 'child'); other.document.body.append(child); + const childMap = child.computedStyleMap(); + const childNames = [...other.getComputedStyle(child)].sort(compare); + same([...StylePropertyMapReadOnly.prototype.keys.call(childMap)], childNames, 'borrowed keys use owner declaration'); + check(childMap.size === childNames.length && childNames.includes('--child'), 'child map size'); + element.remove(); + check(map.size === 0 && [...map.keys()].length === 0 && [...map.entries()].length === 0 && [...map.values()].length === 0, 'disconnected map empty'); + map.forEach(() => { throw new Error('disconnected entry'); }); + parent.append(element); + same([...map.keys()], currentNames, 'reconnected held map'); + parent.remove(); child.remove(); + return true; +})() diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/css_computed_map_enumeration.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/css_computed_map_enumeration.rs new file mode 100644 index 0000000000..8b20253e95 --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/css_computed_map_enumeration.rs @@ -0,0 +1,39 @@ +use super::*; + +#[test] +fn css_computed_map_enumeration_uses_native_declarations_across_all_iterators() { + let mut vm = new_parsed_test_vm( + "https://computed-map-enumeration.test/", + "", + ); + assert_eq!( + vm.eval(include_str!("css_computed_map_enumeration.js")) + .unwrap(), + "true" + ); +} + +#[test] +fn css_computed_map_custom_names_follow_unicode_code_point_order() { + let mut vm = new_parsed_test_vm( + "https://computed-map-unicode.test/", + "", + ); + // The specification and css-houdini-drafts#700 require U+F900 before + // U+1F4A9. The upstream iterable WPT currently asserts the opposite, + // UTF-16 code-unit order; do not adopt that ordering to pass the test. + assert_eq!( + vm.eval( + r#"(() => { + const el = document.createElement('div'); + el.style.cssText = '--💩: astral; --豈: bmp; --z: z; --A: A; --a: a;'; + document.body.append(el); + const names = [...el.computedStyleMap().keys()].filter(name => name.startsWith('--')); + el.remove(); + return JSON.stringify(names); + })()"# + ) + .unwrap(), + r#"["--A","--a","--z","--豈","--💩"]"# + ); +} diff --git a/moli-renderer-v8/src/style_engine/property_metadata.rs b/moli-renderer-v8/src/style_engine/property_metadata.rs index 99afad90f5..b0b46f7660 100644 --- a/moli-renderer-v8/src/style_engine/property_metadata.rs +++ b/moli-renderer-v8/src/style_engine/property_metadata.rs @@ -3,10 +3,10 @@ use std::{cmp::Ordering, sync::LazyLock}; use style::properties::{LonghandId, PropertyId, ShorthandId}; use style::stylesheets::{CssRuleType, CssRuleTypes}; -use crate::css_style::mask_compat_property_name; +use crate::css_style::{mask_compat_property_name, stylo_property_is_chromium_exposed}; struct ComputedLonghandMetadata { - longhands: Box<[LonghandId]>, + longhands: Box<[&'static str]>, first_vendor_index: usize, } @@ -17,9 +17,7 @@ impl ComputedLonghandMetadata { } else { 0..self.first_vendor_index }; - self.longhands[range] - .binary_search_by(|id| id.name().cmp(name)) - .is_ok() + self.longhands[range].binary_search(&name).is_ok() } } @@ -30,22 +28,32 @@ const COMPAT_COMPUTED_QUERY_PROPERTIES: &[&str] = &["animation-range", "mask"]; static COMPUTED_LONGHAND_METADATA: LazyLock = LazyLock::new(|| { super::ensure_stylo_browser_compat_prefs(); - // Match Servo's CSSStyleDeclaration metadata source. The `all` shorthand - // contains every enabled longhand except direction and unicode-bidi. + // Use Stylo's longhand metadata with the same public-property policy as the + // CSSStyleDeclaration accessors. The `all` shorthand contains every enabled + // longhand except direction and unicode-bidi. let mut longhands = ShorthandId::All .longhands() - .filter(|id| stylo_property_is_enabled_for_style_rule(id.name())) + .filter(|id| { + stylo_property_is_enabled_for_style_rule(id.name()) + && stylo_property_is_chromium_exposed(id.name()) + }) + .map(|id| id.name()) .collect::>(); for id in [LonghandId::Direction, LonghandId::UnicodeBidi] { if stylo_property_is_enabled_for_style_rule(id.name()) { - longhands.push(id); + longhands.push(id.name()); } } - longhands.sort_unstable_by(|left, right| canonical_property_order(left.name(), right.name())); - longhands.dedup_by(|left, right| left.name() == right.name()); + // CSS Masking exposes one mask-position longhand. Stylo represents it with + // private x/y longhands and a shorthand, so expose its public name here. + if stylo_property_is_enabled_for_style_rule("mask-position") { + longhands.push("mask-position"); + } + longhands.sort_unstable_by(|left, right| canonical_property_order(left, right)); + longhands.dedup(); let first_vendor_index = longhands .iter() - .position(|property| property.name().starts_with('-')) + .position(|property| property.starts_with('-')) .unwrap_or(longhands.len()); ComputedLonghandMetadata { longhands: longhands.into_boxed_slice(), @@ -66,11 +74,7 @@ pub(crate) fn computed_longhand_count() -> usize { } pub(crate) fn computed_longhand_name_at(index: usize) -> Option<&'static str> { - COMPUTED_LONGHAND_METADATA - .longhands - .get(index) - .copied() - .map(|id| id.name()) + COMPUTED_LONGHAND_METADATA.longhands.get(index).copied() } pub(crate) fn computed_property_is_queryable(name: &str) -> bool { @@ -113,6 +117,7 @@ mod tests { "font-size", "grid-auto-columns", "inline-size", + "mask-position", "object-fit", "overflow-wrap", "pointer-events", @@ -122,10 +127,21 @@ mod tests { ] { assert!(names.contains(&representative), "missing {representative}"); } - for non_longhand in ["animation-range", "margin", "mask", "padding-block", "size"] { + for excluded in [ + "animation-range", + "margin", + "mask", + "padding-block", + "size", + "mask-position-x", + "mask-position-y", + "-moz-default-appearance", + "-moz-min-font-size-ratio", + "-x-text-scale", + ] { assert!( - !names.contains(&non_longhand), - "enumerated non-element longhand {non_longhand}" + !names.contains(&excluded), + "enumerated non-public longhand {excluded}" ); } assert_eq!(