Skip to content

Commit 464ed21

Browse files
committed
fix(desktop-browser): label frame dialogs with their own origin and cover reload leave prompts
1 parent 1a4ef00 commit 464ed21

4 files changed

Lines changed: 30 additions & 14 deletions

File tree

‎apps/desktop/e2e/browser-page-dialogs.spec.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,8 @@ test('page dialogs wait for the user on their page and stay automatic for the ag
186186
.getAllWebContents()
187187
.find((candidate) => candidate.getURL().startsWith(url))
188188
if (!contents) throw new Error('No page')
189-
const rect = JSON.parse(
190-
await contents.executeJavaScript(
191-
`JSON.stringify(document.querySelector(${JSON.stringify(selector)}).getBoundingClientRect())`
192-
)
189+
const rect: { x: number; y: number } = await contents.executeJavaScript(
190+
`(() => { const { x, y } = document.querySelector(${JSON.stringify(selector)}).getBoundingClientRect(); return { x, y } })()`
193191
)
194192
const point = { x: Math.round(rect.x + 5), y: Math.round(rect.y + 5) }
195193
contents.sendInputEvent({ type: 'mouseDown', ...point, button: 'left', clickCount: 1 })
@@ -253,6 +251,15 @@ test('page dialogs wait for the user on their page and stay automatic for the ag
253251
expect(await inPage<string>("document.getElementById('draft').value")).toBe('draft')
254252
})
255253

254+
await check('Reload of a draft asks too, and Stay keeps it', async () => {
255+
await panelAction({ action: 'reload' })
256+
await expect.poll(pageDialog).toMatchObject({ kind: 'beforeunload' })
257+
const dialog = await pageDialog()
258+
await panelAction({ action: 'respond-dialog', requestId: dialog?.requestId, allowed: false })
259+
await expect.poll(pageDialog).toBeNull()
260+
expect(await inPage<string>("document.getElementById('draft').value")).toBe('draft')
261+
})
262+
256263
await check('Leave lets the navigation through without asking again', async () => {
257264
await panelAction({ action: 'navigate', url: `${site}/next` })
258265
await expect.poll(pageDialog).toMatchObject({ kind: 'beforeunload' })

‎apps/desktop/src/main/browser-agent/cdp.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export interface CdpCallbacks {
4747
offerToUser: (
4848
kind: 'alert' | 'confirm',
4949
message: string,
50+
frameUrl: string,
5051
respond: (accept: boolean) => void
5152
) => boolean
5253
/** The page closed its dialog itself, by navigating away or crashing. */
@@ -214,7 +215,8 @@ function handleDebuggerEvent(
214215
// action asked for a specific answer. Electron decides beforeunload itself
215216
// (will-prevent-unload), so that kind is only acknowledged here.
216217
if (!requested && (type === 'alert' || type === 'confirm')) {
217-
const offered = callbacks?.offerToUser(type, message, (accept) => {
218+
const frameUrl = typeof params.url === 'string' ? params.url : ''
219+
const offered = callbacks?.offerToUser(type, message, frameUrl, (accept) => {
218220
void answerDialog(contents, { accept }, parentSessionId).then((handled) => {
219221
if (!handled) logger.warn('Could not answer page dialog for the user', { type })
220222
})

‎apps/desktop/src/main/browser-agent/driver.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -617,9 +617,9 @@ function instrumentTab(contents: WebContents): void {
617617
const requested = driverScopeState().dialogResponse
618618
return requested?.contents === contents ? requested.response : null
619619
}),
620-
offerToUser: (kind, message, respond) =>
620+
offerToUser: (kind, message, frameUrl, respond) =>
621621
session.withBrowserScope(scopeId, () =>
622-
session.offerPageDialogToUser(contents, kind, message, respond)
622+
session.offerPageDialogToUser(contents, { kind, message, frameUrl }, respond)
623623
),
624624
onDialogClosed: inScope(() => session.notePageDialogClosed(contents)),
625625
claimUserLeave: () => session.withBrowserScope(scopeId, () => session.claimUserLeave(contents)),

‎apps/desktop/src/main/browser-agent/session.ts‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1901,16 +1901,23 @@ function userOwnsPageDialogs(tab: AgentTab): boolean {
19011901
return automationTabClaimedByUser() && !currentScope.automationActive
19021902
}
19031903

1904+
/**
1905+
* Holds a dialog for the user. `frameUrl` is the frame that opened it, so an
1906+
* embedded site's dialog is labelled with its own origin, not the page's.
1907+
*/
19041908
function holdPageDialogForUser(
19051909
tab: AgentTab,
1906-
kind: BrowserPageDialog['kind'],
1907-
message: string,
1910+
{
1911+
kind,
1912+
message,
1913+
frameUrl,
1914+
}: { kind: BrowserPageDialog['kind']; message: string; frameUrl: string },
19081915
respond: (accept: boolean) => void
19091916
): void {
19101917
if (tab.pageDialog) answerPageDialog(tab, false)
19111918
let origin = ''
19121919
try {
1913-
origin = new URL(tab.view.webContents.getURL()).origin
1920+
origin = new URL(frameUrl || tab.view.webContents.getURL()).origin
19141921
} catch {}
19151922
tab.pageDialog = { request: { requestId: generateId(), kind, message, origin }, respond }
19161923
events?.onPageStateChanged(tab.view.webContents)
@@ -1931,13 +1938,12 @@ function answerPageDialog(tab: AgentTab, accept: boolean): void {
19311938
*/
19321939
export function offerPageDialogToUser(
19331940
contents: WebContents,
1934-
kind: 'alert' | 'confirm',
1935-
message: string,
1941+
dialog: { kind: 'alert' | 'confirm'; message: string; frameUrl: string },
19361942
respond: (accept: boolean) => void
19371943
): boolean {
19381944
const tab = tabForContents(contents)
19391945
if (!tab || !userOwnsPageDialogs(tab)) return false
1940-
holdPageDialogForUser(tab, kind, message, respond)
1946+
holdPageDialogForUser(tab, dialog, respond)
19411947
return true
19421948
}
19431949

@@ -1955,6 +1961,7 @@ export function respondToPageDialog(requestId: string, accept: boolean): void {
19551961
if (tab) answerPageDialog(tab, accept)
19561962
}
19571963

1964+
/** The dialog on this page awaiting the user's answer, if any. */
19581965
export function pageDialogForContents(contents: WebContents): BrowserPageDialog | undefined {
19591966
return tabForContents(contents)?.pageDialog?.request
19601967
}
@@ -1970,7 +1977,7 @@ export function claimUserLeave(contents: WebContents): boolean {
19701977
const leave = tab.pendingLeave
19711978
tab.pendingLeave = undefined
19721979
if (!leave || !userOwnsPageDialogs(tab)) return false
1973-
holdPageDialogForUser(tab, 'beforeunload', '', (accept) => {
1980+
holdPageDialogForUser(tab, { kind: 'beforeunload', message: '', frameUrl: '' }, (accept) => {
19741981
if (!accept) return
19751982
tab.allowNextUnload = true
19761983
leave()

0 commit comments

Comments
 (0)