diff --git a/moli-renderer-v8/src/native_bridge/document/detached_objects/method_forwarders.rs b/moli-renderer-v8/src/native_bridge/document/detached_objects/method_forwarders.rs index bdecaf6a2..fbd31dbb4 100644 --- a/moli-renderer-v8/src/native_bridge/document/detached_objects/method_forwarders.rs +++ b/moli-renderer-v8/src/native_bridge/document/detached_objects/method_forwarders.rs @@ -666,12 +666,9 @@ pub(in crate::native_bridge) fn detached_click_method_callback<'s>( } return; }; - if !sync_detached_download_activation_attributes(scope, target, runtime_ptr, handle) - && let Some(delegate) = detached_live_delegate_object(scope, target) - { - let _ = call_object_method(scope, delegate, "click", &[]); - return; - } + // Reflected attributes already live in the native DOM. Reading public + // href/download properties here would invoke author getters and turn the + // default empty download value into a real download attribute. let outcome = crate::native_bridge::element::activate_handle_via_synthetic_click( scope, runtime_ptr, @@ -689,53 +686,6 @@ pub(in crate::native_bridge) fn detached_click_method_callback<'s>( } } -fn sync_detached_download_activation_attributes<'s>( - scope: &mut v8::PinScope<'s, '_>, - target: v8::Local<'s, v8::Object>, - runtime_ptr: *mut JsContextHost, - handle: DomHandle, -) -> bool { - let runtime = unsafe { &*runtime_ptr }; - let Some(element) = runtime.dom_host().node(handle).and_then(Node::as_element) else { - return false; - }; - if !matches!(element.local_name(), "a" | "area") { - return false; - } - - let property_href = detached_optional_string_property(scope, target, "href"); - let property_download = detached_optional_string_property(scope, target, "download"); - if let Some(href) = property_href { - let _ = write_detached_native_attribute(scope, target, "href", &href); - } - if let Some(download) = property_download { - let _ = write_detached_native_attribute(scope, target, "download", &download); - } - - let runtime = unsafe { &*runtime_ptr }; - runtime - .dom_host() - .node(handle) - .and_then(Node::as_element) - .is_some_and(|element| { - element.attribute("href").is_some() && element.attribute("download").is_some() - }) -} - -fn detached_optional_string_property<'s>( - scope: &mut v8::PinScope<'s, '_>, - target: v8::Local<'s, v8::Object>, - name: &'static str, -) -> Option { - let value = target.get(scope, v8str(scope, name).into())?; - if value.is_undefined() { - return None; - } - value - .to_string(scope) - .map(|value| value.to_rust_string_lossy(scope)) -} - pub(in crate::native_bridge::document) fn detached_compare_document_position_method_callback<'s>( scope: &mut v8::PinScope<'s, '_>, args: v8::FunctionCallbackArguments<'s>, diff --git a/moli-renderer-v8/src/native_bridge/document/detached_objects/state_tree.rs b/moli-renderer-v8/src/native_bridge/document/detached_objects/state_tree.rs index 7b59e2151..0f903a94e 100644 --- a/moli-renderer-v8/src/native_bridge/document/detached_objects/state_tree.rs +++ b/moli-renderer-v8/src/native_bridge/document/detached_objects/state_tree.rs @@ -783,42 +783,6 @@ pub(in crate::native_bridge::document) fn sync_detached_native_set_attribute_ns< ); } -pub(crate) fn write_detached_native_attribute<'s>( - scope: &mut v8::PinScope<'s, '_>, - element: v8::Local<'s, v8::Object>, - name: &str, - value: &str, -) -> bool { - let Some(runtime_ptr) = context_host_ptr_from_global_bridge(scope) else { - return false; - }; - let Some(handle) = detached_native_handle_for_runtime(scope, runtime_ptr, element) else { - return false; - }; - if unsafe { &*runtime_ptr } - .dom_host() - .node(handle) - .and_then(|node| node.as_element()) - .is_none() - { - return false; - } - clear_detached_iframe_context_before_navigation_attribute_change( - scope, - runtime_ptr, - element, - handle, - name, - Some(value), - ); - let changed = - unsafe { &mut *runtime_ptr }.set_attribute(scope, runtime_ptr, handle, name, value); - if changed { - detached_record_tree_mutation(scope, element); - } - true -} - pub(crate) fn write_detached_native_attribute_appending_to_current_reaction_queue<'s>( scope: &mut v8::PinScope<'s, '_>, element: v8::Local<'s, v8::Object>, diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/mod.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/mod.rs index 46ab2c49e..6875c888b 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/mod.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/mod.rs @@ -40,6 +40,7 @@ mod performance; mod performance_memory; mod platform_identity; mod pointer_lock; +mod popup_hyperlinks; mod promise_rejection; mod security_policy; mod service_worker_drain; diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/popup_hyperlinks.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/popup_hyperlinks.rs new file mode 100644 index 000000000..be6e8e24c --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/popup_hyperlinks.rs @@ -0,0 +1,205 @@ +use super::*; + +#[test] +fn popup_and_windowless_hyperlink_clicks_do_not_rewrite_reflected_attributes() { + let mut vm = new_storage_test_vm("https://popup-hyperlink-attributes.test/path/index.html"); + let result = vm + .eval( + r#" +(() => { + const root = document.documentElement || document.appendChild(document.createElement('html')); + if (!document.body) root.appendChild(document.createElement('body')); + const popup = open(); + try { + const docs = [document, popup.document, + document.implementation.createHTMLDocument(''), + new DOMParser().parseFromString('', 'text/html')]; + const failures = []; + for (const [index, doc] of docs.entries()) { + for (const tag of ['a', 'area']) { + for (const attributes of [[], [['href','../next']], + [['href','../next'],['download','']], [['href','../next'],['download','saved.txt']]]) { + const link = doc.createElement(tag); + for (const [name, value] of attributes) link.setAttribute(name, value); + doc.body.appendChild(link); + const before = link.outerHTML; + const observer = new MutationObserver(() => {}); + observer.observe(link, {attributes:true}); + let clicks = 0; + link.addEventListener('click', event => { + ++clicks; + if (event.isTrusted || event.target !== link) failures.push([index,tag,'event']); + event.preventDefault(); + }); + link.click(); + if (clicks !== 1 || link.outerHTML !== before || observer.takeRecords().length !== 0) { + failures.push([index,tag,before,link.outerHTML,clicks]); + } + observer.disconnect(); + } + } + } + return JSON.stringify(failures); + } finally { popup.close(); } +})() +"#, + ) + .expect("clicking a hyperlink should not change its attributes before event dispatch"); + assert_eq!(result, "[]"); + assert!(vm.take_pending_download_activations().is_empty()); +} + +#[test] +fn popup_hyperlink_activation_ignores_author_getters_and_uses_listener_updated_attributes() { + let mut vm = new_storage_test_vm("https://popup-hyperlink-download.test/path/index.html"); + vm.eval("globalThis.__downloadPopup = open(); 'ready'") + .expect("popup should open"); + vm.take_pending_popup_activations(); + let result = vm + .eval( + r#" +(() => { + const doc = __downloadPopup.document; + const states = []; + for (const tag of ['a', 'area']) { + for (const download of ['', 'initial.txt']) { + const link = doc.createElement(tag); + link.href = '/initial'; + link.download = download; + doc.body.appendChild(link); + let getterCalls = 0; + for (const name of ['href', 'download']) { + Object.defineProperty(link, name, { + configurable:true, get() { ++getterCalls; throw new Error('author ' + name); } + }); + } + let clicks = 0; + link.addEventListener('click', () => { + ++clicks; + link.setAttribute('href', tag + '.txt'); + if (download !== '') link.setAttribute('download', 'listener.txt'); + }); + link.click(); + states.push([getterCalls, clicks, link.getAttribute('href'), link.getAttribute('download')]); + } + } + return JSON.stringify(states); +})() +"#, + ) + .expect("activation should read native attributes after click listeners run"); + assert_eq!( + result, + r#"[[0,1,"a.txt",""],[0,1,"a.txt","listener.txt"],[0,1,"area.txt",""],[0,1,"area.txt","listener.txt"]]"# + ); + let downloads = vm.take_pending_download_activations(); + assert_eq!(downloads.len(), 4); + for (index, download) in downloads.iter().enumerate() { + let tag = if index < 2 { "a" } else { "area" }; + assert_eq!( + download.url, + format!("https://popup-hyperlink-download.test/path/{tag}.txt") + ); + assert_eq!( + download.suggested_filename.as_deref(), + (index % 2 == 1).then_some("listener.txt") + ); + } + assert!(vm.take_pending_popup_activations().is_empty()); + assert!(vm.take_pending_location_navigation_with_seed().is_none()); + vm.eval("__downloadPopup.close()").expect("close popup"); +} + +#[test] +fn popup_hyperlink_click_does_not_activate_an_expando_href_or_download() { + let mut vm = new_storage_test_vm("https://popup-hyperlink-expando.test/"); + vm.eval("globalThis.__expandoPopup = open(); 'ready'") + .expect("popup should open"); + vm.take_pending_popup_activations(); + let result = vm + .eval( + r#" +(() => { + const doc = __expandoPopup.document; + const results = []; + for (const tag of ['a', 'area']) { + const link = doc.createElement(tag); + Object.defineProperty(link, 'href', {value:'https://popup-hyperlink-expando.test/fake'}); + Object.defineProperty(link, 'download', {value:'fake.txt'}); + let clicks = 0; + link.addEventListener('click', () => ++clicks); + doc.body.appendChild(link); + link.click(); + results.push([clicks, link.hasAttribute('href'), link.hasAttribute('download')]); + } + return JSON.stringify(results); +})() +"#, + ) + .expect("JS expando properties should not supply hyperlink activation attributes"); + assert_eq!(result, "[[1,false,false],[1,false,false]]"); + assert!(vm.take_pending_download_activations().is_empty()); + assert!(vm.take_pending_popup_activations().is_empty()); + assert!(vm.take_pending_location_navigation_with_seed().is_none()); + vm.eval("__expandoPopup.close()").expect("close popup"); +} + +#[tokio::test] +async fn popup_hyperlink_opens_a_nested_window_and_relays_messages_to_its_opener() { + let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader"); + let mut vm = new_parsed_page_task_executor_test_vm( + "https://popup-nested-hyperlink.test/", + "", + &loader, + ); + vm.eval( + r#" +(() => { + window.name = 'root'; + window.__nestedHyperlinkMessages = []; + window.addEventListener('message', event => { + __nestedHyperlinkMessages.push([event.data, event.source === __firstHyperlinkPopup]); + }); + const childMarkup = ``; + const childUrl = URL.createObjectURL(new Blob([childMarkup], {type:'text/html'})); + const firstMarkup = ``; + const firstUrl = URL.createObjectURL(new Blob([firstMarkup], {type:'text/html'})); + window.__firstHyperlinkPopup = open(firstUrl, 'first-popup'); +})() +"#, + ) + .expect("nested popup fixture should open"); + advance_page_task_executor_until_eval_equals( + &mut vm, + &loader, + "String(__nestedHyperlinkMessages.length)", + "1", + "nested popup hyperlink message", + ) + .await; + assert_eq!( + vm.eval("JSON.stringify([__nestedHyperlinkMessages,__nestedHyperlinkMutated])") + .unwrap(), + r#"[[[{"child":{"name":"nested-child","openerName":"first-popup","isTop":true},"name":"first-popup","openerName":"root","childOpener":true},true]],false]"#, + ); + assert!(vm.take_pending_download_activations().is_empty()); + vm.eval("__firstHyperlinkPopup.__nestedChild.close(); __firstHyperlinkPopup.close();") + .expect("close nested popup fixtures"); +}