diff --git a/apps/desktop/src/i18n/strings/en_EN.json b/apps/desktop/src/i18n/strings/en_EN.json index c4e0bad993..c4574fd130 100644 --- a/apps/desktop/src/i18n/strings/en_EN.json +++ b/apps/desktop/src/i18n/strings/en_EN.json @@ -72,6 +72,10 @@ "save_image_as_error_description": "The image failed to save", "save_image_as_error_title": "Failed to save image" }, + "save_dialog": { + "all_files": "All Files", + "named_file_type": "%(extension)s File" + }, "store": { "error": { "backend_changed": "Clear data and reload?", diff --git a/apps/desktop/src/webcontents-handler.test.ts b/apps/desktop/src/webcontents-handler.test.ts index b2cc105afa..e629352f9f 100644 --- a/apps/desktop/src/webcontents-handler.test.ts +++ b/apps/desktop/src/webcontents-handler.test.ts @@ -10,14 +10,41 @@ 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(() => ({ +const { ipcHandlers, menus } = vi.hoisted(() => ({ ipcHandlers: {} as Record unknown>, + // Every context menu the handler builds, in construction order, so a test can click an entry. + menus: [] as MenuStub[], })); +interface MenuItemStub { + label: string; + click?: () => void | Promise; +} + +interface MenuStub { + items: MenuItemStub[]; +} + vi.mock("electron", () => ({ - clipboard: {}, - Menu: class {}, - MenuItem: class {}, + clipboard: { writeText: vi.fn() }, + Menu: class { + public readonly items: MenuItemStub[] = []; + public constructor() { + menus.push(this); + } + public append(item: MenuItemStub): void { + this.items.push(item); + } + public popup(): void {} + }, + MenuItem: class { + public readonly label: string; + public readonly click?: () => void | Promise; + public constructor(options: MenuItemStub) { + this.label = options.label; + this.click = options.click; + } + }, shell: { openExternal: vi.fn(), openPath: vi.fn() }, dialog: { showMessageBox: vi.fn(), showSaveDialog: vi.fn() }, ipcMain: { @@ -36,39 +63,61 @@ interface MockWebContents { setWindowOpenHandler: Mock; on: Mock; send: Mock; + copyImageAt: Mock; session: { on: Mock }; + handlers: Record void>; sessionHandlers: Record void>; } function makeWebContents(): MockWebContents { + const handlers: Record void> = {}; const sessionHandlers: Record void> = {}; return { setWindowOpenHandler: vi.fn(), - on: vi.fn(), + on: vi.fn((ev: string, cb: (...args: unknown[]) => void): void => { + handlers[ev] = cb; + }), send: vi.fn(), + copyImageAt: vi.fn(), session: { on: vi.fn((ev: string, cb: (...args: unknown[]) => void): void => { sessionHandlers[ev] = cb; }), }, + handlers, sessionHandlers, }; } +interface MockDownloadItem { + once: (ev: string, cb: (...args: unknown[]) => void) => void; + getFilename: () => string; + getSavePath: () => string; + setSaveDialogOptions: Mock; + doneHandlers: Record void>; +} + +function makeDownloadItem(savePath: string): MockDownloadItem { + const doneHandlers: Record void> = {}; + return { + once: (ev: string, cb: (...args: unknown[]) => void): void => { + doneHandlers[ev] = cb; + }, + getFilename: (): string => savePath.split("/").pop()!, + getSavePath: (): string => savePath, + setSaveDialogOptions: vi.fn(), + doneHandlers, + }; +} + /** * 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, - }; + const item = makeDownloadItem(savePath); wc.sessionHandlers["will-download"]({}, item); - doneHandlers["done"]({}, "completed"); + item.doneHandlers["done"]({}, "completed"); const completed = wc.send.mock.calls.find((c) => c[0] === "userDownloadCompleted"); return (completed![1] as { id: number }).id; } @@ -123,3 +172,53 @@ describe("userDownloadAction handler", () => { expect(shell.openPath).not.toHaveBeenCalled(); }); }); + +describe("save dialog filters", () => { + let wc: MockWebContents; + + beforeEach(() => { + vi.clearAllMocks(); + menus.length = 0; + wc = makeWebContents(); + registerWebContentsHandlers(wc as unknown as WebContents); + }); + + it("names a download's own file type, so renaming it does not strip the extension", () => { + const item = makeDownloadItem("/tmp/photo.jpg"); + + wc.sessionHandlers["will-download"]({}, item); + + expect(item.setSaveDialogOptions).toHaveBeenCalledWith({ + filters: [expect.objectContaining({ extensions: ["jpg"] }), expect.objectContaining({ extensions: ["*"] })], + }); + }); + + it("leaves the dialog alone for a download which has no extension to preserve", () => { + const item = makeDownloadItem("/tmp/archive"); + + wc.sessionHandlers["will-download"]({}, item); + + expect(item.setSaveDialogOptions).not.toHaveBeenCalled(); + }); + + it("names the file type when saving an image from the context menu too", async () => { + vi.mocked(dialog.showSaveDialog).mockResolvedValue({ canceled: false, filePath: "/tmp/renamed.jpg" }); + + wc.handlers["context-menu"]( + { preventDefault: vi.fn() }, + { srcURL: "https://example.org/photo.jpg", hasImageContents: true, suggestedFilename: "photo.jpg" }, + ); + const saveAs = menus[0].items.find((item) => item.label === "right_click_menu|save_image_as"); + await saveAs!.click!(); + + expect(dialog.showSaveDialog).toHaveBeenCalledWith( + expect.objectContaining({ + defaultPath: "photo.jpg", + filters: [ + expect.objectContaining({ extensions: ["jpg"] }), + expect.objectContaining({ extensions: ["*"] }), + ], + }), + ); + }); +}); diff --git a/apps/desktop/src/webcontents-handler.ts b/apps/desktop/src/webcontents-handler.ts index 4e473f9929..15dfdcfd84 100644 --- a/apps/desktop/src/webcontents-handler.ts +++ b/apps/desktop/src/webcontents-handler.ts @@ -15,6 +15,7 @@ import { type WebContents, type ContextMenuParams, type DownloadItem, + type FileFilter, type MenuItemConstructorOptions, type IpcMainEvent, type Event, @@ -30,6 +31,27 @@ const MAILTO_PREFIX = "mailto:"; const PERMITTED_URL_SCHEMES: string[] = ["http:", "https:", MAILTO_PREFIX]; +/** + * Work out the filters a save dialog should offer so that a file keeps its own extension. + * + * A dialog which only offers "All Files" lets someone replace "photo.jpg" with "photo" and end up + * with a file the shell no longer knows how to open — the extension is simply gone. Naming the + * file's own type first means the dialog puts the extension back, which is what a browser already + * does for the same download. + * + * @param fileName - The name being suggested to the user, which may carry no extension at all. + * @returns Filters to pass to a save dialog, or undefined when there is no extension to preserve. + */ +function saveDialogFilters(fileName: string): FileFilter[] | undefined { + // extname() keeps the leading dot, and returns an empty string for a name which has none. + const extension = path.extname(fileName).slice(1); + if (!extension) return undefined; + return [ + { name: _t("save_dialog|named_file_type", { extension: extension.toUpperCase() }), extensions: [extension] }, + { name: _t("save_dialog|all_files"), extensions: ["*"] }, + ]; +} + function safeOpenURL(target: string): void { // openExternal passes the target to open/start/xdg-open, // so put fairly stringent limits on what can be opened @@ -129,6 +151,7 @@ function onLinkContextMenu(ev: Event, params: ContextMenuParams, webContents: We const targetFileName = params.suggestedFilename || params.altText || "image.png"; const { filePath } = await dialog.showSaveDialog({ defaultPath: targetFileName, + filters: saveDialogFilters(targetFileName), }); if (!filePath) return; // user cancelled dialog @@ -283,6 +306,11 @@ export default (webContents: WebContents): void => { }); webContents.session.on("will-download", (event: Event, item: DownloadItem): void => { + // Electron only offers "All Files" unless it is told otherwise, so say what this download is + // before it puts the save dialog up. + const filters = saveDialogFilters(item.getFilename()); + if (filters) item.setSaveDialogOptions({ filters }); + item.once("done", (event, state) => { if (state === "completed") { const savePath = item.getSavePath();