fix(cssom): preserve stylesheet EOF-open var values

This commit is contained in:
ldm0
2026-10-11 18:04:09 +08:00
parent 5d5f17115c
commit d0d608580f
9 changed files with 363 additions and 8 deletions
+137
View File
@@ -40,6 +40,112 @@ pub fn parse_declaration_list(
.collect()
}
/// Recover the declaration-list serialization for a stylesheet that ended in
/// one valid, unclosed `var()` reference.
///
/// Stylo accepts the EOF-recovered rule but currently appends the missing `)`
/// to its stored value. This helper only removes that character when the raw
/// final declaration and Stylo's serialized final declaration otherwise match
/// exactly. The caller is responsible for associating the result with the
/// rightmost native style rule.
pub fn recover_stylesheet_eof_open_var_declaration_text(
stylesheet_text: &str,
serialized_declaration_text: &str,
) -> Option<String> {
let options = DeclarationParseOptions {
canonicalize_property_name: false,
unescape_value_semicolons: true,
preserve_empty_values: false,
};
let serialized_declaration = parse_declaration_list(serialized_declaration_text, options)
.into_iter()
.last()?;
for block_start in eof_open_curly_block_starts(stylesheet_text) {
let Some(source_declaration) =
parse_declaration_list(&stylesheet_text[block_start + 1..], options)
.into_iter()
.last()
else {
continue;
};
if !crate::value::css_value_is_eof_open_var_function(&source_declaration.value)
|| crate::canonical_style_property_name(&source_declaration.name)
!= crate::canonical_style_property_name(&serialized_declaration.name)
|| source_declaration.important != serialized_declaration.important
|| serialized_declaration.value != format!("{})", source_declaration.value)
{
continue;
}
let value_start = serialized_declaration_text.rfind(&serialized_declaration.value)?;
let closing_parenthesis = value_start + serialized_declaration.value.len() - 1;
if serialized_declaration_text.as_bytes()[closing_parenthesis] != b')'
|| !serialized_declaration_suffix_matches(
&serialized_declaration_text[closing_parenthesis + 1..],
serialized_declaration.important,
)
{
continue;
}
let mut recovered = String::with_capacity(serialized_declaration_text.len() - 1);
recovered.push_str(&serialized_declaration_text[..closing_parenthesis]);
recovered.push_str(&serialized_declaration_text[closing_parenthesis + 1..]);
return Some(recovered);
}
None
}
fn serialized_declaration_suffix_matches(suffix: &str, important: bool) -> bool {
let suffix = suffix.trim();
if important {
suffix.eq_ignore_ascii_case("!important;")
} else {
suffix == ";"
}
}
fn eof_open_curly_block_starts(css_text: &str) -> Vec<usize> {
let mut open_blocks = Vec::new();
let mut input = ParserInput::new(css_text);
let mut input = Parser::new(&mut input);
collect_eof_open_curly_block_starts(&mut input, &mut open_blocks);
open_blocks
}
fn collect_eof_open_curly_block_starts<'i, 't>(
input: &mut Parser<'i, 't>,
open_blocks: &mut Vec<usize>,
) {
loop {
let token_start = input.position();
let Ok(token) = input.next_including_whitespace_and_comments().cloned() else {
break;
};
let is_curly = matches!(token, cssparser::Token::CurlyBracketBlock);
if !matches!(
token,
cssparser::Token::Function(_)
| cssparser::Token::ParenthesisBlock
| cssparser::Token::SquareBracketBlock
| cssparser::Token::CurlyBracketBlock
) {
continue;
}
let mut nested_end = None;
let nested: Result<(), ParseError<'i, ()>> = input.parse_nested_block(|input| {
collect_eof_open_curly_block_starts(input, open_blocks);
nested_end = Some(input.position());
Ok(())
});
if nested.is_ok() && is_curly && nested_end == Some(input.position()) {
open_blocks.push(token_start.byte_index());
}
}
}
struct DeclarationListParser {
options: DeclarationParseOptions,
}
@@ -178,6 +284,37 @@ mod tests {
assert_eq!(entries[0].value, "var(--prop");
}
#[test]
fn stylesheet_eof_open_var_recovery_removes_only_stylos_inserted_parenthesis() {
let source = r#"
.ignored::before { content: "}"; }
/* ignored { */
@media all { .target { color: red; width: var(--prop
"#;
assert_eq!(
super::recover_stylesheet_eof_open_var_declaration_text(
source,
"color: red; width: var(--prop);",
)
.as_deref(),
Some("color: red; width: var(--prop;")
);
assert_eq!(
super::recover_stylesheet_eof_open_var_declaration_text(
".target { width: var(--prop)",
"width: var(--prop);",
),
None
);
assert_eq!(
super::recover_stylesheet_eof_open_var_declaration_text(
".target { width: var(--prop",
"width: var(--other);",
),
None
);
}
#[test]
fn declaration_parser_canonicalizes_when_requested() {
let entries = parse_declaration_list(
+8 -5
View File
@@ -18,7 +18,10 @@ pub use color::{
css_system_color_srgb, parse_css_color_scheme_flags, parse_css_color_to_opaque_srgb_hex,
parse_css_color_to_srgb_bytes,
};
pub use declaration::{CssDeclaration, DeclarationParseOptions, parse_declaration_list};
pub use declaration::{
CssDeclaration, DeclarationParseOptions, parse_declaration_list,
recover_stylesheet_eof_open_var_declaration_text,
};
pub use font_face::{
CssFontFace, font_load_query_contains_css_wide_keyword, font_load_query_family,
font_load_query_is_valid, normalize_font_face_src, parse_font_faces,
@@ -76,10 +79,10 @@ pub use util::{
unescape_top_level_semicolons, unquote_css_string,
};
pub use value::{
css_declaration_value_has_valid_env_functions, css_value_may_contain_env_function,
css_value_may_contain_var_function, normalize_css_variable_specified_value,
normalize_cssom_component_value_serialization, normalize_custom_property_specified_value,
serialize_component_values_single_line,
css_declaration_value_has_valid_env_functions, css_value_is_eof_open_var_function,
css_value_may_contain_env_function, css_value_may_contain_var_function,
normalize_css_variable_specified_value, normalize_cssom_component_value_serialization,
normalize_custom_property_specified_value, serialize_component_values_single_line,
};
/// Returns the non-custom CSS properties that the pinned Stylo world currently
+18
View File
@@ -33,6 +33,15 @@ pub fn css_value_may_contain_var_function(value: &str) -> bool {
.any(|window| matches!(window, [b'v' | b'V', b'a' | b'A', b'r' | b'R', b'(']))
}
/// Whether `value` is a valid standalone `var()` reference whose final
/// parenthesis was omitted at the end of a CSS input fragment.
///
/// CSSOM preserves that omission in the specified value even though Stylo's
/// declaration storage currently appends the missing parenthesis internally.
pub fn css_value_is_eof_open_var_function(value: &str) -> bool {
recover_left_open_css_var_function(value).is_some()
}
/// This validates var() token structure before substitution, but does not decide
/// ordinary property grammar.
pub(crate) fn css_declaration_value_has_valid_var_functions(value: &str) -> bool {
@@ -559,6 +568,15 @@ mod tests {
);
}
#[test]
fn eof_open_var_function_detection_is_narrow() {
assert!(super::css_value_is_eof_open_var_function("var(--x"));
assert!(super::css_value_is_eof_open_var_function("VAR(--x;"));
assert!(!super::css_value_is_eof_open_var_function("var(--x)"));
assert!(!super::css_value_is_eof_open_var_function("1px var(--x"));
assert!(!super::css_value_is_eof_open_var_function("var(-x"));
}
#[test]
fn custom_property_specified_value_normalizer_preserves_ident_var_names() {
assert_eq!(
+90 -2
View File
@@ -496,11 +496,20 @@ pub(crate) struct StylesheetRuleWrapperBinding {
path: Vec<usize>,
rule: NativeStylesheetRule,
shared_lock: SharedRwLock,
source_declaration_override: LiveStylesheetSourceDeclarationOverrideState,
detached_source_declaration_override: Option<String>,
}
impl StylesheetRuleWrapperBinding {
fn snapshot(&self) -> CssRuleSnapshot {
self.rule.snapshot(&self.shared_lock)
let mut snapshot = self.rule.snapshot(&self.shared_lock);
if let Some(declaration_text) = self.source_declaration_override() {
snapshot.declaration_text = Some(declaration_text.clone());
if let Some(selector_text) = snapshot.selector_text.as_deref() {
snapshot.css_text = format!("{selector_text} {{ {declaration_text} }}");
}
}
snapshot
}
pub(crate) fn rule_type(&self) -> CssRuleType {
@@ -508,9 +517,25 @@ impl StylesheetRuleWrapperBinding {
}
pub(crate) fn css_text(&self) -> String {
if let Some(declaration_text) = self.source_declaration_override()
&& let Some(selector_text) = self.rule.style_selector_text(&self.shared_lock)
{
return format!("{selector_text} {{ {declaration_text} }}");
}
self.rule.css_text(&self.shared_lock)
}
fn source_declaration_override(&self) -> Option<String> {
if self.stylesheet_id.is_none() {
return self.detached_source_declaration_override.clone();
}
let source_override = self.source_declaration_override.borrow();
source_override
.as_ref()
.filter(|source_override| source_override.rule_path == self.path)
.map(|source_override| source_override.declaration_text.clone())
}
pub(crate) fn style_selector_text(&self) -> Option<String> {
self.rule.style_selector_text(&self.shared_lock)
}
@@ -568,6 +593,7 @@ impl StylesheetRuleWrapperBinding {
}
fn mark_detached(&mut self) {
self.detached_source_declaration_override = self.source_declaration_override();
self.stylesheet_id = None;
self.path.clear();
}
@@ -967,6 +993,15 @@ struct StylesheetContentsCacheKey {
allow_import_rules: bool,
}
#[derive(Clone, Debug, Eq, PartialEq)]
struct LiveStylesheetSourceDeclarationOverride {
rule_path: Vec<usize>,
declaration_text: String,
}
type LiveStylesheetSourceDeclarationOverrideState =
Rc<RefCell<Option<LiveStylesheetSourceDeclarationOverride>>>;
impl StylesheetContentsCacheKey {
fn new(
css_text: &str,
@@ -993,6 +1028,7 @@ pub(crate) struct SharedStylesheetContents {
base_url: url::Url,
quirks_mode: QuirksMode,
allow_import_rules: AllowImportRules,
source_declaration_override: Option<LiveStylesheetSourceDeclarationOverride>,
}
/// Renderer-owned identity for one authoritative parsed stylesheet.
@@ -1026,6 +1062,7 @@ pub(crate) struct LiveStylesheet {
next_font_face_rule_identity: Cell<u64>,
font_face_rule_identities: RefCell<Vec<FontFaceRuleIdentity>>,
shared_initial_contents: RefCell<Option<StdArc<SharedStylesheetContents>>>,
source_declaration_override: LiveStylesheetSourceDeclarationOverrideState,
rule_wrapper_leases: RefCell<
HashMap<
StylesheetRuleWrapperLeaseId,
@@ -1226,6 +1263,8 @@ impl LiveStylesheet {
quirks_mode,
allow_import_rules,
));
let source_declaration_override =
stylesheet_eof_open_var_declaration_override(&stylesheet, css_text);
Self::from_stylesheet(
id,
stylesheet,
@@ -1234,6 +1273,7 @@ impl LiveStylesheet {
allow_import_rules,
runtime_state_kind,
None,
source_declaration_override,
)
}
@@ -1242,6 +1282,7 @@ impl LiveStylesheet {
shared_lock: SharedRwLock,
shared_contents: StdArc<SharedStylesheetContents>,
) -> Self {
let source_declaration_override = shared_contents.source_declaration_override.clone();
let media = ServoArc::new(shared_lock.wrap(MediaList::empty()));
let stylesheet = ServoArc::new(Stylesheet {
contents: shared_lock.wrap(shared_contents.contents.clone()),
@@ -1257,6 +1298,7 @@ impl LiveStylesheet {
shared_contents.allow_import_rules,
LiveStylesheetRuntimeStateKind::Cascade,
Some(shared_contents),
source_declaration_override,
)
}
@@ -1268,6 +1310,7 @@ impl LiveStylesheet {
allow_import_rules: AllowImportRules,
runtime_state_kind: LiveStylesheetRuntimeStateKind,
shared_initial_contents: Option<StdArc<SharedStylesheetContents>>,
source_declaration_override: Option<LiveStylesheetSourceDeclarationOverride>,
) -> Self {
let cssom_runtime_state = match runtime_state_kind {
LiveStylesheetRuntimeStateKind::Cascade => LiveStylesheetCssomRuntimeState::Cascade,
@@ -1298,6 +1341,7 @@ impl LiveStylesheet {
next_font_face_rule_identity: Cell::new(0),
font_face_rule_identities: RefCell::new(Vec::new()),
shared_initial_contents: RefCell::new(shared_initial_contents),
source_declaration_override: Rc::new(RefCell::new(source_declaration_override)),
rule_wrapper_leases: RefCell::new(HashMap::new()),
};
stylesheet.reconcile_import_edges();
@@ -1323,6 +1367,7 @@ impl LiveStylesheet {
base_url: self.base_url.clone(),
quirks_mode: self.quirks_mode,
allow_import_rules: self.allow_import_rules,
source_declaration_override: self.source_declaration_override.borrow().clone(),
});
*self.shared_initial_contents.borrow_mut() = Some(StdArc::clone(&shared_contents));
shared_contents
@@ -1500,9 +1545,16 @@ impl LiveStylesheet {
Some(match native_rule {
NativeStylesheetRule::Css(CssRule::Style(rule)) => {
let rule = rule.read_with(&guard);
let declaration_text = self
.source_declaration_override
.borrow()
.as_ref()
.filter(|source_override| source_override.rule_path == path)
.map(|source_override| source_override.declaration_text.clone())
.unwrap_or_else(|| serialize_declaration_block(&rule.block, &guard));
LiveStylesheetRuleWrapperSeed::Style {
selector_text: cssparser::ToCss::to_css_string(&rule.selectors),
declaration_text: serialize_declaration_block(&rule.block, &guard),
declaration_text,
}
}
NativeStylesheetRule::Keyframe(rule) => {
@@ -1934,6 +1986,42 @@ impl LiveStylesheet {
}
}
fn stylesheet_eof_open_var_declaration_override(
stylesheet: &ServoArc<Stylesheet>,
css_text: &str,
) -> Option<LiveStylesheetSourceDeclarationOverride> {
let guard = stylesheet.shared_lock.read();
let contents = stylesheet.contents.read_with(&guard);
let rules = contents.rules.read_with(&guard);
let mut rule_path = vec![rules.0.len().checked_sub(1)?];
let mut rule = rules.0.last()?.clone();
loop {
let children = rule.children(&guard);
let Some(child_index) = children.len().checked_sub(1) else {
break;
};
rule_path.push(child_index);
rule = children[child_index].clone();
}
let CssRule::Style(rule) = rule else {
return None;
};
let serialized_declaration_text = {
let rule = rule.read_with(&guard);
serialize_declaration_block(&rule.block, &guard)
};
let declaration_text = moli_css_parse::recover_stylesheet_eof_open_var_declaration_text(
css_text,
&serialized_declaration_text,
)?;
Some(LiveStylesheetSourceDeclarationOverride {
rule_path,
declaration_text,
})
}
mod import_graph;
mod mutation;
mod registry;
@@ -1189,6 +1189,8 @@ impl LiveStylesheet {
self.quirks_mode,
self.allow_import_rules,
));
let source_declaration_override =
stylesheet_eof_open_var_declaration_override(&replacement, css_text);
let replacement_contents = {
let guard = replacement.shared_lock.read();
replacement.contents.read_with(&guard).clone()
@@ -1205,9 +1207,11 @@ impl LiveStylesheet {
self.reconcile_import_edges();
self.font_face_rule_identities.borrow_mut().clear();
self.note_contents_mutation();
*self.source_declaration_override.borrow_mut() = source_declaration_override;
}
pub(crate) fn note_contents_mutation(&self) {
self.source_declaration_override.borrow_mut().take();
self.contents_revision
.set(self.contents_revision.get().saturating_add(1));
self.note_cascade_mutation();
@@ -192,6 +192,7 @@ impl LiveStylesheetRegistry {
allow_import_rules,
LiveStylesheetRuntimeStateKind::Cascade,
None,
None,
));
self.entries
.borrow_mut()
@@ -525,6 +526,8 @@ impl LiveStylesheetRegistry {
path,
rule,
shared_lock: stylesheet.stylesheet.shared_lock.clone(),
source_declaration_override: Rc::clone(&stylesheet.source_declaration_override),
detached_source_declaration_override: None,
};
if let Some(id) = existing_id {
@@ -19,6 +19,67 @@ fn serialized_stylesheet_text(stylesheet: &LiveStylesheetRef) -> StdArc<str> {
.serialized_css_text()
}
fn style_rule_seed_declaration_text(stylesheet: &LiveStylesheet, path: &[usize]) -> String {
let Some(LiveStylesheetRuleWrapperSeed::Style {
declaration_text, ..
}) = stylesheet.rule_wrapper_seed_at_path(path)
else {
panic!("style rule seed should resolve at {path:?}");
};
declaration_text
}
#[test]
fn eof_open_var_cssom_override_is_shared_and_cleared_by_native_mutation() {
let registry = LiveStylesheetRegistry::default();
let shared_lock = SharedRwLock::new();
let base_url = url::Url::parse("https://example.test/styles/open-var.css").unwrap();
let source = ".target { color: red; width: var(--prop";
let first = registry.create_inline_with_shared_initial_contents(
source,
base_url.clone(),
QuirksMode::NoQuirks,
shared_lock.clone(),
);
let second = registry.create_inline_with_shared_initial_contents(
source,
base_url,
QuirksMode::NoQuirks,
shared_lock,
);
assert_eq!(
style_rule_seed_declaration_text(&first, &[0]),
"color: red; width: var(--prop;"
);
assert_eq!(
style_rule_seed_declaration_text(&second, &[0]),
"color: red; width: var(--prop;"
);
assert_eq!(
native_rule_css_text(&first, &[0]),
".target { color: red; width: var(--prop); }"
);
first
.set_style_rule_declarations(&[0], "width: var(--updated);")
.unwrap();
assert_eq!(
style_rule_seed_declaration_text(&first, &[0]),
"width: var(--updated);"
);
assert_eq!(
style_rule_seed_declaration_text(&second, &[0]),
"color: red; width: var(--prop;"
);
first.replace_from_text(".replacement { height: var(--replacement");
assert_eq!(
style_rule_seed_declaration_text(&first, &[0]),
"height: var(--replacement;"
);
}
#[test]
fn native_import_edges_own_distinct_children_and_propagate_cascade_changes() {
let registry = LiveStylesheetRegistry::default();
@@ -206,6 +206,9 @@ pub(super) fn style_entries_affecting_property(
}
pub(super) fn cssom_style_property_write_uses_pdb(name: &str, value: &str) -> bool {
if moli_css_parse::css_value_is_eof_open_var_function(value) {
return false;
}
let name = canonical_style_property_name(name);
if crate::detached_css_style::css_style_declaration_is_shell_property(&name) {
return false;
@@ -66,7 +66,7 @@ fn css_variable_specified_values_preserve_cssom_shorthand_boundaries() {
assert_eq!(
result,
"margin: var(--prop);,var(--prop),|margin-right: ; margin-bottom: ; margin-left: ; margin-top: 10px;,,,10px|var(--prop),|,,var(--prop) /* keep */ var(--prop),color: var(--prop) /* keep */ var(--prop);|var(--open)|3px,1px,1px,1px"
"margin: var(--prop);,var(--prop),|margin-right: ; margin-bottom: ; margin-left: ; margin-top: 10px;,,,10px|var(--prop),|,,var(--prop) /* keep */ var(--prop),color: var(--prop) /* keep */ var(--prop);|var(--open|3px,1px,1px,1px"
);
}
#[test]
@@ -3189,3 +3189,41 @@ fn webkit_text_stroke_shorthand_uses_pdb_backing_across_cssom_surfaces() {
assert_eq!(result, "PASS");
}
#[test]
fn stylesheet_eof_open_var_preserves_cssom_text_views() {
let mut vm = new_storage_test_vm("https://css-var-stylesheet-eof.test/");
let result = vm
.eval(
r#"
(() => {
const style = document.createElement('style');
style.textContent = 'div { width: var(--open';
(document.head || document.documentElement || document).appendChild(style);
const rule = style.sheet.cssRules[0];
const constructed = new CSSStyleSheet();
constructed.replaceSync('span { height: var(--retained');
const retained = constructed.cssRules[0];
constructed.replaceSync('span { height: 1px; }');
constructed.cssRules[0].style.width = 'var(--written';
return [
rule.style.getPropertyValue('width'),
rule.style.cssText,
rule.cssText,
retained.style.getPropertyValue('height'),
retained.cssText,
constructed.cssRules[0].style.width
].join('|');
})()
"#,
)
.expect("stylesheet EOF-open var CSSOM views should evaluate");
assert_eq!(
result,
"var(--open|width: var(--open;|div { width: var(--open; }|var(--retained|span { height: var(--retained; }|var(--written"
);
}