Skip to content

fix(browser): dismiss a page's beforeunload prompt instead of wedging the session - #2257

Open
Benjamin-eecs wants to merge 1 commit into
jackwener:mainfrom
Benjamin-eecs:fix/browser-beforeunload-guard
Open

fix(browser): dismiss a page's beforeunload prompt instead of wedging the session#2257
Benjamin-eecs wants to merge 1 commit into
jackwener:mainfrom
Benjamin-eecs:fix/browser-beforeunload-guard

Conversation

@Benjamin-eecs

@Benjamin-eecs Benjamin-eecs commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Description

A page that arms beforeunload can wedge a browser session: the next navigation opens the native "Leave site?" prompt, nothing in the CLI can answer it (the daemon owns the CDP session, so browser dialog dismiss reports no dialog), navigation never returns, and every later evaluate reads an empty document. Any adapter that clicks before navigating can supply the user gesture the prompt needs; twitter post wedged a session for me twice.

Both attach paths, the extension and the direct-CDP CDPBridge, install two halves: a guard registered through Page.addScriptToEvaluateOnNewDocument, early enough to stop the site's own handler, and a Page.javascriptDialogOpening handler that accepts only beforeunload dialogs, covering documents already loaded at attach. alert, confirm and prompt stay with browser dialog.

Related issue: none filed; #1972 is @biospark33's twitter-scoped PR for the same wedge, and this installs the guard for every adapter in the layer that runs before page scripts.

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 🌐 New site adapter
  • 📝 Documentation
  • ♻️ Refactor
  • 🔧 CI / build / tooling

Checklist

  • I ran the checks relevant to this PR
  • I updated tests or docs if needed
  • I included output or screenshots when useful

Documentation (if adding/modifying an adapter)

  • Added doc page under docs/adapters/ (if new adapter)
  • Updated docs/adapters/index.md table (if new adapter)
  • Updated sidebar in docs/.vitepress/config.mts (if new adapter)
  • Updated README.md / README.zh-CN.md when command discoverability changed
  • Used positional args for the command's primary subject unless a named flag is clearly better
  • Normalized expected adapter failures to CliError subclasses instead of raw Error

Screenshots / Output

Navigation-level A/B on Chrome 151.0.7922.76 over raw CDP: a page arms beforeunload at load, receives a real click for the user gesture, then navigates away. Without the guard the prompt opens and the navigation never completes, which is the wedge; with the guard pre-injected the navigation completes with no dialog:

UNGUARDED {"dialogOpened":true,"dialogType":"beforeunload","navigationCompleted":false}
GUARDED   {"dialogOpened":false,"dialogType":null,"navigationCompleted":true}

npx vitest run src/browser 433 / 433 and the extension suite 99 / 99, with tests for the guard, the injection at attach, the CDPBridge dialog accept, keeping the two guard copies in step, and leaving non-beforeunload dialogs alone. Typecheck, both lint gates and doc coverage pass. extension/dist/background.js is rebuilt, as the repo tracks it.

Not verified end to end through a loaded build of this extension, since the daemon tracks one extension connection at a time and the installed build holds the slot.

@Benjamin-eecs
Benjamin-eecs force-pushed the fix/browser-beforeunload-guard branch from 3b043ac to ad31816 Compare August 5, 2026 10:37
@Benjamin-eecs Benjamin-eecs changed the title fix(browser): keep a page's beforeunload prompt from wedging the session fix(browser): dismiss a page's beforeunload prompt instead of wedging the session Aug 5, 2026
@Benjamin-eecs
Benjamin-eecs marked this pull request as ready for review August 5, 2026 10:46
Copilot AI lite review requested due to automatic review settings August 5, 2026 10:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Benjamin-eecs
Benjamin-eecs force-pushed the fix/browser-beforeunload-guard branch 2 times, most recently from 0a8515e to dce1af7 Compare August 6, 2026 02:15
@Benjamin-eecs
Benjamin-eecs force-pushed the fix/browser-beforeunload-guard branch from dce1af7 to 763b87c Compare August 8, 2026 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants