Skip to content

Commit 4a7ba87

Browse files
authored
improvement(desktop): bring agent browser menus, keys and fullscreen in line with Chrome (#9512)
* improvement(desktop): bring agent browser menus, keys and fullscreen in line with Chrome - Page-menu Back/Forward/Reload go through the user-navigation path, so the leave-site prompt and load-error handling apply - Page menu gains spelling suggestions, Undo/Redo/Cut/Paste and Match Style/Select All, and image items (open, save, copy, copy address); Open Link in New Tab sends the referrer - Spellcheck on in the agent browser on macOS - Ctrl+wheel zoom steps the same zoom ladder as the menu - Chrome keys: Alt+Left/Right, Cmd+Opt+Left/Right, Cmd+Shift+[ ], Ctrl+PgUp/PgDn, Esc stops loading, mouse back/forward buttons - Page fullscreen fills the browser panel without resizing the Sim window * fix(desktop): ignore browser shortcuts and mouse buttons before any chat opens a browser
1 parent 621e4ef commit 4a7ba87

7 files changed

Lines changed: 786 additions & 94 deletions

File tree

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

Lines changed: 445 additions & 0 deletions
Large diffs are not rendered by default.

‎apps/desktop/src/main/browser-agent/context-menu.test.ts‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,18 @@ type Page = Parameters<typeof buildAgentContextMenuTemplate>[1]
2929
type Handlers = Parameters<typeof buildAgentContextMenuTemplate>[2]
3030

3131
function params(overrides: Partial<Params> = {}): Params {
32-
return { selectionText: '', linkURL: '', isEditable: false, editFlags: EDIT_FLAGS, ...overrides }
32+
return {
33+
selectionText: '',
34+
linkURL: '',
35+
isEditable: false,
36+
editFlags: EDIT_FLAGS,
37+
misspelledWord: '',
38+
dictionarySuggestions: [],
39+
mediaType: 'none',
40+
srcURL: '',
41+
hasImageContents: false,
42+
...overrides,
43+
}
3344
}
3445

3546
function page(overrides: Partial<Page> = {}): Page {
@@ -46,13 +57,20 @@ function page(overrides: Partial<Page> = {}): Page {
4657
function handlers(): Handlers {
4758
return {
4859
addToChat: vi.fn(),
60+
replaceMisspelling: vi.fn(),
61+
addToDictionary: vi.fn(),
62+
undo: vi.fn(),
63+
redo: vi.fn(),
64+
cut: vi.fn(),
4965
copy: vi.fn(),
5066
paste: vi.fn(),
51-
back: vi.fn(),
52-
forward: vi.fn(),
53-
reload: vi.fn(),
67+
pasteAndMatchStyle: vi.fn(),
68+
selectAll: vi.fn(),
69+
navigate: vi.fn(),
5470
openTab: vi.fn(),
5571
copyLink: vi.fn(),
72+
copyImage: vi.fn(),
73+
saveImage: vi.fn(),
5674
setZoomFactor: vi.fn(),
5775
}
5876
}
@@ -82,11 +100,16 @@ describe('buildAgentContextMenuTemplate', () => {
82100
})
83101

84102
describe('attachAgentContextMenu', () => {
103+
const navigationHost = {
104+
history: () => ({ canGoBack: false, canGoForward: false }),
105+
navigate: vi.fn(),
106+
}
85107
type ContextMenuListener = (event: unknown, params: Params) => void
86108

87109
it('suppresses one agent context menu and immediately allows the next menu', async () => {
88110
const contents = new WebContentsView().webContents
89111
attachAgentContextMenu(contents, {
112+
...navigationHost,
90113
addToChat: vi.fn(),
91114
openTab: vi.fn(),
92115
defaultZoomFactor: () => BASE_ZOOM_FACTOR,
@@ -110,6 +133,7 @@ describe('attachAgentContextMenu', () => {
110133
async (inputEvent) => {
111134
const contents = new WebContentsView().webContents
112135
attachAgentContextMenu(contents, {
136+
...navigationHost,
113137
addToChat: vi.fn(),
114138
openTab: vi.fn(),
115139
defaultZoomFactor: () => BASE_ZOOM_FACTOR,

‎apps/desktop/src/main/browser-agent/context-menu.ts‎

Lines changed: 133 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,20 @@
1010
* while the page keeps rendering, so none of that applies.
1111
*
1212
* It is also what a browser's page menu is everywhere else, and unlike the
13-
* terminal's hidden textarea, the roles here act on a real page: `copy` and
13+
* terminal's hidden textarea, the edit items act on a real page: `copy` and
1414
* `paste` go to the frame that was clicked.
1515
*/
1616

1717
import { resolveDesktopZoom } from '@sim/desktop-bridge'
18-
import type { ContextMenuParams, MenuItemConstructorOptions, WebContents } from 'electron'
18+
import type { ContextMenuParams, MenuItemConstructorOptions, Referrer, WebContents } from 'electron'
1919
import { clipboard, Menu } from 'electron'
2020
import {
2121
clearAgentContextMenu,
2222
consumeAgentContextMenu,
2323
isDispatchingAgentInput,
2424
} from '@/main/browser-agent/cdp'
25+
import { spellingItems } from '@/main/context-menu'
26+
import { parseHttpUrl } from '@/main/navigation'
2527

2628
/**
2729
* Page-zoom ladder for the embedded browser, in Chromium's absolute zoom
@@ -78,7 +80,15 @@ export function steppedZoomFactor(current: number, direction: 1 | -1): number {
7880
/** The parts of a right-click the menu acts on. */
7981
type AgentContextMenuParams = Pick<
8082
ContextMenuParams,
81-
'selectionText' | 'linkURL' | 'isEditable' | 'editFlags'
83+
| 'selectionText'
84+
| 'linkURL'
85+
| 'isEditable'
86+
| 'editFlags'
87+
| 'misspelledWord'
88+
| 'dictionarySuggestions'
89+
| 'mediaType'
90+
| 'srcURL'
91+
| 'hasImageContents'
8292
>
8393

8494
/** What the page can currently do, read at the moment of the click. */
@@ -89,43 +99,103 @@ interface AgentPageContext {
8999
defaultZoomFactor: number
90100
}
91101

102+
/** A history move or reload started from browser chrome rather than by the page. */
103+
export type PageNavigation = 'back' | 'forward' | 'reload'
104+
105+
/**
106+
* Edit actions are explicit calls on this page rather than menu roles: a role
107+
* acts on whichever contents holds focus, and a right-click need not focus the page.
108+
*/
92109
interface AgentContextMenuHandlers {
93110
addToChat(text: string): void
111+
replaceMisspelling(word: string): void
112+
addToDictionary(word: string): void
113+
undo(): void
114+
redo(): void
115+
cut(): void
94116
copy(): void
95117
paste(): void
96-
back(): void
97-
forward(): void
98-
reload(): void
118+
pasteAndMatchStyle(): void
119+
selectAll(): void
120+
navigate(kind: PageNavigation): void
99121
openTab(url: string): void
100122
copyLink(url: string): void
123+
copyImage(): void
124+
saveImage(url: string): void
101125
setZoomFactor(factor: number): void
102126
}
103127

104128
export interface AgentContextMenuHost {
105129
/** Attaches selected page text to the chat that owns this browser tab. */
106130
addToChat(text: string): void
107-
/** Opens a link from the page in another tab of the same browser. */
108-
openTab(url: string): void
131+
/** Opens a URL from the page in another tab of the same browser. */
132+
openTab(url: string, referrer: Referrer): void
109133
/** Returns the device's current default page zoom factor. */
110134
defaultZoomFactor(): number
135+
history(): Pick<AgentPageContext, 'canGoBack' | 'canGoForward'>
136+
/** Runs through the user-navigation path, so a page's leave-site question reaches the user. */
137+
navigate(kind: PageNavigation): void
138+
}
139+
140+
const isWebUrl = (url: string) => parseHttpUrl(url) !== null
141+
142+
const REFERRER_POLICIES: ReadonlySet<unknown> = new Set<Referrer['policy']>([
143+
'default',
144+
'unsafe-url',
145+
'no-referrer-when-downgrade',
146+
'no-referrer',
147+
'origin',
148+
'strict-origin-when-cross-origin',
149+
'same-origin',
150+
'strict-origin',
151+
])
152+
153+
function isReferrerPolicy(value: unknown): value is Referrer['policy'] {
154+
return REFERRER_POLICIES.has(value)
155+
}
156+
157+
/** Electron types `referrerPolicy` as a Referrer, but it arrives as the bare policy string. */
158+
function menuReferrer(
159+
params: Pick<ContextMenuParams, 'referrerPolicy' | 'frameURL' | 'pageURL'>
160+
): Referrer {
161+
const policy: unknown = params.referrerPolicy
162+
const url = params.frameURL || params.pageURL
163+
return {
164+
url: isWebUrl(url) ? url : '',
165+
policy: isReferrerPolicy(policy) ? policy : 'default',
166+
}
167+
}
168+
169+
/** Ends the template with one separator, never two in a row or one at the top. */
170+
function endGroup(template: MenuItemConstructorOptions[]): void {
171+
if (template.length > 0 && template[template.length - 1].type !== 'separator') {
172+
template.push({ type: 'separator' })
173+
}
111174
}
112175

113176
/**
114177
* Builds the page menu. Unlike the main window's menu this is never empty —
115-
* navigation and zoom always apply, and only the clipboard and link items
116-
* depend on what was clicked.
178+
* navigation and zoom always apply, and only the spelling, clipboard, link, and
179+
* image items depend on what was clicked.
117180
*
118-
* Only http(s) links get link items: the actions open a browser tab or copy an
119-
* address, and neither means anything for a `javascript:` or `mailto:` target.
181+
* Only http(s) links and image sources get address items: the actions open a
182+
* browser tab, download, or copy an address, and none of them means anything
183+
* for a `javascript:`, `data:`, or `blob:` target. Copy Image needs no address,
184+
* so it is offered for any loaded image.
120185
*/
121186
export function buildAgentContextMenuTemplate(
122187
params: AgentContextMenuParams,
123188
page: AgentPageContext,
124189
handlers: AgentContextMenuHandlers
125190
): MenuItemConstructorOptions[] {
126191
const template: MenuItemConstructorOptions[] = []
127-
const linkUrl = /^https?:\/\//i.test(params.linkURL) ? params.linkURL : ''
192+
const linkUrl = isWebUrl(params.linkURL) ? params.linkURL : ''
193+
const imageUrl = isWebUrl(params.srcURL) ? params.srcURL : ''
128194
const selectionText = params.selectionText
195+
const flags = params.editFlags
196+
197+
template.push(...spellingItems(params, handlers))
198+
endGroup(template)
129199

130200
if (selectionText.trim()) {
131201
template.push(
@@ -142,20 +212,47 @@ export function buildAgentContextMenuTemplate(
142212
)
143213
}
144214

145-
if (selectionText.trim()) {
146-
template.push({ label: 'Copy', click: () => handlers.copy() })
147-
}
148-
if (params.isEditable && params.editFlags.canPaste) {
149-
template.push({ label: 'Paste', click: () => handlers.paste() })
215+
if (params.mediaType === 'image') {
216+
const copyImage: MenuItemConstructorOptions[] = params.hasImageContents
217+
? [{ label: 'Copy Image', click: () => handlers.copyImage() }]
218+
: []
219+
template.push(
220+
...(imageUrl
221+
? [
222+
{ label: 'Open Image in New Tab', click: () => handlers.openTab(imageUrl) },
223+
{ label: 'Save Image', click: () => handlers.saveImage(imageUrl) },
224+
...copyImage,
225+
{ label: 'Copy Image Address', click: () => handlers.copyLink(imageUrl) },
226+
]
227+
: copyImage)
228+
)
229+
endGroup(template)
150230
}
151-
if (template.length > 0 && template[template.length - 1].type !== 'separator') {
152-
template.push({ type: 'separator' })
231+
232+
if (params.isEditable) {
233+
template.push(
234+
{ label: 'Undo', enabled: flags.canUndo, click: () => handlers.undo() },
235+
{ label: 'Redo', enabled: flags.canRedo, click: () => handlers.redo() },
236+
{ type: 'separator' },
237+
{ label: 'Cut', enabled: flags.canCut, click: () => handlers.cut() },
238+
{ label: 'Copy', enabled: flags.canCopy, click: () => handlers.copy() },
239+
{ label: 'Paste', enabled: flags.canPaste, click: () => handlers.paste() },
240+
{
241+
label: 'Paste and Match Style',
242+
enabled: flags.canPaste,
243+
click: () => handlers.pasteAndMatchStyle(),
244+
},
245+
{ label: 'Select All', enabled: flags.canSelectAll, click: () => handlers.selectAll() }
246+
)
247+
} else if (selectionText.trim()) {
248+
template.push({ label: 'Copy', click: () => handlers.copy() })
153249
}
250+
endGroup(template)
154251

155252
template.push(
156-
{ label: 'Back', enabled: page.canGoBack, click: () => handlers.back() },
157-
{ label: 'Forward', enabled: page.canGoForward, click: () => handlers.forward() },
158-
{ label: 'Reload', click: () => handlers.reload() },
253+
{ label: 'Back', enabled: page.canGoBack, click: () => handlers.navigate('back') },
254+
{ label: 'Forward', enabled: page.canGoForward, click: () => handlers.navigate('forward') },
255+
{ label: 'Reload', click: () => handlers.navigate('reload') },
159256
{ type: 'separator' }
160257
)
161258

@@ -198,20 +295,27 @@ export function attachAgentContextMenu(contents: WebContents, host: AgentContext
198295
const template = buildAgentContextMenuTemplate(
199296
params,
200297
{
201-
canGoBack: contents.navigationHistory.canGoBack(),
202-
canGoForward: contents.navigationHistory.canGoForward(),
298+
...host.history(),
203299
zoomFactor: contents.getZoomFactor(),
204300
defaultZoomFactor: host.defaultZoomFactor(),
205301
},
206302
{
207303
addToChat: (text) => host.addToChat(text),
304+
replaceMisspelling: (word) => contents.replaceMisspelling(word),
305+
addToDictionary: (word) => contents.session.addWordToSpellCheckerDictionary(word),
306+
undo: () => contents.undo(),
307+
redo: () => contents.redo(),
308+
cut: () => contents.cut(),
208309
copy: () => contents.copy(),
209310
paste: () => contents.paste(),
210-
back: () => contents.navigationHistory.goBack(),
211-
forward: () => contents.navigationHistory.goForward(),
212-
reload: () => contents.reload(),
213-
openTab: (url) => host.openTab(url),
311+
pasteAndMatchStyle: () => contents.pasteAndMatchStyle(),
312+
selectAll: () => contents.selectAll(),
313+
navigate: (kind) => host.navigate(kind),
314+
openTab: (url) => host.openTab(url, menuReferrer(params)),
214315
copyLink: (url) => clipboard.writeText(url),
316+
copyImage: () => contents.copyImageAt(params.x, params.y),
317+
// A download, so the browser's download limits and request guards apply.
318+
saveImage: (url) => contents.downloadURL(url),
215319
setZoomFactor: (factor) => contents.setZoomFactor(factor),
216320
}
217321
)

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,13 @@ describe('browser-agent session', () => {
352352
session = freshSession(win)
353353
})
354354

355+
it('ignores browser shortcuts and mouse buttons before any chat has opened a browser', () => {
356+
session.disposeBrowserScope('chat-test')
357+
358+
expect(session.handleFocusedShortcut('back', win)).toBe(false)
359+
expect(session.handleFocusedShortcut('close-tab', win)).toBe(false)
360+
})
361+
355362
it('invalidates only top-frame starts and identifies same-document commits', () => {
356363
const onTabNavigated = vi.fn()
357364
session = freshSession(win, { onTabNavigated })

0 commit comments

Comments
 (0)