fix(viewer): hold file-path clicks to the card's modifier promise
showFileCard now records modifierPromised like the OSC 8 hover, and the file-path provider's gate goes through the handler's new opensFileLink, so a "Shift+click to open" card cannot be answered by a bare click after the container drops mouse tracking. A click before the session's project is known now toasts instead of doing nothing. Refusal-card tests assert the card is present; a misplaced test comment is back on its test. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1625,11 +1625,14 @@ describe("createOsc8LinkHandler — clicking a link Claude Code printed", () =>
|
|||||||
h.hover?.(new MouseEvent("mousemove"), "javascript:alert(1)", range);
|
h.hover?.(new MouseEvent("mousemove"), "javascript:alert(1)", range);
|
||||||
expect(onOpenFile).not.toHaveBeenCalled();
|
expect(onOpenFile).not.toHaveBeenCalled();
|
||||||
expect(openUrlExternal).not.toHaveBeenCalled();
|
expect(openUrlExternal).not.toHaveBeenCalled();
|
||||||
// The card, if any, is the refusal — never the target, never an offer to
|
// The refusal card (ruling (a)): present, no origin, never the target,
|
||||||
// open it. (The brief asked for no card here; the existing test "says so
|
// never an offer to open it.
|
||||||
// on hover when the target would be refused" requires the refusal card.)
|
const card = hoverCard();
|
||||||
expect(hoverCard()?.textContent ?? "").not.toContain("javascript:");
|
expect(card).not.toBeNull();
|
||||||
expect(hoverCard()?.textContent ?? "").not.toContain("Open in viewer");
|
expect(card!.querySelector('[data-testid="osc8-hover-origin"]')).toBeNull();
|
||||||
|
expect(card!.textContent).toContain("will not be opened");
|
||||||
|
expect(card!.textContent).not.toContain("javascript:");
|
||||||
|
expect(card!.textContent).not.toContain("Open in viewer");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("refuses a file: target whose escapes do not decode", () => {
|
it("refuses a file: target whose escapes do not decode", () => {
|
||||||
@@ -1638,7 +1641,11 @@ describe("createOsc8LinkHandler — clicking a link Claude Code printed", () =>
|
|||||||
h.activate(click(), "file:///workspace/%E0%A4%A", range);
|
h.activate(click(), "file:///workspace/%E0%A4%A", range);
|
||||||
h.hover?.(new MouseEvent("mousemove"), "file:///workspace/%E0%A4%A", range);
|
h.hover?.(new MouseEvent("mousemove"), "file:///workspace/%E0%A4%A", range);
|
||||||
expect(onOpenFile).not.toHaveBeenCalled();
|
expect(onOpenFile).not.toHaveBeenCalled();
|
||||||
expect(hoverCard()?.textContent ?? "").not.toContain("Open in viewer");
|
const card = hoverCard();
|
||||||
|
expect(card).not.toBeNull();
|
||||||
|
expect(card!.querySelector('[data-testid="osc8-hover-origin"]')).toBeNull();
|
||||||
|
expect(card!.textContent).toContain("will not be opened");
|
||||||
|
expect(card!.textContent).not.toContain("Open in viewer");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("the file: hover card names the viewer and the path", () => {
|
it("the file: hover card names the viewer and the path", () => {
|
||||||
@@ -1657,6 +1664,19 @@ describe("createOsc8LinkHandler — clicking a link Claude Code printed", () =>
|
|||||||
expect(hoverCard()).toBeNull();
|
expect(hoverCard()).toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("holds a file-path click to the modifier its card promised", () => {
|
||||||
|
const h = createOsc8LinkHandler(() => host, () => state, vi.fn());
|
||||||
|
state.mouseTracking = true;
|
||||||
|
h.showFileCard("src/foo.ts");
|
||||||
|
expect(hoverCard()?.textContent).toContain("Shift+click");
|
||||||
|
state.mouseTracking = false;
|
||||||
|
expect(h.opensFileLink(click())).toBe(false);
|
||||||
|
expect(h.opensFileLink(click({ shiftKey: true }))).toBe(true);
|
||||||
|
// A card drawn with nothing tracking promises nothing.
|
||||||
|
h.showFileCard("src/foo.ts");
|
||||||
|
expect(h.opensFileLink(click())).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
it("declares allowNonHttpProtocols so file: targets reach it", () => {
|
it("declares allowNonHttpProtocols so file: targets reach it", () => {
|
||||||
expect(createOsc8LinkHandler(() => host, () => state).allowNonHttpProtocols).toBe(true);
|
expect(createOsc8LinkHandler(() => host, () => state).allowNonHttpProtocols).toBe(true);
|
||||||
});
|
});
|
||||||
@@ -1711,9 +1731,6 @@ describe("the link handler is wired into the terminal, and reads its live mode",
|
|||||||
expect(typeof handler.hover).toBe("function");
|
expect(typeof handler.hover).toBe("function");
|
||||||
});
|
});
|
||||||
|
|
||||||
// The gate has to ask the terminal, not a boolean captured at construction:
|
|
||||||
// the mode changes whenever the container prints a DECSET, which is several
|
|
||||||
// times a second in Claude Code.
|
|
||||||
it("opens a file: target in the viewer against the session's project", async () => {
|
it("opens a file: target in the viewer against the session's project", async () => {
|
||||||
vi.mocked(openFileViewer).mockClear();
|
vi.mocked(openFileViewer).mockClear();
|
||||||
mountSession("bash");
|
mountSession("bash");
|
||||||
@@ -1727,7 +1744,9 @@ describe("the link handler is wired into the terminal, and reads its live mode",
|
|||||||
await Promise.resolve();
|
await Promise.resolve();
|
||||||
});
|
});
|
||||||
|
|
||||||
expect(openFileViewer).toHaveBeenCalledWith("p1", "/workspace/api/src/a b.ts");
|
expect(openFileViewer).toHaveBeenCalledWith(
|
||||||
|
"p1", "/workspace/api/src/a b.ts", undefined, undefined, undefined,
|
||||||
|
);
|
||||||
expect(openUrlExternal).not.toHaveBeenCalled();
|
expect(openUrlExternal).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -1774,6 +1793,65 @@ describe("the link handler is wired into the terminal, and reads its live mode",
|
|||||||
expect(toasts.at(-1)?.dedupeKey).toBe("file-viewer-open");
|
expect(toasts.at(-1)?.dedupeKey).toBe("file-viewer-open");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("holds a plain-text file link to the modifier its hover card promised", async () => {
|
||||||
|
vi.mocked(openFileViewer).mockClear();
|
||||||
|
const platform = navigator.platform;
|
||||||
|
Object.defineProperty(navigator, "platform", { value: "Linux x86_64", configurable: true });
|
||||||
|
const register = vi.spyOn(Terminal.prototype, "registerLinkProvider");
|
||||||
|
try {
|
||||||
|
mountSession("claude");
|
||||||
|
const provider = register.mock.calls.at(-1)?.[0];
|
||||||
|
if (!provider) throw new Error("no link provider was registered");
|
||||||
|
await write("Edited src/foo.ts:42 today");
|
||||||
|
const links = vi.fn();
|
||||||
|
provider.provideLinks(1, links);
|
||||||
|
const [link] = links.mock.calls[0][0];
|
||||||
|
|
||||||
|
await write("\x1b[?1002h");
|
||||||
|
link.hover?.(new MouseEvent("mousemove"), link.text);
|
||||||
|
expect(document.body.textContent).toContain("Shift+click to open");
|
||||||
|
await write("\x1b[?1002l");
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
link.activate(new MouseEvent("click", { button: 0, detail: 1 }), link.text);
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
expect(openFileViewer).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
link.activate(
|
||||||
|
new MouseEvent("click", { button: 0, detail: 1, shiftKey: true }),
|
||||||
|
link.text,
|
||||||
|
);
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
expect(openFileViewer).toHaveBeenCalledWith("p1", "src/foo.ts", 42, undefined, undefined);
|
||||||
|
} finally {
|
||||||
|
register.mockRestore();
|
||||||
|
Object.defineProperty(navigator, "platform", { value: platform, configurable: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("says the project is not ready rather than doing nothing", async () => {
|
||||||
|
vi.mocked(openFileViewer).mockClear();
|
||||||
|
useAppState.setState({ sessions: [] });
|
||||||
|
render(<TerminalView sessionId="s-unknown" active />);
|
||||||
|
|
||||||
|
wiredHandler().activate(
|
||||||
|
new MouseEvent("click", { button: 0, detail: 1 }),
|
||||||
|
"file:///workspace/api/README.md",
|
||||||
|
range,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(openFileViewer).not.toHaveBeenCalled();
|
||||||
|
const toasts = useAppState.getState().toasts;
|
||||||
|
expect(toasts.at(-1)?.detail).toContain("not ready yet");
|
||||||
|
expect(toasts.at(-1)?.dedupeKey).toBe("file-viewer-open");
|
||||||
|
});
|
||||||
|
|
||||||
|
// The gate has to ask the terminal, not a boolean captured at construction:
|
||||||
|
// the mode changes whenever the container prints a DECSET, which is several
|
||||||
|
// times a second in Claude Code.
|
||||||
it("refuses a plain click once the container turns mouse tracking on", async () => {
|
it("refuses a plain click once the container turns mouse tracking on", async () => {
|
||||||
mountSession("claude");
|
mountSession("claude");
|
||||||
await write("\x1b[?1002h");
|
await write("\x1b[?1002h");
|
||||||
|
|||||||
@@ -140,6 +140,30 @@ function reportOpenFailure(e: unknown) {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Open a path in the file viewer for this terminal's project, or say why not.
|
||||||
|
* The session record (and so the project) can arrive after the first render;
|
||||||
|
* a click in that window gets a toast rather than silently doing nothing.
|
||||||
|
*/
|
||||||
|
function openInViewer(
|
||||||
|
projectId: string | undefined,
|
||||||
|
path: string,
|
||||||
|
line?: number,
|
||||||
|
col?: number,
|
||||||
|
endLine?: number,
|
||||||
|
): void {
|
||||||
|
if (!projectId) {
|
||||||
|
useAppState.getState().pushToast({
|
||||||
|
kind: "error",
|
||||||
|
message: "Could not open the file",
|
||||||
|
detail: "This terminal's project is not ready yet",
|
||||||
|
dedupeKey: "file-viewer-open",
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
openFileViewer(projectId, path, line, col, endLine).catch(reportViewerFailure);
|
||||||
|
}
|
||||||
|
|
||||||
/** Report a file-viewer open that the backend refused (not found, no container, cap). */
|
/** Report a file-viewer open that the backend refused (not found, no container, cap). */
|
||||||
function reportViewerFailure(e: unknown): void {
|
function reportViewerFailure(e: unknown): void {
|
||||||
useAppState.getState().pushToast({
|
useAppState.getState().pushToast({
|
||||||
@@ -164,10 +188,17 @@ export type Osc8LinkHandler = ILinkHandler & {
|
|||||||
/**
|
/**
|
||||||
* Draw the "Open in viewer" card for a path matched in plain text by the
|
* Draw the "Open in viewer" card for a path matched in plain text by the
|
||||||
* file-path link provider. Takes the raw path (a relative path stays
|
* file-path link provider. Takes the raw path (a relative path stays
|
||||||
* relative — `file://src/x` would parse `src` as a host), and leaves
|
* relative — `file://src/x` would parse `src` as a host). Records the
|
||||||
* `modifierPromised` alone: the provider applies its own gate.
|
* card's modifier promise exactly as `hover` does, so
|
||||||
|
* {@link opensFileLink} can hold the click to it.
|
||||||
*/
|
*/
|
||||||
showFileCard(path: string): void;
|
showFileCard(path: string): void;
|
||||||
|
/**
|
||||||
|
* The file-path provider's click gate: {@link opensOnClick} with whatever
|
||||||
|
* the card on screen promised. There is only ever one card, and `clear()`
|
||||||
|
* resets the promise with it, so sharing the flag with OSC 8 links is exact.
|
||||||
|
*/
|
||||||
|
opensFileLink(event: MouseEvent): boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -652,11 +683,19 @@ export function createOsc8LinkHandler(
|
|||||||
clear();
|
clear();
|
||||||
const host = getHost();
|
const host = getHost();
|
||||||
if (!host) return;
|
if (!host) return;
|
||||||
|
const ctx = readState();
|
||||||
|
// Same promise as `hover`: the card names a modifier, so the click is
|
||||||
|
// held to it even if the container drops tracking before it lands.
|
||||||
|
modifierPromised = ctx.mouseTracking;
|
||||||
card = makeCard();
|
card = makeCard();
|
||||||
fillFileCard(card, path, readState());
|
fillFileCard(card, path, ctx);
|
||||||
host.appendChild(card);
|
host.appendChild(card);
|
||||||
},
|
},
|
||||||
|
|
||||||
|
opensFileLink(event) {
|
||||||
|
return opensOnClick(event, readState(), modifierPromised);
|
||||||
|
},
|
||||||
|
|
||||||
allowNonHttpProtocols: true,
|
allowNonHttpProtocols: true,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -998,11 +1037,7 @@ export default function TerminalView({ sessionId, active }: Props) {
|
|||||||
linkHandler: (osc8LinkHandlerRef.current = createOsc8LinkHandler(
|
linkHandler: (osc8LinkHandlerRef.current = createOsc8LinkHandler(
|
||||||
() => term.element ?? null,
|
() => term.element ?? null,
|
||||||
() => readClickContext(term),
|
() => readClickContext(term),
|
||||||
(path) => {
|
(path) => openInViewer(projectIdRef.current, path),
|
||||||
if (projectIdRef.current) {
|
|
||||||
openFileViewer(projectIdRef.current, path).catch(reportViewerFailure);
|
|
||||||
}
|
|
||||||
},
|
|
||||||
)),
|
)),
|
||||||
fontFamily: "'JetBrains Mono', 'Fira Code', 'Cascadia Code', Menlo, Monaco, monospace",
|
fontFamily: "'JetBrains Mono', 'Fira Code', 'Cascadia Code', Menlo, Monaco, monospace",
|
||||||
theme: {
|
theme: {
|
||||||
@@ -1073,17 +1108,15 @@ export default function TerminalView({ sessionId, active }: Props) {
|
|||||||
|
|
||||||
// File paths in plain text open in the file viewer. Registered after the
|
// File paths in plain text open in the file viewer. Registered after the
|
||||||
// addon so URLs are claimed first; the matcher also refuses anything
|
// addon so URLs are claimed first; the matcher also refuses anything
|
||||||
// inside a `scheme://` span. Same gate as the URL branch above. The hover
|
// inside a `scheme://` span. The hover card is the OSC 8 handler's, fed the
|
||||||
// card is the OSC 8 handler's, fed the raw path (see `showFileCard`).
|
// raw path, and the click gate honours the modifier that card promised.
|
||||||
const filePathLinks = term.registerLinkProvider(
|
const filePathLinks = term.registerLinkProvider(
|
||||||
createFilePathLinkProvider(
|
createFilePathLinkProvider(
|
||||||
term,
|
term,
|
||||||
(m: FilePathMatch) => {
|
(m: FilePathMatch) =>
|
||||||
const pid = projectIdRef.current;
|
openInViewer(projectIdRef.current, m.path, m.line, m.col, m.endLine),
|
||||||
if (!pid) return;
|
// Held to what the card promised, like OSC 8 links (see `showFileCard`).
|
||||||
openFileViewer(pid, m.path, m.line, m.col, m.endLine).catch(reportViewerFailure);
|
(event) => osc8LinkHandlerRef.current?.opensFileLink(event) ?? false,
|
||||||
},
|
|
||||||
(event) => opensOnClick(event, readClickContext(term)),
|
|
||||||
{
|
{
|
||||||
show: (path) => osc8LinkHandlerRef.current?.showFileCard(path),
|
show: (path) => osc8LinkHandlerRef.current?.showFileCard(path),
|
||||||
hide: () => osc8LinkHandlerRef.current?.dismiss(),
|
hide: () => osc8LinkHandlerRef.current?.dismiss(),
|
||||||
|
|||||||
Reference in New Issue
Block a user