From 77112073936c1a277c0f65dbba9d908f2b7944fb Mon Sep 17 00:00:00 2001 From: hayyaksi <193020925+hayaksi1@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:40:18 +0300 Subject: [PATCH] Keep a file's extension when it is renamed in the desktop save dialog (#34601) Electron's save dialog offers only "All Files" unless it is handed filters, so someone who replaced "photo.jpg" with "photo" got a file with no extension at all and no way to open it. The browser does not do this: it offers the file's own type, and puts the extension back when the name arrives without one. The download now names its type before the dialog goes up, and the "Save image as..." context menu passes the same filters. A file with no extension to preserve is left alone rather than being given a meaningless filter, and "All Files" stays on the list so anyone who really wants an extensionless name can still pick it. Tests: a download with an extension gets filters, one without does not, and the context menu path is covered as well. Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --- apps/desktop/src/i18n/strings/en_EN.json | 4 + apps/desktop/src/webcontents-handler.test.ts | 125 +++++++++++++++++-- apps/desktop/src/webcontents-handler.ts | 28 +++++ 3 files changed, 144 insertions(+), 13 deletions(-) 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();