From b63935fa66cfa2049f5855e5a33585a8a9f9145a Mon Sep 17 00:00:00 2001 From: JD Date: Mon, 26 Oct 2026 01:56:16 -0400 Subject: [PATCH] fix(response-viewer): a middle click on a file path opens it instead of the dashboard root Path links and in-document links carry a placeholder href (#, #fragment) that the click delegate resolves itself, but a middle click fires auxclick, never click, so the browser followed the placeholder and opened the dashboard root in a new tab. A middle click on those links now does what a click does; real URLs keep the browser's own middle click. --- src/web/public/app.js | 18 +- test/response-viewer-middle-click.test.ts | 262 ++++++++++++++++++++++ 2 files changed, 277 insertions(+), 3 deletions(-) create mode 100644 test/response-viewer-middle-click.test.ts diff --git a/src/web/public/app.js b/src/web/public/app.js index 62d98ce20..f7cefd1ef 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -2857,7 +2857,9 @@ class CodemanApp { _bindResponseViewerInteractions(body) { if (!body || body.dataset.rvBound === '1') return; body.dataset.rvBound = '1'; - body.addEventListener('click', async (ev) => { + // The two link kinds whose href is a placeholder the browser cannot open + // (`#`, `#fragment`). Returns true when it took the event. + const openPlaceholderLink = (ev) => { // File path (_linkifyFilePaths): open it in the preview overlay, which // resolves workspace and out-of-workspace paths alike. const pathLink = ev.target.closest('a.rv-path'); @@ -2874,7 +2876,7 @@ class CodemanApp { const target = pathLink.dataset.linkified ? this._resolveLinkedFilePath?.(filePath, sessionId) : filePath; if (filePath && pathLink.dataset.folder) this.openFolderGrid(target || filePath, sessionId); else if (filePath) this.openFilePreview(target || filePath, sessionId); - return; + return true; } // An in-document link (`[Install](#installation)`). The browser must not follow it: with @@ -2891,8 +2893,18 @@ class CodemanApp { const calm = window.matchMedia?.('(prefers-reduced-motion: reduce)')?.matches; target.scrollIntoView({ block: 'start', behavior: calm ? 'auto' : 'smooth' }); } - return; + return true; } + return false; + }; + // A middle click fires auxclick, never click, so the browser followed the + // placeholder and opened the dashboard root in a new tab: do what a click + // does instead. A real URL keeps the browser's own middle click. + body.addEventListener('auxclick', (ev) => { + if (ev.button === 1) openPlaceholderLink(ev); + }); + body.addEventListener('click', async (ev) => { + if (openPlaceholderLink(ev)) return; // A `localhost` URL in the agent's answer: from another device that can // only load through the server, so hand it to a proxied web tab diff --git a/test/response-viewer-middle-click.test.ts b/test/response-viewer-middle-click.test.ts new file mode 100644 index 000000000..b2d1c18df --- /dev/null +++ b/test/response-viewer-middle-click.test.ts @@ -0,0 +1,262 @@ +/** + * @fileoverview A middle click on a placeholder link in rendered markdown + * (`CodemanApp._bindResponseViewerInteractions`). + * + * File-path links (`a.rv-path`) and in-document links carry `href="#"` / + * `href="#frag"`: placeholders the click delegate resolves itself. A middle + * click fires `auxclick`, never `click`, so the browser used to follow the + * placeholder and open the dashboard root (`` + `#`) in a new + * tab. A middle click on a placeholder now does exactly what a click does; a + * real URL keeps the browser's own middle-click behaviour. + * + * Both surfaces are covered: the Response Viewer's messages + * (`_buildResponseViewerMessage` + `_linkifyFilePaths`) and the File Viewer's + * rendered markdown, whose rebased links name their own session. + * + * Drives the shipping `constants.js`, `app.js` and `terminal-ui.js` (for + * `_resolveLinkedFilePath`) in a `vm` with a jsdom window injected (the + * technique from response-viewer-file-links.test.ts; a per-file jsdom + * environment would externalize node:fs under vite). + */ +import { readFileSync } from 'node:fs'; +import { performance } from 'node:perf_hooks'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { JSDOM } from 'jsdom'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const publicFile = (name: string) => readFileSync(resolve(import.meta.dirname, '../src/web/public', name), 'utf8'); + +const dom = new JSDOM('', { + url: 'http://localhost:3000/', +}); +const win = dom.window; +const { document } = win; +// app.js instantiates the whole app on DOMContentLoaded; let it pass before app.js is loaded. +if (document.readyState === 'loading') await new Promise((r) => document.addEventListener('DOMContentLoaded', r)); + +type App = { + activeSessionId: string; + sessions: Map; + openFilePreview: ReturnType; + openFolderGrid: ReturnType; + openLinkThroughWebTabIfLoopback: ReturnType; + _linkifyFilePaths(root: Element): void; + _bindResponseViewerInteractions(body: Element): void; +}; + +function loadCodemanAppClass(): { prototype: App } { + const context = vm.createContext({ + console, + performance, + setInterval: vi.fn(), + clearInterval: vi.fn(), + setTimeout, + clearTimeout, + requestAnimationFrame: vi.fn(), + HTMLCanvasElement: class HTMLCanvasElement {}, + fetch: vi.fn(), + document, + NodeFilter: win.NodeFilter, + localStorage: { length: 0, key: vi.fn(), getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() }, + // The real window, so constants.js publishes CodemanMarkdownAnchors where the delegate reads it. + window: win, + MobileDetection: {}, + }); + vm.runInContext( + `${publicFile('constants.js')}\n${publicFile('app.js')}\n${publicFile('terminal-ui.js')}\nglobalThis.__CodemanApp = CodemanApp;`, + context + ); + return (context as unknown as { __CodemanApp: { prototype: App } }).__CodemanApp; +} + +const CodemanApp = loadCodemanAppClass(); + +let app: App; + +beforeEach(() => { + app = Object.create(CodemanApp.prototype) as App; + app.activeSessionId = 'sess-1'; + app.sessions = new Map([ + ['sess-1', { workingDir: '/home/me/proj' }], + ['sess-2', { workingDir: '/srv/other' }], + ]); + app.openFilePreview = vi.fn(); + app.openFolderGrid = vi.fn(); + // A loopback URL would be handed to a web tab on a left click; a middle click must not reach it. + app.openLinkThroughWebTabIfLoopback = vi.fn(() => true); +}); + +/** A Response Viewer message body: rendered `.rv-text` inside the bound container. */ +function responseViewer(html: string): HTMLElement { + const body = document.createElement('div'); + const text = document.createElement('div'); + text.className = 'rv-text'; + text.innerHTML = html; + app._linkifyFilePaths(text); + body.appendChild(text); + document.body.replaceChildren(body); + app._bindResponseViewerInteractions(body); + return body; +} + +function press(el: Element, type: 'auxclick' | 'click', init: MouseEventInit = {}): MouseEvent { + const ev = new win.MouseEvent(type, { bubbles: true, cancelable: true, ...init }); + el.dispatchEvent(ev); + return ev; +} + +function noneCalled(): void { + expect(app.openFilePreview).not.toHaveBeenCalled(); + expect(app.openFolderGrid).not.toHaveBeenCalled(); + expect(app.openLinkThroughWebTabIfLoopback).not.toHaveBeenCalled(); +} + +describe('middle click on a Response Viewer path link', () => { + it('opens a relative file path in the preview, resolved against the active session', () => { + const body = responseViewer('

wrote src/app.ts

'); + const link = body.querySelector('a.rv-path')!; + expect(link.getAttribute('href')).toBe('#'); + + const ev = press(link, 'auxclick', { button: 1 }); + + expect(ev.defaultPrevented).toBe(true); + expect(app.openFilePreview.mock.calls).toEqual([['/home/me/proj/src/app.ts', 'sess-1']]); + expect(app.openFolderGrid).not.toHaveBeenCalled(); + expect(app.openLinkThroughWebTabIfLoopback).not.toHaveBeenCalled(); + }); + + it('opens a markdown link whose destination is a path, like a click does', () => { + const body = responseViewer('

record

'); + + const ev = press(body.querySelector('a.rv-path')!, 'auxclick', { button: 1 }); + + expect(ev.defaultPrevented).toBe(true); + expect(app.openFilePreview.mock.calls).toEqual([['/home/me/run/summary.json', 'sess-1']]); + }); + + it('opens a folder path in the folder grid', () => { + const body = responseViewer('

renders

'); + + const ev = press(body.querySelector('a.rv-path')!, 'auxclick', { button: 1 }); + + expect(ev.defaultPrevented).toBe(true); + expect(app.openFolderGrid.mock.calls).toEqual([['/tmp/renders/', 'sess-1']]); + expect(app.openFilePreview).not.toHaveBeenCalled(); + }); + + it('does the same with Ctrl held, as Ctrl+click does', () => { + const body = responseViewer('

wrote src/app.ts

'); + + const ev = press(body.querySelector('a.rv-path')!, 'auxclick', { button: 1, ctrlKey: true }); + + expect(ev.defaultPrevented).toBe(true); + expect(app.openFilePreview.mock.calls).toEqual([['/home/me/proj/src/app.ts', 'sess-1']]); + }); + + it.each([ + ['right', 2], + ['back', 3], + ['forward', 4], + ])('ignores the %s button', (_name, button) => { + const body = responseViewer('

wrote src/app.ts

'); + + const ev = press(body.querySelector('a.rv-path')!, 'auxclick', { button }); + + expect(ev.defaultPrevented).toBe(false); + noneCalled(); + }); + + it('opens once per gesture: a left click still goes through click alone', () => { + const body = responseViewer('

wrote src/app.ts

'); + // Binding is idempotent: a second bind must not add a second auxclick listener. + app._bindResponseViewerInteractions(body); + const link = body.querySelector('a.rv-path')!; + + press(link, 'auxclick', { button: 1 }); + expect(app.openFilePreview).toHaveBeenCalledTimes(1); + + app.openFilePreview.mockClear(); + const click = press(link, 'click', { button: 0 }); + expect(click.defaultPrevented).toBe(true); + expect(app.openFilePreview.mock.calls).toEqual([['/home/me/proj/src/app.ts', 'sess-1']]); + }); +}); + +describe('middle click on an in-document link', () => { + it('scrolls to the heading in place instead of following the fragment', () => { + const body = responseViewer('

Install

see install

'); + const heading = body.querySelector('h2')! as HTMLElement & { scrollIntoView: ReturnType }; + heading.scrollIntoView = vi.fn(); + + const ev = press(body.querySelector('a[href="#install"]')!, 'auxclick', { button: 1 }); + + expect(ev.defaultPrevented).toBe(true); + // jsdom has no matchMedia, so reduced motion reads as off. + expect(heading.scrollIntoView.mock.calls).toEqual([[{ block: 'start', behavior: 'smooth' }]]); + noneCalled(); + }); + + it('ignores a right click on it', () => { + const body = responseViewer('

Install

see install

'); + const heading = body.querySelector('h2')! as HTMLElement & { scrollIntoView: ReturnType }; + heading.scrollIntoView = vi.fn(); + + const ev = press(body.querySelector('a[href="#install"]')!, 'auxclick', { button: 2 }); + + expect(ev.defaultPrevented).toBe(false); + expect(heading.scrollIntoView).not.toHaveBeenCalled(); + }); +}); + +describe('middle click on a real URL', () => { + it.each(['https://example.com/a.json', 'http://localhost:5173/'])('leaves %s to the browser', (href) => { + const body = responseViewer(`

x

`); + expect(body.querySelector('a.rv-path')).toBeNull(); + + const ev = press(body.querySelector('a')!, 'auxclick', { button: 1 }); + + expect(ev.defaultPrevented).toBe(false); + noneCalled(); + }); +}); + +describe('middle click in the File Viewer rendered markdown', () => { + it('opens a rebased link in the session the preview was opened for, not the active one', () => { + // The shape _rebaseFilePreviewMarkdownRefs gives a workspace link (panels-ui.js). + const bodyEl = document.createElement('div'); + bodyEl.innerHTML = + '
notes
'; + document.body.replaceChildren(bodyEl); + app._bindResponseViewerInteractions(bodyEl); + + const ev = press(bodyEl.querySelector('a.rv-path')!, 'auxclick', { button: 1 }); + + expect(ev.defaultPrevented).toBe(true); + expect(app.openFilePreview.mock.calls).toEqual([['/srv/other/notes.md', 'sess-2']]); + }); +}); + +describe('a left click keeps its behaviour', () => { + it('opens a path link in the preview', () => { + const body = responseViewer('

wrote src/app.ts

'); + + const ev = press(body.querySelector('a.rv-path')!, 'click', { button: 0 }); + + expect(ev.defaultPrevented).toBe(true); + expect(app.openFilePreview.mock.calls).toEqual([['/home/me/proj/src/app.ts', 'sess-1']]); + expect(app.openLinkThroughWebTabIfLoopback).not.toHaveBeenCalled(); + }); + + it('hands a loopback URL to the web-tab hook', () => { + const body = responseViewer( + '

dev server

' + ); + + const ev = press(body.querySelector('a')!, 'click', { button: 0 }); + + expect(ev.defaultPrevented).toBe(true); + expect(app.openLinkThroughWebTabIfLoopback.mock.calls).toEqual([['http://localhost:5173/']]); + expect(app.openFilePreview).not.toHaveBeenCalled(); + }); +});