diff --git a/moli-renderer-v8/src/css_style.rs b/moli-renderer-v8/src/css_style.rs index 8ee2496923..f784c751c3 100644 --- a/moli-renderer-v8/src/css_style.rs +++ b/moli-renderer-v8/src/css_style.rs @@ -38,6 +38,36 @@ pub(crate) struct CssStyleDeclarationItemArgs { pub(crate) index: u32, } +#[derive(webidl::WebIdlArgs)] +#[webidl(prefix = "CSSStyleDeclaration.cssText")] +pub(crate) struct CssStyleDeclarationCssTextArgs { + // An attribute setter converts undefined even when called with no argument. + #[webidl(default = "undefined", treat_null_as_empty_string)] + pub(crate) value: String, +} + +/// Named interceptors receive a value rather than function arguments. Use the +/// same WebIDL string conversion as the declaration's generated arguments. +pub(crate) fn css_style_declaration_property_value<'s>( + scope: &mut v8::PinScope<'s, '_>, + value: v8::Local<'s, v8::Value>, +) -> Option { + match webidl::convert_with_options::( + scope, + value, + webidl::Context::argument("CSSStyleDeclaration", 1), + &webidl::StringOptions { + treat_null_as_empty_string: true, + }, + ) { + Ok(value) => Some(value.0), + Err(error) => { + webidl::throw_error(scope, &error); + None + } + } +} + #[derive(webidl::WebIdlArgs)] #[webidl(prefix = "CSSStyleDeclaration.setProperty")] pub(crate) struct CssStyleDeclarationSetPropertyArgs { diff --git a/moli-renderer-v8/src/detached_css_style.rs b/moli-renderer-v8/src/detached_css_style.rs index 72e1de03c8..c39dfd098a 100644 --- a/moli-renderer-v8/src/detached_css_style.rs +++ b/moli-renderer-v8/src/detached_css_style.rs @@ -2780,19 +2780,15 @@ fn style_css_text_setter_callback<'s>( crate::native_bridge::element::style_css_text_setter_callback(scope, args, _rv); return; } - let value = args.get(0); let Some(style) = lightweight_style_receiver(scope, args.this()) else { return; }; - let css_text = if value.is_null_or_undefined() { - String::new() - } else { - value - .to_string(scope) - .map(|value| value.to_rust_string_lossy(scope)) - .unwrap_or_default() + let Some(parsed) = + webidl::parse_args::(scope, &args) + else { + return; }; - let _ = set_lightweight_css_style_css_text(scope, style, &css_text); + let _ = set_lightweight_css_style_css_text(scope, style, &parsed.value); } fn style_named_property_getter_callback<'s>( @@ -2858,17 +2854,12 @@ fn style_named_property_setter_callback<'s>( ); return; } - let value = if value.is_null_or_undefined() { - String::new() - } else { - value - .to_string(scope) - .map(|value| value.to_rust_string_lossy(scope)) - .unwrap_or_default() - }; let Some(style) = lightweight_style_receiver(scope, args.this()) else { return; }; + let Some(value) = crate::css_style::css_style_declaration_property_value(scope, value) else { + return; + }; set_style_entry(scope, style, &name, &value, false); notify_style_changed(scope, style); } @@ -2886,9 +2877,6 @@ fn style_set_property_callback<'s>( crate::native_bridge::element::style_set_property_callback(scope, args, _rv); return; } - if args.length() > 1 && args.get(1).is_undefined() { - return; - } let Some(parsed) = webidl::parse_args::(scope, &args) else { return; diff --git a/moli-renderer-v8/src/native_bridge/element/styles/accessors.rs b/moli-renderer-v8/src/native_bridge/element/styles/accessors.rs index 67c4e1951b..abe4c2b2d6 100644 --- a/moli-renderer-v8/src/native_bridge/element/styles/accessors.rs +++ b/moli-renderer-v8/src/native_bridge/element/styles/accessors.rs @@ -1,8 +1,5 @@ use crate::util::{throw_type_error, v8_string}; -use crate::css_style::serialize_css_style_entries; - -use super::super::property_string_value; use super::declaration::{ StyleMode, set_inline_style_css_text_with_pdb_storage, style_base_url, style_css_text_for_computed, style_entries, style_entries_css_text_with_pdb, @@ -12,6 +9,10 @@ use super::{ style_object_computation_context, style_object_forces_empty_computed, style_object_property_count_with_context, }; +use crate::{ + css_style::{CssStyleDeclarationCssTextArgs, serialize_css_style_entries}, + webidl, +}; pub(crate) fn style_length_getter_callback<'s>( scope: &mut v8::PinScope<'s, '_>, @@ -91,6 +92,15 @@ pub(crate) fn style_css_text_setter_callback<'s>( args: v8::FunctionCallbackArguments<'s>, mut rv: v8::ReturnValue<'_, v8::Value>, ) { + if style_runtime_and_handle_from_object(scope, args.this()).is_err() { + throw_style_declaration_illegal_invocation(scope, "set", "cssText"); + return; + } + let Some(parsed) = webidl::parse_args::(scope, &args) else { + return; + }; + // Conversion can run author code. Resolve the native declaration again + // before accessing its runtime or applying the readonly flag. let Ok((runtime_ptr, handle, mode)) = style_runtime_and_handle_from_object(scope, args.this()) else { throw_style_declaration_illegal_invocation(scope, "set", "cssText"); @@ -105,14 +115,9 @@ pub(crate) fn style_css_text_setter_callback<'s>( ); return; } - let value = if args.get(0).is_null_or_undefined() { - String::new() - } else { - property_string_value(scope, args.get(0)).unwrap_or_default() - }; let base_url = style_base_url(unsafe { &*runtime_ptr }, handle); unsafe { &mut *runtime_ptr }.set_element_inline_style_base_url(handle, base_url.clone()); - set_inline_style_css_text_with_pdb_storage(scope, runtime_ptr, handle, &value); + set_inline_style_css_text_with_pdb_storage(scope, runtime_ptr, handle, &parsed.value); rv.set_undefined(); } diff --git a/moli-renderer-v8/src/native_bridge/element/styles/methods.rs b/moli-renderer-v8/src/native_bridge/element/styles/methods.rs index 3dbca44e70..196a1679af 100644 --- a/moli-renderer-v8/src/native_bridge/element/styles/methods.rs +++ b/moli-renderer-v8/src/native_bridge/element/styles/methods.rs @@ -36,9 +36,6 @@ pub(crate) fn style_set_property_callback<'s>( throw_style_declaration_method_illegal_invocation(scope, "setProperty"); return; }; - if args.length() > 1 && args.get(1).is_undefined() { - return; - } let Some(parsed) = webidl::parse_args::(scope, &args) else { return; diff --git a/moli-renderer-v8/src/native_bridge/element/styles/named.rs b/moli-renderer-v8/src/native_bridge/element/styles/named.rs index ec732a4323..787235b897 100644 --- a/moli-renderer-v8/src/native_bridge/element/styles/named.rs +++ b/moli-renderer-v8/src/native_bridge/element/styles/named.rs @@ -1,6 +1,3 @@ -use crate::util::v8_string; - -use super::super::property_string_value; use super::declaration::{ StyleMode, cssom_style_property_affected_names_with_pdb, expand_unresolved_box_shorthand_entries_for_mutation, is_style_intrinsic_name, @@ -16,6 +13,10 @@ use super::{ style_object_computation_context, style_object_forces_empty_computed, style_object_pseudo_element, }; +use crate::{ + css_style::css_style_declaration_property_value, + util::{throw_type_error, v8_string}, +}; pub(super) fn style_named_getter<'s>( scope: &mut v8::PinScope<'s, '_>, @@ -174,15 +175,6 @@ pub(crate) fn set_live_style_named_property_value<'s>( let Ok((runtime_ptr, handle, mode)) = style_runtime_and_handle_from_object(scope, style) else { return false; }; - if mode == StyleMode::Computed { - crate::native_bridge::throw_dom_exception( - scope, - "NoModificationAllowedError", - 7, - "Cannot modify a read-only CSSStyleDeclaration.", - ); - return true; - } if is_style_intrinsic_name(raw_key) { return false; } @@ -193,11 +185,23 @@ pub(crate) fn set_live_style_named_property_value<'s>( }; property }; - let value = if value.is_null_or_undefined() { - String::new() - } else { - property_string_value(scope, value).unwrap_or_default() + let Some(value) = css_style_declaration_property_value(scope, value) else { + return true; }; + // Do not retain a native runtime lookup across author string conversion. + let Ok((runtime_ptr, handle, mode)) = style_runtime_and_handle_from_object(scope, style) else { + throw_type_error(scope, "Illegal invocation"); + return true; + }; + if mode == StyleMode::Computed { + crate::native_bridge::throw_dom_exception( + scope, + "NoModificationAllowedError", + 7, + "Cannot modify a read-only CSSStyleDeclaration.", + ); + return true; + } if set_inline_style_property_with_pdb_storage( scope, runtime_ptr, 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 d23e1c5673..65ca237bde 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 @@ -62,3 +62,5 @@ mod css_computed_map_enumeration; mod nested_document_invalidation; mod properties_and_selectors; mod stylesheet_and_document_lifecycle; + +mod cssom_write_conversion; diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.js b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.js new file mode 100644 index 0000000000..ed63d44dc8 --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.js @@ -0,0 +1,152 @@ +(() => { + const checks = []; + const check = (name, fn) => { + try { checks.push({name, passed: fn() === true}); } + catch (error) { checks.push({name, passed: false, error: String(error)}); } + }; + const caught = fn => { try { fn(); return null; } catch (error) { return error; } }; + const frame = document.createElement('iframe'); + document.body.appendChild(frame); + const popup = window.open('about:blank', '_blank'); + const owners = [window, frame.contentWindow, popup]; + const resources = []; + const initial = 'color: red; background-color: blue; --marker: seed;'; + const operations = [ + ['color', (s, v) => { s.color = v; }, 'green', 'color'], + ['camel', (s, v) => { s.backgroundColor = v; }, 'green', 'background-color'], + ['dashed', (s, v) => { s['background-color'] = v; }, 'green', 'background-color'], + ['cssText', (s, v) => { s.cssText = v; }, 'color: green;', 'color'], + ['setProperty', (s, v) => s.setProperty('color', v), 'green', 'color'], + ['custom', (s, v) => s.setProperty('--marker', v), 'green', '--marker'], + ]; + function descriptor(owner, name) { + const style = owner.document.createElement('div').style; + for (let p = Object.getPrototypeOf(style); p; p = Object.getPrototypeOf(p)) { + const d = Object.getOwnPropertyDescriptor(p, name); + if (d) return d; + } + throw new Error('Missing CSSOM descriptor ' + name); + } + try { + for (const [realm, owner] of owners.entries()) { + const doc = owner.document; + const live = doc.createElement('div'); doc.body.appendChild(live); resources.push(live); + const detached = doc.createElement('div'); + const windowless = doc.implementation.createHTMLDocument('').createElement('div'); + const svg = doc.createElementNS('http://www.w3.org/2000/svg', 'svg'); + const sheetElement = doc.createElement('style'); + sheetElement.textContent = '.write-conversion-target { color: red; }'; + doc.head.appendChild(sheetElement); resources.push(sheetElement); + const Sheet = sheetElement.sheet.constructor; + const constructed = new Sheet(); constructed.insertRule('.constructed { color: red; }'); + const providers = [ + ['inline', live.style, false], ['detached', detached.style, false], + ['windowless', windowless.style, false], ['SVG', svg.style, false], + ['rule', sheetElement.sheet.cssRules[0].style, false], + ['constructed rule', constructed.cssRules[0].style, false], + ['computed', owner.getComputedStyle(live), true], + ]; + for (const [kind, style, readonly] of providers) { + const reset = () => { live.style.cssText = initial; if (!readonly) style.cssText = initial; }; + for (const [operation, write, good, property] of operations) { + const prefix = `${realm}/${kind}/${operation}`; + for (const failure of ['toString', 'toPrimitive', 'getter', 'fallback', 'symbol', 'nonprimitive']) { + check(prefix + '/' + failure + '/exception and unchanged declaration', () => { + reset(); const before = style.cssText; const marker = {prefix, failure}; const calls = []; + let value; + if (failure === 'toString') value = {toString() { calls.push('toString'); throw marker; }}; + if (failure === 'toPrimitive') value = {[Symbol.toPrimitive](hint) { calls.push(hint); throw marker; }}; + if (failure === 'getter') value = {get toString() { calls.push('get'); throw marker; }}; + if (failure === 'fallback') value = {toString() { calls.push('toString'); return {}; }, valueOf() { calls.push('valueOf'); throw marker; }}; + if (failure === 'symbol') value = Symbol('value'); + if (failure === 'nonprimitive') value = {[Symbol.toPrimitive](hint) { calls.push(hint); return {}; }}; + const error = caught(() => write(style, value)); + const expected = failure === 'fallback' ? ['toString', 'valueOf'] : failure === 'getter' ? ['get'] : failure === 'toString' ? ['toString'] : failure === 'symbol' ? [] : ['string']; + return (failure === 'symbol' || failure === 'nonprimitive' ? error?.name === 'TypeError' : error === marker) && + JSON.stringify(calls) === JSON.stringify(expected) && style.cssText === before; + }); + } + check(prefix + '/successful conversion precedes native write or readonly error', () => { + reset(); let calls = 0; + const error = caught(() => write(style, {[Symbol.toPrimitive](hint) { calls++; if (hint !== 'string') throw new Error(hint); return good; }})); + return calls === 1 && (readonly ? error?.name === 'NoModificationAllowedError' : error === null && style.getPropertyValue(property) === 'green'); + }); + for (const [name, value] of [['null', null], ['undefined', undefined]]) { + check(prefix + '/' + name + '/IDL string conversion', () => { + reset(); const before = style.getPropertyValue(property); + const error = caught(() => write(style, value)); + if (readonly) return error?.name === 'NoModificationAllowedError'; + const expected = operation === 'cssText' || value === null ? '' : operation === 'custom' ? 'undefined' : before; + return error === null && style.getPropertyValue(property) === expected; + }); + } + if (!readonly) { + for (const throws of [false, true]) { + check(prefix + '/reentrant mutation/' + (throws ? 'throw' : 'success'), () => { + reset(); const marker = {}; + const error = caught(() => write(style, {toString() { + style.setProperty(property, 'green'); + if (throws) throw marker; + return operation === 'cssText' ? 'color: blue;' : 'blue'; + }})); + return error === (throws ? marker : null) && style.getPropertyValue(property) === (throws ? 'green' : 'blue'); + }); + } + } + } + for (const throwingArgument of ['none', 'property', 'value', 'priority']) { + check(`${realm}/${kind}/setProperty/conversion order/${throwingArgument}`, () => { + reset(); const before = style.cssText; const calls = []; const marker = {}; + const argument = (name, text) => ({toString() { calls.push(name); if (name === throwingArgument) throw marker; return text; }}); + const error = caught(() => style.setProperty(argument('property', 'color'), argument('value', 'green'), argument('priority', 'important'))); + const expected = throwingArgument === 'property' ? ['property'] : throwingArgument === 'value' ? ['property', 'value'] : ['property', 'value', 'priority']; + return JSON.stringify(calls) === JSON.stringify(expected) && (throwingArgument === 'none' ? + readonly ? error?.name === 'NoModificationAllowedError' : error === null && style.color === 'green' && style.getPropertyPriority('color') === 'important' : + error === marker && style.cssText === before); + }); + } + for (const property of ['color', '--marker', 'unknown-css-property']) { + for (const priorityThrows of [false, true]) { + check(`${realm}/${kind}/undefined value/${property}/${priorityThrows}`, () => { + reset(); const before = style.cssText; const calls = []; const marker = {}; + const error = caught(() => style.setProperty({toString() { calls.push('property'); return property; }}, undefined, + {toString() { calls.push('priority'); if (priorityThrows) throw marker; return ''; }})); + if (JSON.stringify(calls) !== '["property","priority"]') return false; + if (priorityThrows) return error === marker && style.cssText === before; + if (readonly) return error?.name === 'NoModificationAllowedError'; + return error === null && (property === '--marker' ? style.getPropertyValue(property) === 'undefined' : style.cssText === before); + }); + } + } + check(`${realm}/${kind}/expando bypasses CSS conversion`, () => { + const value = {toString() { throw new Error('Expando coerced'); }}; + style.__writeConversionExpando = value; + return style.__writeConversionExpando === value; + }); + for (const [calleeIndex, callee] of owners.entries()) { + for (const member of ['cssText', 'setProperty']) { + const binding = descriptor(callee, member); + const fn = member === 'setProperty' ? binding.value : binding.set; + const errorPrototype = fn.constructor('return TypeError.prototype')(); + for (const badKind of ['object', 'inherited', 'author Proxy', 'revoked Proxy']) { + check(`${realm}/${kind}/${calleeIndex}/${member}/${badKind}/receiver before conversion`, () => { + const revocable = Proxy.revocable(style, {}); revocable.revoke(); + const receiver = badKind === 'object' ? {} : badKind === 'inherited' ? Object.create(style) : badKind === 'author Proxy' ? new Proxy(style, {}) : revocable.proxy; + let conversions = 0; const value = {toString() { conversions++; return 'green'; }}; + const error = caught(() => Reflect.apply(fn, receiver, member === 'setProperty' ? [value, value, value] : [value])); + return Object.getPrototypeOf(error) === errorPrototype && conversions === 0; + }); + } + } + } + } + } + } finally { + for (const resource of resources) resource.remove(); + popup.close(); frame.remove(); + } + const facts = {complete: true, total: checks.length, passed: checks.filter(row => row.passed).length, checks}; + globalThis.__cssomWriteConversionResults = facts; + globalThis.__uiEventResults = facts; + return facts.passed === facts.total; +})() diff --git a/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.rs b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.rs new file mode 100644 index 0000000000..d2b27acb87 --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/dom_xhr/computed_style/cssom_write_conversion.rs @@ -0,0 +1,18 @@ +use super::*; + +#[test] +fn cssom_writes_validate_receivers_then_convert_before_mutating() { + let mut vm = new_storage_page_task_executor_test_vm("https://cssom-write-conversion.test/"); + vm.eval(include_str!("cssom_write_conversion.js")) + .expect("CSSOM write conversion matrix should evaluate"); + assert_eq!( + vm.eval("JSON.stringify(__cssomWriteConversionResults.checks.filter(row => !row.passed))") + .unwrap(), + "[]" + ); + assert_eq!( + vm.eval("__cssomWriteConversionResults.complete && __cssomWriteConversionResults.total === 2085 && __cssomWriteConversionResults.passed === 2085") + .unwrap(), + "true" + ); +}