From 4900f67fbd9de90eff4e9b9e085e5d74e31aeeba Mon Sep 17 00:00:00 2001 From: ldm0 Date: Sun, 13 Sep 2026 08:52:12 +0800 Subject: [PATCH] fix(intersection-observer): convert options before validating thresholds Convert option members in WebIDL order, including root brand validation and the threshold double-or-sequence union. Propagate conversion failures before constructor margin/range validation and retain duplicate thresholds when sorting. Cover iterable discrimination, getter order, exception precedence, and later getter mutations; update the existing duplicate-threshold expectations. --- moli-core/tests/web_apis.rs | 2 +- moli-renderer-v8/src/observer_runtime/mod.rs | 147 +++++++------ .../src/script_vm/tests/webidl_fetch.rs | 208 ++++++++++++++++++ .../intersectionobserver-basic.html | 2 +- 4 files changed, 285 insertions(+), 74 deletions(-) diff --git a/moli-core/tests/web_apis.rs b/moli-core/tests/web_apis.rs index fea06d1e18..c0bd4236d8 100644 --- a/moli-core/tests/web_apis.rs +++ b/moli-core/tests/web_apis.rs @@ -5709,7 +5709,7 @@ async fn intersection_observer_options_reflect_root_margin_and_thresholds() -> R ); assert_eq!( diagnostic_global(&page, "intersectionObserverThresholds"), - Some(&JsValueSnapshot::String("[0.25,0.75]".to_owned())) + Some(&JsValueSnapshot::String("[0.25,0.75,0.75]".to_owned())) ); assert_eq!( diagnostic_global(&page, "intersectionObserverEntryPrototypeShape"), diff --git a/moli-renderer-v8/src/observer_runtime/mod.rs b/moli-renderer-v8/src/observer_runtime/mod.rs index 6349f808d2..5069eba5a2 100644 --- a/moli-renderer-v8/src/observer_runtime/mod.rs +++ b/moli-renderer-v8/src/observer_runtime/mod.rs @@ -405,17 +405,18 @@ struct MutationObserverConstructorArgs { #[derive(webidl::WebIdlDictionary)] #[webidl(prefix = "IntersectionObserverInit")] -struct IntersectionObserverInitMembers<'s> { - #[webidl(legacy_nullish, converter = "raw")] - root: Option>, +struct IntersectionObserverInitMembers { + // The derive converts members in declaration order; WebIDL requires lexical order. + #[webidl(default = 0)] + delay: i32, + #[webidl(with = intersection_observer_root_member)] + root: Option, #[webidl(default = "0px")] root_margin: String, #[webidl(default = "0px")] scroll_margin: String, - #[webidl(legacy_nullish, converter = "raw")] - threshold: Option>, - #[webidl(default = 0)] - delay: i32, + #[webidl(with = intersection_observer_threshold_member)] + threshold: Vec, #[webidl(default = false)] track_visibility: bool, } @@ -1185,18 +1186,12 @@ pub(super) fn intersection_observer_constructor_callback<'s>( rv.set_undefined(); return; }; - let options = match parse_intersection_observer_options(scope, parsed.options, |root| { - dom_access::is_intersection_root(host_ptr, root) - }) { + let options = match parse_intersection_observer_options(scope, parsed.options) { Ok(options) => options, Err(IntersectionObserverOptionsError::Range(message)) => { throw_range_error(scope, message); return; } - Err(IntersectionObserverOptionsError::Type(message)) => { - throw_type_error(scope, &message); - return; - } Err(IntersectionObserverOptionsError::Syntax(message)) => { throw_dom_exception_value(scope, message, "SyntaxError"); return; @@ -1611,7 +1606,6 @@ fn has_property( } enum IntersectionObserverOptionsError { - Type(String), Syntax(&'static str), WebIdl(webidl::WebIdlError), Range(&'static str), @@ -1620,7 +1614,6 @@ enum IntersectionObserverOptionsError { fn parse_intersection_observer_options<'s>( scope: &mut v8::PinScope<'s, '_>, value: Option>, - mut root_is_valid: impl FnMut(NativeNodeId) -> bool, ) -> Result { let Some(value) = value else { return Ok(IntersectionObserverOptions::default()); @@ -1635,21 +1628,10 @@ fn parse_intersection_observer_options<'s>( Err(error) => return Err(IntersectionObserverOptionsError::WebIdl(error)), }; - let mut options = IntersectionObserverOptions::default(); - if let Some(root_value) = init.root { - let Some(root) = callback_value_dom_handle(scope, root_value) else { - return Err(IntersectionObserverOptionsError::Type( - "Failed to construct 'IntersectionObserver': root is not a Node.".to_owned(), - )); - }; - if !root_is_valid(root) { - return Err(IntersectionObserverOptionsError::Type( - "Failed to construct 'IntersectionObserver': root must be an Element or Document." - .to_owned(), - )); - } - options.root = Some(root); - } + let mut options = IntersectionObserverOptions { + root: init.root, + ..IntersectionObserverOptions::default() + }; options.root_margin = normalize_root_margin(&init.root_margin).ok_or( IntersectionObserverOptionsError::Syntax( @@ -1662,9 +1644,7 @@ fn parse_intersection_observer_options<'s>( ), )?; - if let Some(thresholds) = threshold_option(scope, init.threshold)? { - options.thresholds = thresholds; - } + options.thresholds = normalize_intersection_thresholds(init.threshold)?; options.delay = init.delay; options.track_visibility = init.track_visibility; if options.track_visibility && options.delay < 100 { @@ -1687,53 +1667,76 @@ fn bool_option( .is_some_and(|value| value.boolean_value(scope)) } -fn threshold_option<'s>( +fn intersection_observer_root_member<'s>( scope: &mut v8::PinScope<'s, '_>, - value: Option>, -) -> Result>, IntersectionObserverOptionsError> { - let Some(value) = value else { return Ok(None) }; - - let mut thresholds = if value.is_object() { - webidl::convert::>( - scope, - value, - webidl::Context::member("IntersectionObserverInit", "threshold"), + object: v8::Local<'s, v8::Object>, + member: &'static str, +) -> Result, webidl::WebIdlError> { + let Some(value) = webidl::legacy_optional_member::>( + scope, + object, + member, + webidl::Context::member("IntersectionObserverInit", member), + )? + else { + return Ok(None); + }; + let root = callback_value_dom_handle(scope, value).ok_or_else(|| { + webidl::WebIdlError::custom_message( + "Failed to construct 'IntersectionObserver': root is not a Node.", ) - .map_err(IntersectionObserverOptionsError::WebIdl)? - .0 - .into_iter() - .map(|value| validate_intersection_threshold(value.0)) - .collect::, _>>()? - } else { - vec![threshold_number(scope, value)?] - }; - thresholds.sort_by(|left, right| left.partial_cmp(right).unwrap_or(Ordering::Equal)); - thresholds.dedup_by(|left, right| left.total_cmp(right) == Ordering::Equal); - if thresholds.is_empty() { - thresholds.push(0.0); - } - Ok(Some(thresholds)) -} - -fn threshold_number<'s>( - scope: &mut v8::PinScope<'s, '_>, - value: v8::Local<'s, v8::Value>, -) -> Result { - let Some(number) = value.number_value(scope) else { - return Err(IntersectionObserverOptionsError::Type( - "Failed to construct 'IntersectionObserver': threshold must be a number.".to_owned(), + })?; + let valid = context_host_ptr_from_global_bridge(scope) + .is_some_and(|host_ptr| dom_access::is_intersection_root(host_ptr, root)); + if !valid { + return Err(webidl::WebIdlError::custom_message( + "Failed to construct 'IntersectionObserver': root must be an Element or Document.", )); - }; - validate_intersection_threshold(number) + } + Ok(Some(root)) } -fn validate_intersection_threshold(number: f64) -> Result { - if !number.is_finite() || !(0.0..=1.0).contains(&number) { +fn intersection_observer_threshold_member<'s>( + scope: &mut v8::PinScope<'s, '_>, + object: v8::Local<'s, v8::Object>, + member: &'static str, +) -> Result, webidl::WebIdlError> { + let context = webidl::Context::member("IntersectionObserverInit", member); + let Some(value) = + webidl::optional_member::>(scope, object, member, context)? + else { + return Ok(vec![0.0]); + }; + + // Convert the union at its dictionary position, reading @@iterator once. + if let Some(sequence) = + webidl::convert_optional_sequence::(scope, value, context, &())? + { + Ok(sequence.0.into_iter().map(|value| value.0).collect()) + } else { + Ok(vec![ + webidl::convert::(scope, value, context)?.0, + ]) + } +} + +fn normalize_intersection_thresholds( + mut thresholds: Vec, +) -> Result, IntersectionObserverOptionsError> { + // WebIDL conversion has completed; constructor validation follows margins. + if thresholds + .iter() + .any(|number| !(0.0..=1.0).contains(number)) + { return Err(IntersectionObserverOptionsError::Range( "Failed to construct 'IntersectionObserver': threshold must be between 0 and 1.", )); } - Ok(number) + thresholds.sort_by(|left, right| left.partial_cmp(right).unwrap_or(Ordering::Equal)); + if thresholds.is_empty() { + thresholds.push(0.0); + } + Ok(thresholds) } fn string_set_option( diff --git a/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs b/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs index 12715e4e3b..8465b5aba1 100644 --- a/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs +++ b/moli-renderer-v8/src/script_vm/tests/webidl_fetch.rs @@ -6785,6 +6785,214 @@ fn intersection_observer_margin_errors_preserve_dictionary_conversion_exceptions assert_eq!(result, ""); } +#[test] +fn intersection_observer_threshold_union_converts_numbers_and_iterables() { + let mut vm = new_storage_test_vm("https://intersection-observer-threshold-union.test/"); + let result = vm + .eval( + r#" +(() => { + const check = (condition, label) => { if (!condition) throw new Error(label); }; + const cases = [ + ['default', undefined, [0]], ['null', null, [0]], + ['false', false, [0]], ['true', true, [1]], + ['string', '0.25', [0.25]], ['boxed number', new Number(0.25), [0.25]], + ['object', {valueOf() { return 0.25; }}, [0.25]], + ['null iterator', {[Symbol.iterator]: null, valueOf() { return 0.25; }}, [0.25]], + ['undefined iterator', {[Symbol.iterator]: undefined, valueOf() { return 0.25; }}, [0.25]], + ['string object', new String('01'), [0, 1]], + ['duplicates', [0.75, 0.25, 0.25], [0.25, 0.25, 0.75]], + ['typed array', new Float64Array([0.75, 0.25, 0.25]), [0.25, 0.25, 0.75]], + ['empty sequence', [], [0]] + ]; + for (const [label, threshold, expected] of cases) { + const observer = new IntersectionObserver(() => {}, {threshold}); + check(JSON.stringify(observer.thresholds) === JSON.stringify(expected), label); + observer.disconnect(); + } + + let reads = 0; + let calls = 0; + const iterable = { + get [Symbol.iterator]() { + reads++; + return function*() { + calls++; + check(this === iterable, 'iterator receiver'); + yield 0.75; + yield 0.25; + yield 0.25; + }; + }, + valueOf() { throw new Error('iterable must not use numeric fallback'); } + }; + const observer = new IntersectionObserver(() => {}, {threshold: iterable}); + check(reads === 1 && calls === 1, 'iterator must be read and called once'); + check(JSON.stringify(observer.thresholds) === '[0.25,0.25,0.75]', 'iterable thresholds'); + observer.disconnect(); + + reads = 0; + calls = 0; + const number = new IntersectionObserver(() => {}, {threshold: { + get [Symbol.iterator]() { reads++; return null; }, + valueOf() { calls++; return 0.5; } + }}); + check(reads === 1 && calls === 1 && number.thresholds[0] === 0.5, 'numeric fallback'); + number.disconnect(); + let caught; + try { + new IntersectionObserver(() => {}, {threshold: { + [Symbol.iterator]: 1, + valueOf() { throw new Error('noncallable iterator must not fall back'); } + }}); + } catch (error) { caught = error; } + check(caught instanceof TypeError, 'noncallable iterator'); + return 'ok'; +})() +"#, + ) + .expect("IntersectionObserver threshold union should convert through WebIDL"); + assert_eq!(result, "ok"); +} + +#[test] +fn intersection_observer_converts_dictionary_members_in_order() { + let mut vm = new_storage_test_vm("https://intersection-observer-dictionary-order.test/"); + let result = vm.eval(r#" +(() => { + const check = (condition, label) => { if (!condition) throw new Error(label); }; + const log = []; + const observer = new IntersectionObserver(() => {}, { + get trackVisibility() { log.push('trackVisibility'); return false; }, + get threshold() { + log.push('threshold'); + return {get [Symbol.iterator]() { + log.push('iterator'); + return function*() { + log.push('iterate'); + yield {valueOf() { log.push('double'); return 0.25; }}; + log.push('done'); + }; + }}; + }, + get scrollMargin() { + log.push('scrollMargin'); + return {toString() { log.push('scrollMargin string'); return '0px'; }}; + }, + get rootMargin() { + log.push('rootMargin'); + return {toString() { log.push('rootMargin string'); return '0px'; }}; + }, + get root() { log.push('root'); return document; }, + get delay() { + log.push('delay'); + return {valueOf() { log.push('delay number'); return 3; }}; + } + }); + check(log.join(',') === 'delay,delay number,root,rootMargin,rootMargin string,scrollMargin,scrollMargin string,threshold,iterator,iterate,double,done,trackVisibility', log.join(',')); + check(observer.root === document && observer.delay === 3, 'converted root and delay'); + observer.disconnect(); + + const threshold = [0.25]; + const snapshot = new IntersectionObserver(() => {}, { + threshold, + get trackVisibility() { threshold[0] = 0.75; return false; } + }); + check(snapshot.thresholds[0] === 0.25, 'threshold converted before later getter mutation'); + snapshot.disconnect(); + + const sentinel = {}; + let caught; + try { + new IntersectionObserver(() => {}, { + get delay() { throw sentinel; }, + get root() { throw new Error('root must not be read'); } + }); + } catch (error) { caught = error; } + check(caught === sentinel, 'delay conversion is first'); + + for (const root of [{}, document.createTextNode('root'), document.createDocumentFragment()]) { + for (const member of ['rootMargin', 'scrollMargin', 'threshold', 'trackVisibility']) { + let calls = 0; + caught = undefined; + try { + new IntersectionObserver(() => {}, { + root, + get [member]() { calls++; throw sentinel; } + }); + } catch (error) { caught = error; } + check(caught instanceof TypeError && calls === 0, 'invalid root precedes ' + member); + } + } + for (const root of [null, undefined, document, document.createElement('div')]) { + const valid = new IntersectionObserver(() => {}, {root}); + check(valid.root === (root ?? null), 'valid root'); + valid.disconnect(); + } + return 'ok'; +})() +"#).expect("IntersectionObserver options should convert in dictionary member order"); + assert_eq!(result, "ok"); +} + +#[test] +fn intersection_observer_validates_thresholds_after_dictionary_conversion() { + let mut vm = new_storage_test_vm("https://intersection-observer-threshold-validation.test/"); + let result = vm + .eval( + r#" +(() => { + const check = (condition, label) => { if (!condition) throw new Error(label); }; + const capture = options => { + try { new IntersectionObserver(() => {}, options); } catch (error) { return error; } + return undefined; + }; + for (const value of [NaN, Infinity, -Infinity, 'foo', Symbol(), 1n]) { + for (const threshold of [value, [value]]) { + check(capture({threshold}) instanceof TypeError, 'invalid double'); + check(capture({threshold, rootMargin: 'invalid'}) instanceof TypeError, + 'double conversion precedes margin parsing'); + } + } + for (const value of [-0.25, 1.25]) { + for (const threshold of [value, [value]]) { + check(capture({threshold}) instanceof RangeError, 'out-of-range threshold'); + for (const member of ['rootMargin', 'scrollMargin']) { + const error = capture({threshold, [member]: 'invalid'}); + check(error instanceof DOMException && error.name === 'SyntaxError', + member + ' parsing precedes range validation'); + } + const sentinel = {}; + check(capture({threshold, get trackVisibility() { throw sentinel; }}) === sentinel, + 'dictionary conversion precedes range validation'); + } + } + + const sentinel = {}; + const fail = () => { throw sentinel; }; + for (const threshold of [ + {get [Symbol.iterator]() { throw sentinel; }}, + {valueOf: fail}, + [{valueOf: fail}], + [1.25, {valueOf: fail}] + ]) { + for (const member of ['rootMargin', 'scrollMargin']) { + check(capture({threshold, [member]: 'invalid'}) === sentinel, + member + ' must preserve threshold conversion exception'); + } + let calls = 0; + check(capture({threshold, get trackVisibility() { calls++; return false; }}) === sentinel, + 'threshold conversion must preserve exception identity'); + check(calls === 0, 'failed conversion must stop dictionary reads'); + } + return 'ok'; +})() +"#, + ) + .expect("IntersectionObserver should separate conversion from threshold validation"); + assert_eq!(result, "ok"); +} + #[test] fn resize_observer_callbacks_apply_webidl_conversion() { let mut vm = new_storage_test_vm("https://resize-observer-webidl.test/"); diff --git a/moli-wpt-compat/fixtures/wpt/ported/intersection-observer/intersectionobserver-basic.html b/moli-wpt-compat/fixtures/wpt/ported/intersection-observer/intersectionobserver-basic.html index f3ef062ed2..6fff25b29a 100644 --- a/moli-wpt-compat/fixtures/wpt/ported/intersection-observer/intersectionobserver-basic.html +++ b/moli-wpt-compat/fixtures/wpt/ported/intersection-observer/intersectionobserver-basic.html @@ -43,7 +43,7 @@ test(function () { assert_true(observer instanceof IntersectionObserver); assert_equals(observer.root, root); assert_equals(observer.rootMargin, "10px 20px 10px 20px"); - assert_array_equals(Array.from(observer.thresholds), [0, 0.5, 1]); + assert_array_equals(Array.from(observer.thresholds), [0, 0.5, 0.5, 1]); observer.disconnect(); }, "IntersectionObserver exposes normalized root, rootMargin, and thresholds");