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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
369f48752a
commit
312db04e5e
@@ -492,8 +492,13 @@ impl BrowserManager {
|
|||||||
return Err(format!("Navigation failed: {}", error_text));
|
return Err(format!("Navigation failed: {}", error_text));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 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)
|
self.wait_for_lifecycle(wait_until, &session_id, &mut lifecycle_rx)
|
||||||
.await?;
|
.await?;
|
||||||
|
}
|
||||||
|
|
||||||
let page_url = self.get_url().await.unwrap_or_else(|_| url.to_string());
|
let page_url = self.get_url().await.unwrap_or_else(|_| url.to_string());
|
||||||
let title = self.get_title().await.unwrap_or_default();
|
let title = self.get_title().await.unwrap_or_default();
|
||||||
|
|||||||
@@ -1073,6 +1073,88 @@ async fn e2e_wait() {
|
|||||||
assert_success(&resp);
|
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,<html><body><script>
|
||||||
|
// On first load, change URL via pushState without navigation
|
||||||
|
history.pushState({}, '', '/#/home');
|
||||||
|
</script><h1>Test</h1></body></html>";
|
||||||
|
|
||||||
|
// 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)
|
// Viewport with deviceScaleFactor (retina)
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user