Restore desktop terminal URL modifier clicks

Restore desktop terminal URL and OSC link activation to require Cmd/Ctrl while preserving mobile tap routing.
This commit is contained in:
Neil
2026-07-03 01:32:22 -07:00
committed by GitHub
parent 471881c14f
commit 7c9059c4d4
9 changed files with 162 additions and 42 deletions
@@ -0,0 +1,6 @@
export function isTerminalLinkActivation(
event: Pick<MouseEvent, 'metaKey' | 'ctrlKey'> | undefined
): boolean {
const isMac = navigator.userAgent.includes('Mac')
return isMac ? Boolean(event?.metaKey) : Boolean(event?.ctrlKey)
}
@@ -7,6 +7,7 @@ import {
createFilePathLinkProvider,
getTerminalFileOpenHint,
getTerminalHtmlFileOpenHint,
getTerminalUrlOpenHint,
installFilePathLinkClickFallback,
isTerminalLinkActivation,
openFilePathLinkAtBufferPosition,
@@ -156,12 +157,28 @@ describe('isTerminalLinkActivation', () => {
})
describe('handleOscLink', () => {
it('routes http links on ordinary click', () => {
it('ignores http links without the platform modifier on desktop', () => {
setPlatform('Macintosh')
storeState.settings = { openLinksInApp: true }
const preventDefault = vi.fn()
handleOscLink('https://example.com', { metaKey: false, ctrlKey: false, preventDefault }, deps)
expect(
handleOscLink('https://example.com', { metaKey: false, ctrlKey: false, preventDefault }, deps)
).toBe(false)
expect(openUrlMock).not.toHaveBeenCalled()
expect(createBrowserTabMock).not.toHaveBeenCalled()
expect(preventDefault).not.toHaveBeenCalled()
})
it('routes http links with the platform modifier on desktop', () => {
setPlatform('Macintosh')
storeState.settings = { openLinksInApp: true }
const preventDefault = vi.fn()
expect(
handleOscLink('https://example.com', { metaKey: true, ctrlKey: false, preventDefault }, deps)
).toBe(true)
expect(openUrlMock).not.toHaveBeenCalled()
expect(createBrowserTabMock).toHaveBeenCalledWith('wt-1', 'https://example.com/', {
@@ -438,18 +455,38 @@ describe('handleOscLink', () => {
setPlatform('Macintosh')
expect(getTerminalFileOpenHint()).toBe('⌘+click to open or ⇧⌘+click for default app')
expect(getTerminalHtmlFileOpenHint()).toBe('⌘+click to open or ⇧⌘+click for default browser')
expect(getTerminalUrlOpenHint()).toBe('⌘+click to open or ⇧⌘+click for system browser')
setPlatform('Windows')
expect(getTerminalFileOpenHint()).toBe('Ctrl+click to open or Shift+Ctrl+click for default app')
expect(getTerminalHtmlFileOpenHint()).toBe(
'Ctrl+click to open or Shift+Ctrl+click for default browser'
)
expect(getTerminalUrlOpenHint()).toBe(
'Ctrl+click to open or Shift+Ctrl+click for system browser'
)
})
it('opens local file URL links in Orca on ordinary click', async () => {
it('ignores local file URL links without the platform modifier on desktop', async () => {
setPlatform('Windows')
handleOscLink('file:///tmp/test.txt', { metaKey: false, ctrlKey: false }, deps)
expect(handleOscLink('file:///tmp/test.txt', { metaKey: false, ctrlKey: false }, deps)).toBe(
false
)
await new Promise((resolve) => setTimeout(resolve, 0))
expect(authorizeExternalPathMock).not.toHaveBeenCalled()
expect(openFileMock).not.toHaveBeenCalled()
expect(openFilePathMock).not.toHaveBeenCalled()
})
it('opens local file URL links in Orca with the platform modifier on desktop', async () => {
setPlatform('Windows')
expect(handleOscLink('file:///tmp/test.txt', { metaKey: false, ctrlKey: true }, deps)).toBe(
true
)
// openDetectedFilePath is async (fire-and-forget), so flush the microtask queue
// before asserting on positive behavior.
@@ -1708,7 +1745,7 @@ describe('createFilePathLinkProvider range bounds', () => {
disposable.dispose()
})
it('opens regular URLs from a direct ordinary-click fallback when xterm did not handle them', async () => {
it('ignores regular URLs from a direct ordinary-click fallback on desktop', async () => {
setPlatform('Macintosh')
storeState.settings = { openLinksInApp: false }
const rows = [
@@ -1732,6 +1769,39 @@ describe('createFilePathLinkProvider range bounds', () => {
stopPropagation
} as unknown as MouseEvent)
expect(openUrlMock).not.toHaveBeenCalled()
expect(preventDefault).not.toHaveBeenCalled()
expect(stopPropagation).not.toHaveBeenCalled()
expect(terminal.clearSelection).not.toHaveBeenCalled()
disposable.dispose()
expect(element.removeEventListener).toHaveBeenCalledWith('mouseup', mouseUp)
})
it('opens regular URLs from a direct modifier-click fallback when xterm did not handle them', async () => {
setPlatform('Macintosh')
storeState.settings = { openLinksInApp: false }
const rows = [
makeBufferLine('PR opened: https://github.com/stablyai/orca-marketing-website/pull/82')
]
const { terminal, element } = makeFallbackTerminal(rows)
const disposable = installHttpLinkClickFallback(terminal, { worktreeId: 'wt-1' })
const mouseUp = getRegisteredBubbleMouseUpHandler(element)
const preventDefault = vi.fn()
const stopPropagation = vi.fn()
mouseUp({
button: 0,
metaKey: true,
ctrlKey: false,
shiftKey: false,
defaultPrevented: false,
clientX: 230,
clientY: 25,
preventDefault,
stopPropagation
} as unknown as MouseEvent)
expect(openUrlMock).toHaveBeenCalledWith(
'https://github.com/stablyai/orca-marketing-website/pull/82'
)
@@ -33,14 +33,15 @@ import {
getTerminalOrcaFileOpenHint,
getTerminalWorktreePathOpenHint,
getTerminalFileOpenHint,
getTerminalUrlOpenHint,
isMacPlatform
getTerminalUrlOpenHint
} from './terminal-link-open-hints'
import { resolveKnownWorktreeRootPathLink } from './terminal-worktree-path-link'
import { isTerminalLinkActivation } from './terminal-link-activation'
export { openDetectedFilePath } from './terminal-file-open-routing'
export { openFilePathLinkAtBufferPosition } from './terminal-file-link-hit-testing'
export { getTerminalFileOpenHint, getTerminalHtmlFileOpenHint, getTerminalUrlOpenHint }
export { isTerminalLinkActivation } from './terminal-link-activation'
export type LinkHandlerDeps = {
worktreeId: string
@@ -315,10 +316,3 @@ export function installFilePathLinkClickFallback(
}
}
}
export function isTerminalLinkActivation(
event: Pick<MouseEvent, 'metaKey' | 'ctrlKey'> | undefined
): boolean {
const isMac = isMacPlatform()
return isMac ? Boolean(event?.metaKey) : Boolean(event?.ctrlKey)
}
@@ -22,8 +22,8 @@ export function getTerminalHtmlFileOpenHint(): string {
export function getTerminalUrlOpenHint(): string {
return isMacPlatform()
? 'click to open or ⇧+click for system browser'
: 'click to open or Shift+click for system browser'
? '⌘+click to open or ⇧+click for system browser'
: 'Ctrl+click to open or Shift+Ctrl+click for system browser'
}
export function getTerminalUrlSystemBrowserHint(): string {
@@ -3,6 +3,7 @@ import { isWindowsAbsolutePathLike } from '../../../../shared/cross-platform-pat
import type { LinkHandlerDeps } from './terminal-link-handlers'
import { resolveTerminalFileUrlTarget } from './terminal-file-url-target'
import { openDetectedFilePath } from './terminal-file-open-routing'
import { isTerminalLinkActivation } from './terminal-link-activation'
import {
openTerminalHttpLink,
type TerminalLinkRoutingPreferenceRequester
@@ -11,16 +12,16 @@ import {
type TerminalLinkEvent = Pick<MouseEvent, 'metaKey' | 'ctrlKey'> &
Partial<Pick<MouseEvent, 'button' | 'shiftKey' | 'preventDefault' | 'stopPropagation'>>
function isPrimaryOscLinkActivation(event: TerminalLinkEvent | undefined): boolean {
function isDesktopOscLinkActivation(event: TerminalLinkEvent | undefined): boolean {
if (!event) {
return false
}
if ('button' in event && event.button !== undefined && event.button !== 0) {
return false
}
// Why: macOS Ctrl-click is a context-menu gesture even when Chromium reports
// it as button 0; ordinary OSC links should not steal that secondary action.
return !(navigator.userAgent.includes('Mac') && event.ctrlKey && !event.metaKey)
// Why: desktop xterm links must not open while the user is just placing the
// cursor or selecting text. Mobile URL taps use a separate WebView path.
return isTerminalLinkActivation(event)
}
export function handleOscLink(
@@ -30,9 +31,9 @@ export function handleOscLink(
Partial<Pick<LinkHandlerDeps, 'runtimeEnvironmentId' | 'startupCwd' | 'terminalHomePath'>> & {
requestOpenLinksInAppPreference?: TerminalLinkRoutingPreferenceRequester
}
): void {
if (!isPrimaryOscLinkActivation(event)) {
return
): boolean {
if (!isDesktopOscLinkActivation(event)) {
return false
}
// Why: xterm renders OSC 8 links as clickable anchors. Orca must suppress
// default anchor navigation so link-routing settings can choose the target.
@@ -67,15 +68,14 @@ export function handleOscLink(
) {
// Why: `new URL("C:\\path\\file.ts")` succeeds with protocol `c:`;
// Windows OSC links need file-path routing before generic URL parsing.
return
return true
}
let parsed: URL
try {
parsed = new URL(rawText)
} catch {
openDetectedPathLink()
return
return openDetectedPathLink()
}
if (parsed.protocol === 'http:' || parsed.protocol === 'https:') {
@@ -84,7 +84,7 @@ export function handleOscLink(
forceSystemBrowser: Boolean(event?.shiftKey),
requestOpenLinksInAppPreference: deps.requestOpenLinksInAppPreference
})
return
return true
}
if (parsed.protocol === 'file:') {
@@ -99,11 +99,13 @@ export function handleOscLink(
!deps.runtimeEnvironmentId
const resolved = resolveTerminalFileUrlTarget(parsed, { allowUncHost })
if (!resolved) {
return
return false
}
openDetectedFilePath(resolved.filePath, resolved.line, resolved.column, {
...deps,
openWithSystemDefault: Boolean(event?.shiftKey)
})
return true
}
return false
}
@@ -1,6 +1,7 @@
import type { IBufferLine, IBufferRange, IDisposable, Terminal } from '@xterm/xterm'
import { openHttpLink } from '@/lib/http-link-routing'
import { buildCandidateLogicalLinesForBufferPosition } from './terminal-file-link-hit-testing'
import { isTerminalLinkActivation } from './terminal-link-activation'
import { rangeForParsedFileLink } from './wrapped-terminal-link-ranges'
type UrlLinkHitTestDeps = {
@@ -48,13 +49,14 @@ export function extractTerminalHttpLinks(lineText: string): ParsedTerminalHttpLi
return links
}
function isPrimaryHttpLinkFallbackActivation(event: MouseEvent): boolean {
function isDesktopHttpLinkFallbackActivation(event: MouseEvent): boolean {
if (event.defaultPrevented || event.button !== 0) {
return false
}
// Why: URL links now open on ordinary clicks, but macOS Ctrl-click must stay
// available for context menus even when Chromium reports it as button 0.
return !(navigator.userAgent.includes('Mac') && event.ctrlKey && !event.metaKey)
// Why: desktop terminal links require an intentional Cmd/Ctrl gesture so
// plain clicks remain available for cursor placement and selection. Mobile
// tap routing is handled separately under mobile/src/terminal.
return isTerminalLinkActivation(event)
}
function* iterateTerminalHttpUrlCandidates(
@@ -220,7 +222,7 @@ export function installHttpLinkClickFallback(
deps: UrlLinkClickFallbackDeps
): IDisposable {
const handleMouseUp = (event: MouseEvent): void => {
if (!isPrimaryHttpLinkFallbackActivation(event)) {
if (!isDesktopHttpLinkFallbackActivation(event)) {
return
}
@@ -230,8 +232,8 @@ export function installHttpLinkClickFallback(
}
// Why: xterm's WebLinksAddon only activates after hover state exists. This
// direct mouseup fallback preserves ordinary link clicks when the hover link
// was never established, while defaultPrevented avoids duplicate opens.
// direct mouseup fallback preserves modifier-clicks when the hover link was
// never established, while defaultPrevented avoids duplicate opens.
const opened = openHttpLinkAtBufferPosition(terminal.buffer.active, position, terminal.cols, {
worktreeId: deps.worktreeId,
forceSystemBrowser: event.shiftKey,
@@ -1017,7 +1017,7 @@ export function useTerminalPaneLifecycle({
pane.terminal.options.linkHandler = {
allowNonHttpProtocols: true,
activate: (event, text) => {
handleOscLink(text, event as MouseEvent | undefined, {
const handled = handleOscLink(text, event as MouseEvent | undefined, {
...linkDeps,
startupCwd: getPaneLinkCwd(pane.id),
runtimeEnvironmentId: linkDeps.getRuntimeEnvironmentIdForPane?.(pane.id) ?? null,
@@ -1031,7 +1031,9 @@ export function useTerminalPaneLifecycle({
// moving the mouse extends a selection until the next click/Esc.
// clearSelection() explicitly detaches those listeners (see
// SelectionService._removeMouseDownListeners).
pane.terminal.clearSelection()
if (handled) {
pane.terminal.clearSelection()
}
},
// Show bottom-left tooltip on hover for OSC 8 hyperlinks (e.g.
// GitHub owner/repo#issue references emitted by CLI tools) — same
@@ -1344,7 +1346,7 @@ export function useTerminalPaneLifecycle({
return
}
const activePane = managerRef.current?.getActivePane()
void handleOscLink(url, event, {
const handled = handleOscLink(url, event, {
...linkDeps,
startupCwd: activePane ? getPaneLinkCwd(activePane.id) : startupCwd,
runtimeEnvironmentId: activePane
@@ -1359,7 +1361,9 @@ export function useTerminalPaneLifecycle({
// phantom selection until the next click/Esc. Explicitly clearing the
// selection also detaches those listeners (see
// SelectionService._removeMouseDownListeners).
managerRef.current?.getActivePane()?.terminal.clearSelection()
if (handled) {
managerRef.current?.getActivePane()?.terminal.clearSelection()
}
},
formatLinkTooltip: (url, openLinkHint) => formatTerminalUrlTooltip(url, openLinkHint),
// Why: TerminalPane instances stay mounted for hidden visited worktrees
@@ -1,5 +1,5 @@
// @vitest-environment happy-dom
import { describe, expect, it, vi } from 'vitest'
import { afterEach, describe, expect, it, vi } from 'vitest'
import type { TerminalLeafId } from '../../../../shared/stable-pane-id'
import { createPaneDOM } from './pane-dom-creation'
@@ -48,7 +48,49 @@ vi.mock('@xterm/xterm', () => ({
})
}))
function setPlatform(userAgent: string): void {
vi.stubGlobal('navigator', { userAgent })
}
afterEach(() => {
vi.unstubAllGlobals()
})
describe('createPaneDOM link tooltips', () => {
it('uses desktop modifier-click text for WebLinks hover hints', () => {
const leafId = '11111111-1111-4111-8111-111111111111' as TerminalLeafId
setPlatform('Macintosh')
const macPane = createPaneDOM(
1,
leafId,
{},
{ active: null } as never,
{} as never,
vi.fn(),
vi.fn()
)
webLinksAddonMock.options?.hover?.({} as MouseEvent, 'http://localhost:5180/')
expect(macPane.linkTooltip.textContent).toBe(
'http://localhost:5180/ (⌘+click to open or ⇧⌘+click for system browser)'
)
setPlatform('Windows')
const windowsPane = createPaneDOM(
2,
leafId,
{},
{ active: null } as never,
{} as never,
vi.fn(),
vi.fn()
)
webLinksAddonMock.options?.hover?.({} as MouseEvent, 'http://localhost:5180/')
expect(windowsPane.linkTooltip.textContent).toBe(
'http://localhost:5180/ (Ctrl+click to open or Shift+Ctrl+click for system browser)'
)
})
it('lets callers replace WebLinks hover text for display-only labels', async () => {
const labeledText = 'http://main.orca.localhost:60016/ (localhost:5180; click to open)'
const leafId = '11111111-1111-4111-8111-111111111111' as TerminalLeafId
@@ -16,8 +16,8 @@ import { installGuardedLinkProviderRegistration } from './terminal-link-provider
function getTerminalUrlOpenHint(): string {
return navigator.userAgent.includes('Mac')
? 'click to open or ⇧+click for system browser'
: 'click to open or Shift+click for system browser'
? '⌘+click to open or ⇧+click for system browser'
: 'Ctrl+click to open or Shift+Ctrl+click for system browser'
}
function defaultLinkTooltipText(uri: string, openLinkHint: string): string {