From 312db04e5e2e68a9c1725548b12352d0a55e84db Mon Sep 17 00:00:00 2001 From: hechang27-sprt Date: Mon, 30 Mar 2026 02:25:37 +0800 Subject: [PATCH] fix: skip wait_for_lifecycle on same-document navigation (#1059) Chrome returns loader_id: None for same-document navigations (e.g., hash routing in SPAs). In these cases, Page.loadEventFired never fires, causing wait_for_lifecycle to hang forever. The fix checks nav_result.loader_id.is_some() before waiting for lifecycle events. Also added regression test e2e_navigate_same_url_twice_should_not_hang. Co-authored-by: Claude Opus 4.6 --- cli/src/native/browser.rs | 9 +++- cli/src/native/e2e_tests.rs | 82 +++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/cli/src/native/browser.rs b/cli/src/native/browser.rs index e25ed2f..1ac1539 100644 --- a/cli/src/native/browser.rs +++ b/cli/src/native/browser.rs @@ -492,8 +492,13 @@ impl BrowserManager { return Err(format!("Navigation failed: {}", error_text)); } - self.wait_for_lifecycle(wait_until, &session_id, &mut lifecycle_rx) - .await?; + // Only wait for lifecycle events if Chrome created a new loader (full navigation). + // If loader_id is None, it was a same-document navigation (e.g., hash routing) + // which does not fire Page.loadEventFired or Page.domContentEventFired. + if nav_result.loader_id.is_some() { + self.wait_for_lifecycle(wait_until, &session_id, &mut lifecycle_rx) + .await?; + } let page_url = self.get_url().await.unwrap_or_else(|_| url.to_string()); let title = self.get_title().await.unwrap_or_default(); diff --git a/cli/src/native/e2e_tests.rs b/cli/src/native/e2e_tests.rs index 7c9483c..0136052 100644 --- a/cli/src/native/e2e_tests.rs +++ b/cli/src/native/e2e_tests.rs @@ -1073,6 +1073,88 @@ async fn e2e_wait() { assert_success(&resp); } +// --------------------------------------------------------------------------- +// Same-document navigation regression test +// --------------------------------------------------------------------------- +// +// Chrome may perform a same-document navigation when it determines the target +// URL is the same document as the current page (ignoring fragment). This +// causes Page.loadEventFired to not fire, making wait_for_lifecycle +// hang forever waiting for an event that never comes. +// +// The fix checks loader_id in the Page.navigate response - if None, +// it's a same-document navigation and we skip waiting for lifecycle events. + +#[tokio::test] +#[ignore] +async fn e2e_navigate_same_url_twice_should_not_hang() { + let mut state = DaemonState::new(); + + let resp = execute_command( + &json!({ "id": "1", "action": "launch", "headless": true }), + &mut state, + ) + .await; + assert_success(&resp); + + // Navigate to about:blank first to start from a known state + let resp = execute_command( + &json!({ "id": "2", "action": "navigate", "url": "about:blank" }), + &mut state, + ) + .await; + assert_success(&resp); + + // Create a simple HTML page that changes its own URL via history.pushState + // This simulates SPA routing behavior which triggers same-document navigation + let base_page = "data:text/html,

Test

"; + + // Navigate to the page (first time) + let resp = execute_command( + &json!({ "id": "3", "action": "navigate", "url": base_page }), + &mut state, + ) + .await; + assert_success(&resp); + + // Verify URL changed due to pushState + let resp = execute_command(&json!({ "id": "4", "action": "url" }), &mut state).await; + assert_success(&resp); + let url_after_push = get_data(&resp)["url"].as_str().unwrap(); + // URL should have changed to include /#/home due to pushState + assert!( + url_after_push.contains("/%23/home") || url_after_push.contains("/#/home"), + "URL should have changed via pushState, got: {}", + url_after_push + ); + + // Navigate to the SAME base URL again + // Without fix: Chrome may do same-document nav, wait_for_lifecycle hangs + // With fix: We detect loader_id is None and skip waiting + let start = std::time::Instant::now(); + let resp = execute_command( + &json!({ "id": "5", "action": "navigate", "url": base_page }), + &mut state, + ) + .await; + let elapsed = start.elapsed().as_secs(); + + // Should complete quickly (< 5 seconds) without hanging + // Without fix, this times out after 25 seconds (default_timeout_ms) + assert!( + elapsed < 5, + "Second navigation should not hang, but took {}s", + elapsed + ); + assert_success(&resp); + + let resp = execute_command(&json!({ "id": "99", "action": "close" }), &mut state).await; + assert_success(&resp); +} + // --------------------------------------------------------------------------- // Viewport with deviceScaleFactor (retina) // ---------------------------------------------------------------------------