From 12625e4a7df2932abc295ec79c37bcc4381d42ff Mon Sep 17 00:00:00 2001 From: ldm0 Date: Wed, 23 Sep 2026 02:24:56 +0800 Subject: [PATCH] fix(dom): preserve hyperlink attributes during detached click activation Detached hyperlink clicks copied public href and download properties into native attributes. The default empty download value therefore converted ordinary popup links into downloads and prevented nested popup navigation. Use the existing native handle activation path without this synchronization, and remove its unused attribute writer. Keep fallback delegation for wrappers without a native handle. Cover attribute preservation, author getters and expando properties, listener updates to explicit downloads, and nested popup opener/message delivery. --- .../detached_objects/method_forwarders.rs | 56 +---- .../document/detached_objects/state_tree.rs | 36 --- .../src/script_vm/tests/browser_api/mod.rs | 1 + .../tests/browser_api/popup_hyperlinks.rs | 205 ++++++++++++++++++ 4 files changed, 209 insertions(+), 89 deletions(-) create mode 100644 moli-renderer-v8/src/script_vm/tests/browser_api/popup_hyperlinks.rs 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"); +}