diff --git a/apps/desktop/src/i18n/strings/en_EN.json b/apps/desktop/src/i18n/strings/en_EN.json index fdcdf662d1..c4e0bad993 100644 --- a/apps/desktop/src/i18n/strings/en_EN.json +++ b/apps/desktop/src/i18n/strings/en_EN.json @@ -27,6 +27,10 @@ "yes": "Yes" }, "confirm_quit": "Are you sure you want to quit?", + "download": { + "unable_to_open_description": "The file could not be opened. It may have been moved or deleted.", + "unable_to_open_title": "Unable to open file" + }, "edit_menu": { "speech": "Speech", "speech_start_speaking": "Start Speaking", diff --git a/apps/desktop/src/webcontents-handler.test.ts b/apps/desktop/src/webcontents-handler.test.ts new file mode 100644 index 0000000000..b2cc105afa --- /dev/null +++ b/apps/desktop/src/webcontents-handler.test.ts @@ -0,0 +1,125 @@ +/* +Copyright 2026 hayaksi1 + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE files in the repository root for full details. +*/ + +import { beforeEach, describe, expect, it, vi, type Mock } from "vitest"; +import { dialog, shell, type WebContents } from "electron"; + +// The `userDownloadAction` listener is registered at import time, so capture the callbacks that +// `ipcMain.on` receives in order to invoke the handler under test directly. +const { ipcHandlers } = vi.hoisted(() => ({ + ipcHandlers: {} as Record unknown>, +})); + +vi.mock("electron", () => ({ + clipboard: {}, + Menu: class {}, + MenuItem: class {}, + shell: { openExternal: vi.fn(), openPath: vi.fn() }, + dialog: { showMessageBox: vi.fn(), showSaveDialog: vi.fn() }, + ipcMain: { + on: vi.fn((channel: string, cb: (...args: unknown[]) => unknown) => { + ipcHandlers[channel] = cb; + }), + }, +})); +vi.mock("./language-helper.js", () => ({ _t: (key: string): string => key })); +vi.mock("./config.js", () => ({ getConfig: (): Record => ({}) })); +vi.mock("./save-image.js", () => ({ saveImageToFile: vi.fn() })); + +const registerWebContentsHandlers = (await import("./webcontents-handler.js")).default; + +interface MockWebContents { + setWindowOpenHandler: Mock; + on: Mock; + send: Mock; + session: { on: Mock }; + sessionHandlers: Record void>; +} + +function makeWebContents(): MockWebContents { + const sessionHandlers: Record void> = {}; + return { + setWindowOpenHandler: vi.fn(), + on: vi.fn(), + send: vi.fn(), + session: { + on: vi.fn((ev: string, cb: (...args: unknown[]) => void): void => { + sessionHandlers[ev] = cb; + }), + }, + sessionHandlers, + }; +} + +/** + * Drives the real will-download → done("completed") flow so the download is registered the way it is + * in production, and returns the id the handler assigned to it. + */ +function completeDownload(wc: MockWebContents, savePath: string): number { + const doneHandlers: Record void> = {}; + const item = { + once: (ev: string, cb: (...args: unknown[]) => void): void => { + doneHandlers[ev] = cb; + }, + getSavePath: (): string => savePath, + }; + wc.sessionHandlers["will-download"]({}, item); + doneHandlers["done"]({}, "completed"); + const completed = wc.send.mock.calls.find((c) => c[0] === "userDownloadCompleted"); + return (completed![1] as { id: number }).id; +} + +describe("userDownloadAction handler", () => { + let wc: MockWebContents; + + beforeEach(() => { + vi.clearAllMocks(); + wc = makeWebContents(); + registerWebContentsHandlers(wc as unknown as WebContents); + }); + + it("opens the file when the user clicks Open on a known download", async () => { + vi.mocked(shell.openPath).mockResolvedValue(""); + const id = completeDownload(wc, "/tmp/file.pdf"); + + await ipcHandlers["userDownloadAction"]({}, { id, open: true }); + + expect(shell.openPath).toHaveBeenCalledWith("/tmp/file.pdf"); + expect(dialog.showMessageBox).not.toHaveBeenCalled(); + }); + + it("shows the underlying error when the open fails, rather than failing silently", async () => { + vi.mocked(shell.openPath).mockResolvedValue("LSOpenURLsWithRole failed"); + const id = completeDownload(wc, "/tmp/file.pdf"); + + await ipcHandlers["userDownloadAction"]({}, { id, open: true }); + + expect(shell.openPath).toHaveBeenCalledWith("/tmp/file.pdf"); + expect(dialog.showMessageBox).toHaveBeenCalledWith( + expect.objectContaining({ type: "error", detail: "LSOpenURLsWithRole failed" }), + ); + }); + + it("does not open anything on a plain dismiss", async () => { + const id = completeDownload(wc, "/tmp/file.pdf"); + + await ipcHandlers["userDownloadAction"]({}, { id, open: false }); + + expect(shell.openPath).not.toHaveBeenCalled(); + }); + + it("removes the entry so a repeated open is a no-op", async () => { + vi.mocked(shell.openPath).mockResolvedValue(""); + const id = completeDownload(wc, "/tmp/file.pdf"); + + await ipcHandlers["userDownloadAction"]({}, { id, open: true }); + vi.mocked(shell.openPath).mockClear(); + await ipcHandlers["userDownloadAction"]({}, { id, open: true }); + + expect(shell.openPath).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/desktop/src/webcontents-handler.ts b/apps/desktop/src/webcontents-handler.ts index 3dc3d26ca0..4e473f9929 100644 --- a/apps/desktop/src/webcontents-handler.ts +++ b/apps/desktop/src/webcontents-handler.ts @@ -243,12 +243,22 @@ function onEditableContextMenu(ev: Event, params: ContextMenuParams, webContents let userDownloadIndex = 0; const userDownloadMap = new Map(); // Map from id to path -ipcMain.on("userDownloadAction", function (ev: IpcMainEvent, { id, open = false }) { +ipcMain.on("userDownloadAction", async function (ev: IpcMainEvent, { id, open = false }) { const path = userDownloadMap.get(id); - if (open && path) { - void shell.openPath(path); - } userDownloadMap.delete(id); + if (open && path) { + // openPath resolves to a non-empty error string on failure, an empty one on success. + const error = await shell.openPath(path); + if (error) { + console.error(`Failed to open downloaded file ${path}: ${error}`); + void dialog.showMessageBox({ + type: "error", + title: _t("download|unable_to_open_title"), + message: _t("download|unable_to_open_description"), + detail: error, + }); + } + } }); export default (webContents: WebContents): void => {