Skip to content

Commit ce07a73

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

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
@@ -616,9 +616,9 @@ function instrumentTab(contents: WebContents): void {
616616
const requested = driverScopeState().dialogResponse
617617
return requested?.contents === contents ? requested.response : null
618618
}),
619-
offerToUser: (kind, message, respond) =>
619+
offerToUser: (kind, message, frameUrl, respond) =>
620620
session.withBrowserScope(scopeId, () =>
621-
session.offerPageDialogToUser(contents, kind, message, respond)
621+
session.offerPageDialogToUser(contents, { kind, message, frameUrl }, respond)
622622
),
623623
onDialogClosed: inScope(() => session.notePageDialogClosed(contents)),
624624
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
@@ -1895,16 +1895,23 @@ function userOwnsPageDialogs(tab: AgentTab): boolean {
18951895
return automationTabClaimedByUser() && !currentScope.automationActive
18961896
}
18971897

1898+
/**
1899+
* Holds a dialog for the user. `frameUrl` is the frame that opened it, so an
1900+
* embedded site's dialog is labelled with its own origin, not the page's.
1901+
*/
18981902
function holdPageDialogForUser(
18991903
tab: AgentTab,
1900-
kind: BrowserPageDialog['kind'],
1901-
message: string,
1904+
{
1905+
kind,
1906+
message,
1907+
frameUrl,
1908+
}: { kind: BrowserPageDialog['kind']; message: string; frameUrl: string },
19021909
respond: (accept: boolean) => void
19031910
): void {
19041911
if (tab.pageDialog) answerPageDialog(tab, false)
19051912
let origin = ''
19061913
try {
1907-
origin = new URL(tab.view.webContents.getURL()).origin
1914+
origin = new URL(frameUrl || tab.view.webContents.getURL()).origin
19081915
} catch {}
19091916
tab.pageDialog = { request: { requestId: generateId(), kind, message, origin }, respond }
19101917
events?.onPageStateChanged(tab.view.webContents)
@@ -1925,13 +1932,12 @@ function answerPageDialog(tab: AgentTab, accept: boolean): void {
19251932
*/
19261933
export function offerPageDialogToUser(
19271934
contents: WebContents,
1928-
kind: 'alert' | 'confirm',
1929-
message: string,
1935+
dialog: { kind: 'alert' | 'confirm'; message: string; frameUrl: string },
19301936
respond: (accept: boolean) => void
19311937
): boolean {
19321938
const tab = tabForContents(contents)
19331939
if (!tab || !userOwnsPageDialogs(tab)) return false
1934-
holdPageDialogForUser(tab, kind, message, respond)
1940+
holdPageDialogForUser(tab, dialog, respond)
19351941
return true
19361942
}
19371943

@@ -1949,6 +1955,7 @@ export function respondToPageDialog(requestId: string, accept: boolean): void {
19491955
if (tab) answerPageDialog(tab, accept)
19501956
}
19511957

1958+
/** The dialog on this page awaiting the user's answer, if any. */
19521959
export function pageDialogForContents(contents: WebContents): BrowserPageDialog | undefined {
19531960
return tabForContents(contents)?.pageDialog?.request
19541961
}
@@ -1964,7 +1971,7 @@ export function claimUserLeave(contents: WebContents): boolean {
19641971
const leave = tab.pendingLeave
19651972
tab.pendingLeave = undefined
19661973
if (!leave || !userOwnsPageDialogs(tab)) return false
1967-
holdPageDialogForUser(tab, 'beforeunload', '', (accept) => {
1974+
holdPageDialogForUser(tab, { kind: 'beforeunload', message: '', frameUrl: '' }, (accept) => {
19681975
if (!accept) return
19691976
tab.allowNextUnload = true
19701977
leave()

0 commit comments

Comments
 (0)