From 7ffef8b6364f4a5093f32d16aa1b44d108c822fc Mon Sep 17 00:00:00 2001 From: ldm0 Date: Wed, 30 Sep 2026 03:41:05 +0800 Subject: [PATCH] fix(navigation): classify same-document requests by URL fragments --- .../navigation_callbacks/navigation.rs | 33 +-- .../navigation_cross_document.rs | 4 +- .../browser_api/navigation/same_document.rs | 193 ++++++++++++++++++ 3 files changed, 201 insertions(+), 29 deletions(-) diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_callbacks/navigation.rs b/moli-renderer-v8/src/context_bootstrap/navigation_callbacks/navigation.rs index 2e64223092..90fe9b5abc 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_callbacks/navigation.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_callbacks/navigation.rs @@ -237,12 +237,11 @@ pub(in crate::context_bootstrap) fn navigation_navigate_callback<'s>( }; let current_url = url::Url::parse(¤t_href).ok(); let can_update_current_entry = navigation_document_can_update_current_entry(scope, owner); - let exact_same_url_push = matches!(navigate_history_kind, NavigationNavigateHistoryKind::Push) - && next_url.as_str() == current_href; - if can_update_current_entry - && is_same_document_fragment_navigation(current_url.as_ref(), &next_url) - && !exact_same_url_push - { + // History behavior chooses which entry changes, not whether the navigation + // replaces the Document. Even an empty fragment is a fragment navigation. + let fragment_navigation = next_url.fragment().is_some() + && is_same_document_fragment_navigation(current_url.as_ref(), &next_url); + if can_update_current_entry && fragment_navigation { let navigation_for_event = window_navigation_for_holder(scope, owner); let canceled_cross_document = if let Some(navigation) = navigation_for_event { let _ = cancel_active_navigation_event(scope, navigation); @@ -373,28 +372,6 @@ pub(in crate::context_bootstrap) fn navigation_navigate_callback<'s>( .as_ref() .and_then(|outcome| outcome.redirected_state) .or(cloned_navigation_state); - if effective_href == current_href - && matches!(effective_kind, LocationNavigationKind::Replace) - && !runtime_window_is_global(scope, owner) - && !navigate_outcome - .as_ref() - .is_some_and(|outcome| outcome.intercepted) - { - rv.set(handle_navigation_navigate_cross_document( - scope, - owner, - history, - &next_url, - NavigationNavigateHistoryKind::Replace, - navigation_for_event.map(|navigation| { - ( - navigation, - navigate_outcome.as_ref().and_then(|outcome| outcome.signal), - ) - }), - )); - return; - } if let Some(outcome) = navigate_outcome.as_ref() && let Some(precommit_event) = outcome.precommit_event && let Some(pending) = navigation_result_with_pending_commit(scope) diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_cross_document.rs b/moli-renderer-v8/src/context_bootstrap/navigation_cross_document.rs index 0bdfabfebf..587aa8c042 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_cross_document.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_cross_document.rs @@ -57,7 +57,9 @@ pub(super) fn handle_navigation_navigate_cross_document<'s>( NavigationNavigateHistoryKind::Push => NavigationHistoryMutation::Push, NavigationNavigateHistoryKind::Replace => NavigationHistoryMutation::Replace, NavigationNavigateHistoryKind::Default => { - if current_href.as_deref() == Some("about:blank") && current_entry_is_about_blank { + if current_href.as_deref() == Some(next_url.as_str()) + || (current_href.as_deref() == Some("about:blank") && current_entry_is_about_blank) + { NavigationHistoryMutation::Replace } else { NavigationHistoryMutation::Push diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/navigation/same_document.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/navigation/same_document.rs index 912a284b67..fef82bc836 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/navigation/same_document.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/navigation/same_document.rs @@ -720,3 +720,196 @@ fn same_document_location_intercept_reject_dispatches_navigateerror() { .expect("location intercept reject microtasks should evaluate"); assert_eq!(after_microtasks, "error:true:#one|microtask"); } + +#[test] +fn same_url_navigation_classifies_fragments_before_interception() { + for (fragment, remove_fragment, same_document) in [ + ("", false, false), + ("#", false, true), + ("#fragment", false, true), + ("#fragment", true, false), + ] { + for history in ["auto", "push", "replace"] { + for action in ["navigate", "intercept", "cancel"] { + let mut vm = new_storage_test_vm(&format!( + "https://same-url-navigation.test/page{fragment}" + )); + vm.exec( + &format!( + r#" +const beforeLength = history.length; +const events = [], settled = []; +navigation.addEventListener('navigate', event => {{ + events.push([event.navigationType, event.destination.sameDocument, event.hashChange]); + if ({action:?} === 'intercept') event.intercept(); + if ({action:?} === 'cancel') event.preventDefault(); +}}); +const destination = {remove_fragment} ? location.href.split('#')[0] : location.href; +const result = navigation.navigate(destination, {{history: {history:?}}}); +for (const name of ['committed', 'finished']) + result[name].then(() => settled.push(name), error => settled.push(name + ':' + error.name)); +"# + ), + None, + ) + .unwrap(); + let context = format!("{fragment}/{remove_fragment}/{history}/{action}"); + let pushes = history == "push" || (history == "auto" && remove_fragment); + let navigation_type = if pushes { "push" } else { "replace" }; + let pending = vm.take_pending_location_navigation_with_seed(); + assert_eq!( + pending.is_some(), + !same_document && action == "navigate", + "{context}" + ); + if let Some(pending) = pending { + assert_eq!( + pending.url.as_str(), + "https://same-url-navigation.test/page" + ); + assert_eq!( + pending + .entry_seed + .unwrap() + .activation + .unwrap() + .navigation_type + .as_deref(), + Some(navigation_type), + "{context}" + ); + } + let result = vm + .eval("JSON.stringify({events, settled, delta: history.length - beforeLength})") + .unwrap(); + let result: serde_json::Value = serde_json::from_str(&result).unwrap(); + let committed = action != "cancel" && (same_document || action == "intercept"); + let settled = if action == "cancel" { + serde_json::json!(["committed:AbortError", "finished:AbortError"]) + } else if committed { + serde_json::json!(["committed", "finished"]) + } else { + serde_json::json!([]) + }; + assert_eq!( + result, + serde_json::json!({ + "events": [[navigation_type, same_document, false]], + "settled": settled, + "delta": i32::from(committed && pushes), + }), + "{context}" + ); + } + } + } +} + +#[tokio::test] +async fn same_url_navigation_preserves_fragment_documents_in_iframes() { + for (fragment, remove_fragment, same_document) in [ + ("", false, false), + ("#", false, true), + ("#fragment", false, true), + ("#fragment", true, false), + ] { + for history in ["auto", "push", "replace"] { + let request_count = if same_document { 1 } else { 2 }; + let server = StaticHttpServer::spawn(request_count).await; + let parent = server.base_url().join("parent").unwrap(); + let loader = static_http_loader([]); + let mut vm = + new_storage_page_task_executor_test_vm_with_loader(parent.as_str(), &loader); + let context = format!("{fragment}/{remove_fragment}/{history}"); + vm.eval( + r#" +const frame = document.createElement('iframe'); +frame.src = '/child'; document.body.append(frame); +const child = frame.contentWindow; +"#, + ) + .unwrap(); + advance_page_task_executor_until_eval_equals( + &mut vm, &loader, + "(() => { try { return String(child.location.pathname === '/child' && child.document.readyState === 'complete'); } catch { return 'false'; } })()", + "true", &context, + ).await; + vm.exec( + &format!( + r#" +child.history.replaceState(null, '', child.location.pathname + {fragment:?}); +const originalDocument = child.document; +const beforeLength = child.history.length; +const beforeEntries = child.navigation.entries().length; +const beforeIndex = child.navigation.currentEntry.index; +const events = [], settled = []; +child.navigation.addEventListener('navigate', event => + events.push([event.navigationType, event.destination.sameDocument, event.hashChange])); +const destination = {remove_fragment} ? child.location.href.split('#')[0] : child.location.href; +const result = child.navigation.navigate(destination, {{history: {history:?}}}); +for (const name of ['committed', 'finished']) + result[name].then(() => settled.push(name), error => settled.push(error.name)); +"# + ), + None, + ) + .unwrap(); + advance_page_task_executor_until_eval_equals( + &mut vm, &loader, + if same_document { + "String(settled.length === 2)" + } else { + "(() => { try { return String(child.document !== originalDocument && child.document.readyState === 'complete'); } catch { return 'false'; } })()" + }, + "true", &context, + ).await; + let result = vm + .eval( + r#"JSON.stringify({ +sameDocument: originalDocument === child.document, +historyDelta: child.history.length - beforeLength, +entriesDelta: child.navigation.entries().length - beforeEntries, +indexDelta: child.navigation.currentEntry.index - beforeIndex, +events, settled})"#, + ) + .unwrap(); + let result: serde_json::Value = serde_json::from_str(&result).unwrap(); + let pushes = history == "push" || (history == "auto" && remove_fragment); + assert_eq!( + result["sameDocument"], + serde_json::json!(same_document), + "{context}" + ); + assert_eq!( + result["events"], + serde_json::json!([[ + if pushes { "push" } else { "replace" }, + same_document, + false + ]]), + "{context}" + ); + assert_eq!( + result["settled"], + serde_json::json!(if same_document { + vec!["committed", "finished"] + } else { + vec![] + }), + "{context}" + ); + for key in ["historyDelta", "entriesDelta", "indexDelta"] { + assert_eq!( + result[key], + serde_json::json!(i32::from(pushes)), + "{context}/{key}" + ); + } + assert_eq!( + server.finish_targets().await, + vec!["/child"; request_count], + "{context}" + ); + } + } +}