From ca2cc82cd68a456eb66101e6a11fcfe30bea5c59 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Wed, 21 Oct 2026 18:53:01 +0800 Subject: [PATCH] fix(page): load a new document when navigation removes a fragment Require a destination fragment in the protocol fragment classifier so X#frag -> X loads a fresh document instead of calling a renderer path that rejects the destination. Cover named and empty source fragments, new loader/request, Runtime context replacement and Page load events. Extracted from 3af340a5f72504dde0df1dc99651c71b0ee3a7c3 onto main 8d57e02b3c578bf63188cbfdcae78dce8329da85 as an independent fix. Validated cargo fmt --all, strict workspace/all-targets/all-features Clippy and cargo nextest run --no-fail-fast: 20,489 passed, 18 skipped. Live Playwright/Puppeteer CDP tests pass for both source fragments, matching Chromium's new-request/document and load-event behavior. --- moli-protocol/src/domains/page/navigation.rs | 8 +- .../domains/page/tests/navigation/requests.rs | 103 ++++++++++++++++++ 2 files changed, 107 insertions(+), 4 deletions(-) diff --git a/moli-protocol/src/domains/page/navigation.rs b/moli-protocol/src/domains/page/navigation.rs index 103ff3a186..4b64f14f1b 100644 --- a/moli-protocol/src/domains/page/navigation.rs +++ b/moli-protocol/src/domains/page/navigation.rs @@ -1416,10 +1416,10 @@ fn session_owner_navigation_is_same_document_fragment( let (Ok(mut current), Ok(mut target)) = (Url::parse(¤t), Url::parse(target)) else { return false; }; - // Chromium treats an exact repeat of a fragment-free URL as a new - // document navigation. Removing or changing a fragment is same-document, - // including a repeat of the exact same fragment URL. - if current.fragment().is_none() && target.fragment().is_none() { + // A fragment navigation requires a fragment in the destination, even an + // empty one. Removing a fragment loads a new document, as does repeating a + // fragment-free URL. History traversal has separate same-document rules. + if target.fragment().is_none() { return false; } current.set_fragment(None); diff --git a/moli-protocol/src/domains/page/tests/navigation/requests.rs b/moli-protocol/src/domains/page/tests/navigation/requests.rs index b1a6b3bb51..ab53000c59 100644 --- a/moli-protocol/src/domains/page/tests/navigation/requests.rs +++ b/moli-protocol/src/domains/page/tests/navigation/requests.rs @@ -520,6 +520,109 @@ async fn navigate_passes_referrer_header() { server.abort(); } +#[tokio::test(flavor = "multi_thread")] +async fn protocol_navigate_removing_fragment_loads_a_new_document() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let request_count = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let server_request_count = request_count.clone(); + let server = tokio::spawn(async move { + axum::serve( + listener, + axum::Router::new().route( + "/page", + axum::routing::get(move || { + let request_count = server_request_count.clone(); + async move { + request_count.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + axum::response::Html("loaded") + } + }), + ), + ) + .await + .unwrap(); + }); + let url = format!("http://{addr}/page"); + for fragment in ["#frag", "#"] { + let mut ctx = TestContext::new(); + load_bc_with_session(&mut ctx, "BID-1", "TID-1", "SID-1", "about:blank"); + ensure_initial_document_for_session(&mut ctx, Some("SID-1")).await; + for (id, method) in [(410, "Page.enable"), (411, "Runtime.enable")] { + ctx.process_async(json!({"id":id,"method":method,"sessionId":"SID-1"})) + .await; + ctx.expect_result(id, json!({}), Some("SID-1")); + } + ctx.sent.clear(); + ctx.process_async(json!({ + "id": 412, + "method": "Page.navigate", + "sessionId": "SID-1", + "params": { "url": format!("{url}{fragment}") } + })) + .await; + let initial = take_response_by_id(&mut ctx, 412); + let initial_loader = initial["result"]["loaderId"] + .as_str() + .expect("initial loader"); + wait_until_message( + &mut ctx, + Some("SID-1"), + "initial document load", + |message| message["method"] == "Page.loadEventFired", + ) + .await; + ctx.sent.clear(); + + ctx.process_async(json!({ + "id": 413, + "method": "Page.navigate", + "sessionId": "SID-1", + "params": { "url": url } + })) + .await; + let navigated = take_response_by_id(&mut ctx, 413); + let loader = navigated["result"]["loaderId"] + .as_str() + .expect("removing a fragment must return a new document loader"); + assert_ne!(loader, initial_loader); + wait_until_message( + &mut ctx, + Some("SID-1"), + "fragment removal commit", + |message| { + message["method"] == "Page.frameNavigated" + && message["params"]["frame"]["url"] == url + }, + ) + .await; + wait_until_message( + &mut ctx, + Some("SID-1"), + "fragment removal load", + |message| message["method"] == "Page.loadEventFired", + ) + .await; + assert!( + !ctx.sent + .iter() + .any(|message| message["method"] == "Page.navigatedWithinDocument") + ); + assert_runtime_navigation_context_reset(&ctx.sent, "SID-1", "TID-1"); + assert_eq!( + ctx.conn.browser_context.as_ref().unwrap().target_url(), + url, + "the committed URL must no longer contain {fragment}" + ); + } + assert_eq!( + request_count.load(std::sync::atomic::Ordering::SeqCst), + 4, + "both fragment removals must fetch a fresh document" + ); + server.abort(); +} + #[tokio::test(flavor = "multi_thread")] async fn repeated_protocol_navigate_to_same_url_does_not_inherit_referrer_or_reload_headers() { async fn page(headers: axum::http::HeaderMap) -> impl axum::response::IntoResponse {