From 9790aca7650c3ebc31c5c2acc42ea98c1fc79b1a Mon Sep 17 00:00:00 2001 From: Sai Muppiri Date: Mon, 27 Apr 2026 16:14:35 -0400 Subject: [PATCH 01/19] downloads: Add onDeterminingFilename event schema Add the onDeterminingFilename event definition to the downloads API schema. Listeners receive a DownloadItem and may return an optional { filename, conflictAction } object to influence the final filename before the download begins. --- .../extensions/schemas/downloads.json | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/toolkit/components/extensions/schemas/downloads.json b/toolkit/components/extensions/schemas/downloads.json index ed3c1002e0d52..27a8126417223 100644 --- a/toolkit/components/extensions/schemas/downloads.json +++ b/toolkit/components/extensions/schemas/downloads.json @@ -725,6 +725,32 @@ ], "type": "function" }, + { + "name": "onDeterminingFilename", + "description": "Fires with the DownloadItem before the download's filename is finalized. The listener may return an object with an optional filename and/or conflictAction to influence the final filename. The first listener to return a non-null value wins.", + "parameters": [ + { + "$ref": "DownloadItem", + "name": "downloadItem" + } + ], + "returns": { + "type": "object", + "optional": true, + "properties": { + "filename": { + "type": "string", + "description": "The filename to use for the download. Must not be an absolute path, and must not contain path components that would traverse back to a parent directory. If a path separator is included, the file will be saved in the corresponding subdirectory beneath the user's default Downloads directory.", + "optional": true + }, + "conflictAction": { + "$ref": "FilenameConflictAction", + "optional": true + } + } + }, + "type": "function" + }, { "name": "onChanged", "description": "When any of a DownloadItem's properties except bytesReceived changes, this event fires with the downloadId and an object containing the properties that changed.", From fc7a0fadab7e3fe6debfcec76508e19fe215c71d Mon Sep 17 00:00:00 2001 From: Sai Muppiri Date: Mon, 27 Apr 2026 16:14:43 -0400 Subject: [PATCH 02/19] downloads: Add determineFilename hook to DownloadIntegration Add _determineFilenameCallbacks set and determineFilename() method to DownloadIntegration. This provides a registration point for the WebExtensions downloads API to intercept filename determination for all downloads before execution begins. --- .../downloads/DownloadIntegration.sys.mjs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/toolkit/components/downloads/DownloadIntegration.sys.mjs b/toolkit/components/downloads/DownloadIntegration.sys.mjs index 038472a1c33c5..eab09f573666f 100644 --- a/toolkit/components/downloads/DownloadIntegration.sys.mjs +++ b/toolkit/components/downloads/DownloadIntegration.sys.mjs @@ -410,6 +410,30 @@ export var DownloadIntegration = { return directoryPath; }, + /** + * A set of callbacks registered by the WebExtensions downloads API to handle + * the onDeterminingFilename event. Each callback receives a Download object + * and may return a { filename, conflictAction } suggestion. + */ + _determineFilenameCallbacks: new Set(), + + /** + * Called by Download.start() before execution begins, to give registered + * callbacks (i.e. WebExtension onDeterminingFilename listeners) a chance to + * override the target filename. + * + * @param {Download} download + */ + async determineFilename(download) { + for (let callback of this._determineFilenameCallbacks) { + try { + await callback(download); + } catch (ex) { + console.error(ex); + } + } + }, + /** * Checks to determine whether to block downloads for parental controls. * From f9b371e3de7323fc845c6bdc83551f04a6a78bf8 Mon Sep 17 00:00:00 2001 From: Sai Muppiri Date: Mon, 27 Apr 2026 16:15:38 -0400 Subject: [PATCH 03/19] downloads: Call DownloadIntegration.determineFilename() in Download.start() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make Download.start() async and call DownloadIntegration.determineFilename() before execution begins. This is the single interception point that covers all downloads — both extension-initiated and browser-initiated (legacy) — since start() is always called before the target path is locked in by the saver. --- toolkit/components/downloads/DownloadCore.sys.mjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/toolkit/components/downloads/DownloadCore.sys.mjs b/toolkit/components/downloads/DownloadCore.sys.mjs index 523b60a7a8984..f8f0175a114c9 100644 --- a/toolkit/components/downloads/DownloadCore.sys.mjs +++ b/toolkit/components/downloads/DownloadCore.sys.mjs @@ -375,7 +375,7 @@ Download.prototype = { * Resolves when the download has finished successfully. * @rejects JavaScript exception if the download failed. */ - start: function D_start() { + start: async function D_start() { // If the download succeeded, it's the final state, we have nothing to do. if (this.succeeded) { return Promise.resolve(); @@ -474,6 +474,8 @@ Download.prototype = { } } + await lazy.DownloadIntegration.determineFilename(this); + // Now that we stored the promise in the download object, we can start the // task that will actually execute the download. deferAttempt.resolve( From 35b9488715e48ee330852e07ccbc3021b2f03d5b Mon Sep 17 00:00:00 2001 From: Sai Muppiri Date: Mon, 27 Apr 2026 16:16:28 -0400 Subject: [PATCH 04/19] extensions: Implement downloads.onDeterminingFilename Replace the ignoreEvent() stub with a real EventManager implementation. Listeners register a callback with DownloadIntegration._determineFilenameCallbacks that fires fire.async(item.serialize()) for each download before it starts. The first listener to return a { filename, conflictAction } suggestion wins; the suggestion is validated (no absolute paths, no path traversal) and applied to download.target.path before execution begins. Covers both extension-initiated downloads (via downloads.download()) and all other browser-initiated downloads via the DownloadCore.start() hook. --- .../extensions/parent/ext-downloads.js | 100 +++++++++++++++++- 1 file changed, 96 insertions(+), 4 deletions(-) diff --git a/toolkit/components/extensions/parent/ext-downloads.js b/toolkit/components/extensions/parent/ext-downloads.js index d3d1234e91f6c..bd25ae634c8cf 100644 --- a/toolkit/components/extensions/parent/ext-downloads.js +++ b/toolkit/components/extensions/parent/ext-downloads.js @@ -5,13 +5,15 @@ "use strict"; ChromeUtils.defineESModuleGetters(this, { + DownloadIntegration: + "resource://gre/modules/DownloadIntegration.sys.mjs", DownloadLastDir: "resource://gre/modules/DownloadLastDir.sys.mjs", DownloadPaths: "resource://gre/modules/DownloadPaths.sys.mjs", Downloads: "resource://gre/modules/Downloads.sys.mjs", FileUtils: "resource://gre/modules/FileUtils.sys.mjs", }); -var { EventEmitter, ignoreEvent } = ExtensionCommon; +var { EventEmitter } = ExtensionCommon; var { ExtensionError } = ExtensionUtils; const DOWNLOAD_ITEM_FIELDS = [ @@ -613,6 +615,61 @@ const queryHelper = async query => { return results; }; +async function applyFilenameSuggestion(suggestion, currentPath) { + let { filename: suggestedFilename, conflictAction } = suggestion; + if (!suggestedFilename) { + return null; + } + + if (PathUtils.isAbsolute(suggestedFilename)) { + Cu.reportError( + "downloads.onDeterminingFilename: suggested filename must not be an absolute path" + ); + return null; + } + + if (AppConstants.platform === "win") { + suggestedFilename = suggestedFilename.replace(/\//g, "\\"); + } + + let pathComponents; + try { + pathComponents = PathUtils.splitRelative(suggestedFilename, { + allowEmpty: true, + allowCurrentDir: true, + allowParentDir: true, + }); + } catch (e) { + Cu.reportError( + `downloads.onDeterminingFilename: invalid suggested filename: ${e}` + ); + return null; + } + + if (pathComponents.some(component => component == "..")) { + Cu.reportError( + "downloads.onDeterminingFilename: suggested filename must not contain back-references (..)" + ); + return null; + } + + const dir = PathUtils.parent(currentPath); + const newPath = PathUtils.joinRelative(dir, suggestedFilename); + await IOUtils.makeDirectory(PathUtils.parent(newPath)); + + const resolvedConflictAction = conflictAction || "uniquify"; + + if (await IOUtils.exists(newPath)) { + if (resolvedConflictAction === "uniquify") { + return DownloadPaths.createNiceUniqueFile(new FileUtils.File(newPath)) + .path; + } + // "overwrite" — use the path as-is + } + + return newPath; +} + this.downloads = class extends ExtensionAPIPersistent { downloadEventRegistrar(event, listener) { let { extension } = this; @@ -1275,10 +1332,45 @@ this.downloads = class extends ExtensionAPIPersistent { extensionApi: this, }).api(), - onDeterminingFilename: ignoreEvent( + onDeterminingFilename: new EventManager({ context, - "downloads.onDeterminingFilename" - ), + name: "downloads.onDeterminingFilename", + register: fire => { + DownloadMap.lazyInit(); + const callback = async download => { + if (!extension.privateBrowsingAllowed && download.source.isPrivate) { + return; + } + // Ensure the download has an entry in DownloadMap even if it has + // not been added to the DownloadList yet (e.g. legacy downloads + // call start() before list.add()). + const item = + DownloadMap.byDownload.get(download) || + DownloadMap.newFromDownload(download, null); + let suggestion; + try { + suggestion = await fire.async(item.serialize()); + } catch (e) { + Cu.reportError(e); + return; + } + if (suggestion) { + const newPath = await applyFilenameSuggestion( + suggestion, + download.target.path + ); + if (newPath) { + download.target.path = newPath; + download.target.partFilePath = `${newPath}.part`; + } + } + }; + DownloadIntegration._determineFilenameCallbacks.add(callback); + return () => { + DownloadIntegration._determineFilenameCallbacks.delete(callback); + }; + }, + }).api(), }, }; } From 8321fee59dc65e432eb3af066993f33ab31cb76c Mon Sep 17 00:00:00 2001 From: Sai Muppiri Date: Mon, 27 Apr 2026 16:16:50 -0400 Subject: [PATCH 05/19] extensions: Remove stale onDeterminingFilename comment from eventpage test onDeterminingFilename is now a real event, not an empty event handler, so remove the comment that said otherwise. --- .../extensions/test/xpcshell/test_ext_downloads_eventpage.js | 1 - 1 file changed, 1 deletion(-) diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_downloads_eventpage.js b/toolkit/components/extensions/test/xpcshell/test_ext_downloads_eventpage.js index 9c71c63e962b3..aadf5353d4f11 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_downloads_eventpage.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_downloads_eventpage.js @@ -98,7 +98,6 @@ add_task( }, }); - // onDeterminingFilename is never persisted, it is an empty event handler. const EVENTS = ["onChanged", "onCreated", "onErased"]; await extension.startup(); From 0432fbb6a2a52a882b28419461bcc02980537f26 Mon Sep 17 00:00:00 2001 From: Sai Muppiri Date: Mon, 27 Apr 2026 16:17:01 -0400 Subject: [PATCH 06/19] extensions: Add tests for downloads.onDeterminingFilename Test cases cover: - Event fires with correct DownloadItem properties - Listener can rename the downloaded file - Listener can place the file in a subdirectory - conflictAction "uniquify" creates a unique filename when target exists - conflictAction "overwrite" replaces an existing file - Absolute path suggestions are silently rejected - Path traversal (..) suggestions are silently rejected - First listener's non-null suggestion wins when multiple are registered - removeListener stops the callback from firing - Download proceeds normally when no listener is registered --- ...test_ext_downloads_determining_filename.js | 462 ++++++++++++++++++ 1 file changed, 462 insertions(+) create mode 100644 toolkit/components/extensions/test/xpcshell/test_ext_downloads_determining_filename.js diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_downloads_determining_filename.js b/toolkit/components/extensions/test/xpcshell/test_ext_downloads_determining_filename.js new file mode 100644 index 0000000000000..814e102d90e7c --- /dev/null +++ b/toolkit/components/extensions/test/xpcshell/test_ext_downloads_determining_filename.js @@ -0,0 +1,462 @@ +"use strict"; + +const { Downloads } = ChromeUtils.importESModule( + "resource://gre/modules/Downloads.sys.mjs" +); + +const gServer = createHttpServer(); +gServer.registerDirectory("/data/", do_get_file("data")); + +const BASE = `http://localhost:${gServer.identity.primaryPort}/data`; +const TXT_FILE = "file_download.txt"; +const TXT_URL = `${BASE}/${TXT_FILE}`; +const TXT_LEN = 46; + +let downloadDir; + +add_task(function setup() { + downloadDir = FileUtils.getDir("TmpD", ["downloads"]); + downloadDir.createUnique( + Ci.nsIFile.DIRECTORY_TYPE, + FileUtils.PERMS_DIRECTORY + ); + info(`Using download directory ${downloadDir.path}`); + + Services.prefs.setIntPref("browser.download.folderList", 2); + Services.prefs.setComplexValue( + "browser.download.dir", + Ci.nsIFile, + downloadDir + ); + + registerCleanupFunction(() => { + Services.prefs.clearUserPref("browser.download.folderList"); + Services.prefs.clearUserPref("browser.download.dir"); + + let entries = downloadDir.directoryEntries; + while (entries.hasMoreElements()) { + let entry = entries.nextFile; + ok(false, `Leftover file ${entry.path} in download directory`); + entry.remove(false); + } + + downloadDir.remove(false); + }); +}); + +async function waitForDownloads() { + let list = await Downloads.getList(Downloads.ALL); + let downloads = await list.getAll(); + await Promise.all( + downloads.filter(dl => !dl.stopped).map(dl => dl.whenSucceeded()) + ); + for (let dl of await list.getAll()) { + list.remove(dl); + } +} + +function fileInDownloadDir(...parts) { + let file = downloadDir.clone(); + for (let part of parts) { + file.append(part); + } + return file; +} + +// Returns a simple extension that calls downloads.download() on request. +function makeCallerExtension() { + return ExtensionTestUtils.loadExtension({ + manifest: { permissions: ["downloads"] }, + background() { + browser.test.onMessage.addListener(async url => { + const id = await browser.downloads.download({ url }); + browser.test.sendMessage("id", id); + }); + browser.test.sendMessage("ready"); + }, + }); +} + +// Returns an extension with an onDeterminingFilename listener that forwards +// the DownloadItem to the test harness and waits for the test to send back a +// suggestion (or null/undefined for no change). +function makeListenerExtension() { + return ExtensionTestUtils.loadExtension({ + manifest: { permissions: ["downloads"] }, + background() { + browser.downloads.onDeterminingFilename.addListener(async item => { + browser.test.sendMessage("determining", item); + return new Promise(resolve => + browser.test.onMessage.addListener(function once(suggestion) { + browser.test.onMessage.removeListener(once); + resolve(suggestion); + }) + ); + }); + browser.test.sendMessage("ready"); + }, + }); +} + +// --------------------------------------------------------------------------- +// Test: listener fires and receives the correct DownloadItem filename +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_fires() { + const listener = makeListenerExtension(); + await listener.startup(); + await listener.awaitMessage("ready"); + + const caller = makeCallerExtension(); + await caller.startup(); + await caller.awaitMessage("ready"); + + caller.sendMessage(TXT_URL); + + const item = await listener.awaitMessage("determining"); + ok( + item.filename.endsWith(TXT_FILE), + `onDeterminingFilename received the correct filename (got ${item.filename})` + ); + + listener.sendMessage(null); + + await caller.awaitMessage("id"); + await waitForDownloads(); + + ok( + fileInDownloadDir(TXT_FILE).exists(), + "File saved with original name when listener returns null" + ); + fileInDownloadDir(TXT_FILE).remove(false); + + await caller.unload(); + await listener.unload(); +}); + +// --------------------------------------------------------------------------- +// Test: the DownloadItem passed to the listener has expected properties +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_item_properties() { + const listener = makeListenerExtension(); + await listener.startup(); + await listener.awaitMessage("ready"); + + const caller = makeCallerExtension(); + await caller.startup(); + await caller.awaitMessage("ready"); + + caller.sendMessage(TXT_URL); + + const item = await listener.awaitMessage("determining"); + ok(typeof item.id === "number", "item.id is a number"); + ok(item.url.endsWith(TXT_FILE), `item.url (${item.url}) points to the downloaded file`); + ok(item.filename.endsWith(TXT_FILE), `item.filename (${item.filename}) ends with the expected name`); + equal(item.state, "in_progress", "item.state is in_progress at determination time"); + + listener.sendMessage(null); + await caller.awaitMessage("id"); + await waitForDownloads(); + fileInDownloadDir(TXT_FILE).remove(false); + + await caller.unload(); + await listener.unload(); +}); + +// --------------------------------------------------------------------------- +// Test: listener can rename the file +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_rename() { + const RENAMED = "renamed_download.txt"; + + const listener = makeListenerExtension(); + await listener.startup(); + await listener.awaitMessage("ready"); + + const caller = makeCallerExtension(); + await caller.startup(); + await caller.awaitMessage("ready"); + + caller.sendMessage(TXT_URL); + await listener.awaitMessage("determining"); + listener.sendMessage({ filename: RENAMED }); + + await caller.awaitMessage("id"); + await waitForDownloads(); + + ok(fileInDownloadDir(RENAMED).exists(), "File was saved with the suggested name"); + ok(!fileInDownloadDir(TXT_FILE).exists(), "File was not saved with the original name"); + fileInDownloadDir(RENAMED).remove(false); + + await caller.unload(); + await listener.unload(); +}); + +// --------------------------------------------------------------------------- +// Test: listener can rename into a subdirectory +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_subdir() { + const listener = makeListenerExtension(); + await listener.startup(); + await listener.awaitMessage("ready"); + + const caller = makeCallerExtension(); + await caller.startup(); + await caller.awaitMessage("ready"); + + caller.sendMessage(TXT_URL); + await listener.awaitMessage("determining"); + listener.sendMessage({ filename: "subdir/renamed.txt" }); + + await caller.awaitMessage("id"); + await waitForDownloads(); + + ok( + fileInDownloadDir("subdir", "renamed.txt").exists(), + "File was saved in the suggested subdirectory" + ); + fileInDownloadDir("subdir", "renamed.txt").remove(false); + fileInDownloadDir("subdir").remove(false); + + await caller.unload(); + await listener.unload(); +}); + +// --------------------------------------------------------------------------- +// Test: conflictAction "uniquify" when the suggested name already exists +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_conflictAction_uniquify() { + const EXISTING = "existing.txt"; + const UNIQUE = "existing(1).txt"; + + let existing = downloadDir.clone(); + existing.append(EXISTING); + existing.create(Ci.nsIFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE); + + const listener = makeListenerExtension(); + await listener.startup(); + await listener.awaitMessage("ready"); + + const caller = makeCallerExtension(); + await caller.startup(); + await caller.awaitMessage("ready"); + + caller.sendMessage(TXT_URL); + await listener.awaitMessage("determining"); + listener.sendMessage({ filename: EXISTING, conflictAction: "uniquify" }); + + await caller.awaitMessage("id"); + await waitForDownloads(); + + ok(fileInDownloadDir(EXISTING).exists(), "Original pre-existing file is intact"); + ok(fileInDownloadDir(UNIQUE).exists(), "Downloaded file was saved with a uniquified name"); + + fileInDownloadDir(EXISTING).remove(false); + fileInDownloadDir(UNIQUE).remove(false); + + await caller.unload(); + await listener.unload(); +}); + +// --------------------------------------------------------------------------- +// Test: conflictAction "overwrite" when the suggested name already exists +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_conflictAction_overwrite() { + const TARGET = "overwrite_target.txt"; + + let existing = downloadDir.clone(); + existing.append(TARGET); + existing.create(Ci.nsIFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE); + equal(existing.fileSize, 0, "pre-created file starts at zero size"); + + const listener = makeListenerExtension(); + await listener.startup(); + await listener.awaitMessage("ready"); + + const caller = makeCallerExtension(); + await caller.startup(); + await caller.awaitMessage("ready"); + + caller.sendMessage(TXT_URL); + await listener.awaitMessage("determining"); + listener.sendMessage({ filename: TARGET, conflictAction: "overwrite" }); + + await caller.awaitMessage("id"); + await waitForDownloads(); + + let overwritten = fileInDownloadDir(TARGET); + ok(overwritten.exists(), "Target file exists after download"); + equal(overwritten.fileSize, TXT_LEN, "Target file was overwritten with the downloaded content"); + overwritten.remove(false); + + await caller.unload(); + await listener.unload(); +}); + +// --------------------------------------------------------------------------- +// Test: an absolute path suggestion is silently rejected +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_absolute_path_rejected() { + const listener = makeListenerExtension(); + await listener.startup(); + await listener.awaitMessage("ready"); + + const caller = makeCallerExtension(); + await caller.startup(); + await caller.awaitMessage("ready"); + + caller.sendMessage(TXT_URL); + await listener.awaitMessage("determining"); + listener.sendMessage({ + filename: AppConstants.platform === "win" ? "C:\\bad\\path.txt" : "/bad/path.txt", + }); + + await caller.awaitMessage("id"); + await waitForDownloads(); + + ok( + fileInDownloadDir(TXT_FILE).exists(), + "File was saved with the original name when an absolute path was suggested" + ); + fileInDownloadDir(TXT_FILE).remove(false); + + await caller.unload(); + await listener.unload(); +}); + +// --------------------------------------------------------------------------- +// Test: a path-traversal suggestion (..) is silently rejected +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_traversal_rejected() { + const listener = makeListenerExtension(); + await listener.startup(); + await listener.awaitMessage("ready"); + + const caller = makeCallerExtension(); + await caller.startup(); + await caller.awaitMessage("ready"); + + caller.sendMessage(TXT_URL); + await listener.awaitMessage("determining"); + listener.sendMessage({ filename: "../traversal.txt" }); + + await caller.awaitMessage("id"); + await waitForDownloads(); + + ok( + fileInDownloadDir(TXT_FILE).exists(), + "File was saved with the original name when a path-traversal suggestion was made" + ); + fileInDownloadDir(TXT_FILE).remove(false); + + await caller.unload(); + await listener.unload(); +}); + +// --------------------------------------------------------------------------- +// Test: multiple listeners — the first non-null suggestion wins +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_first_suggestion_wins() { + const FIRST = "from_first_listener.txt"; + const SECOND = "from_second_listener.txt"; + + const extension = ExtensionTestUtils.loadExtension({ + manifest: { permissions: ["downloads"] }, + background() { + // This listener returns a suggestion synchronously. + browser.downloads.onDeterminingFilename.addListener(() => ({ + filename: "from_first_listener.txt", + })); + // This listener also returns a suggestion, but must be ignored because + // the first listener already provided one. + browser.downloads.onDeterminingFilename.addListener(() => ({ + filename: "from_second_listener.txt", + })); + browser.test.onMessage.addListener(async url => { + await browser.downloads.download({ url }); + browser.test.sendMessage("done"); + }); + browser.test.sendMessage("ready"); + }, + }); + await extension.startup(); + await extension.awaitMessage("ready"); + + extension.sendMessage(TXT_URL); + await extension.awaitMessage("done"); + await waitForDownloads(); + + ok(fileInDownloadDir(FIRST).exists(), "File was saved with the first listener's suggestion"); + ok(!fileInDownloadDir(SECOND).exists(), "Second listener's suggestion was not used"); + fileInDownloadDir(FIRST).remove(false); + + await extension.unload(); +}); + +// --------------------------------------------------------------------------- +// Test: after removeListener, the callback no longer fires +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_removeListener() { + const extension = ExtensionTestUtils.loadExtension({ + manifest: { permissions: ["downloads"] }, + background() { + let fired = false; + function listener() { + fired = true; + return { filename: "should_not_appear.txt" }; + } + browser.downloads.onDeterminingFilename.addListener(listener); + browser.downloads.onDeterminingFilename.removeListener(listener); + + browser.test.onMessage.addListener(async url => { + await browser.downloads.download({ url }); + browser.test.sendMessage("fired", fired); + }); + browser.test.sendMessage("ready"); + }, + }); + await extension.startup(); + await extension.awaitMessage("ready"); + + extension.sendMessage(TXT_URL); + const fired = await extension.awaitMessage("fired"); + await waitForDownloads(); + + ok(!fired, "Listener did not fire after removeListener"); + ok( + fileInDownloadDir(TXT_FILE).exists(), + "File was saved with the original name after the listener was removed" + ); + fileInDownloadDir(TXT_FILE).remove(false); + + await extension.unload(); +}); + +// --------------------------------------------------------------------------- +// Test: no listener registered — download proceeds normally +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_no_listener() { + const extension = ExtensionTestUtils.loadExtension({ + manifest: { permissions: ["downloads"] }, + background() { + browser.test.onMessage.addListener(async url => { + await browser.downloads.download({ url }); + browser.test.sendMessage("done"); + }); + browser.test.sendMessage("ready"); + }, + }); + await extension.startup(); + await extension.awaitMessage("ready"); + + extension.sendMessage(TXT_URL); + await extension.awaitMessage("done"); + await waitForDownloads(); + + ok( + fileInDownloadDir(TXT_FILE).exists(), + "File was saved normally with no onDeterminingFilename listener registered" + ); + fileInDownloadDir(TXT_FILE).remove(false); + + await extension.unload(); +}); From 22e075dadea7f0296847b76781ebf46ca3321be6 Mon Sep 17 00:00:00 2001 From: Sai Muppiri Date: Mon, 27 Apr 2026 16:17:16 -0400 Subject: [PATCH 07/19] extensions: Register test_ext_downloads_determining_filename in manifest --- .../components/extensions/test/xpcshell/xpcshell-common.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/toolkit/components/extensions/test/xpcshell/xpcshell-common.toml b/toolkit/components/extensions/test/xpcshell/xpcshell-common.toml index 374268ea6c286..54bd0b8e6b30f 100644 --- a/toolkit/components/extensions/test/xpcshell/xpcshell-common.toml +++ b/toolkit/components/extensions/test/xpcshell/xpcshell-common.toml @@ -337,6 +337,11 @@ skip-if = [ ["test_ext_downloads.js"] +["test_ext_downloads_determining_filename.js"] +skip-if = [ + "os == 'android'", +] + ["test_ext_downloads_blob_url.js"] skip-if = [ "os == 'android'", # downloads API needs to be implemented in GeckoView - bug 1538348 From 822e33bcad82bb8eba295a217fa5893350dc5ff4 Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 25 Jun 2026 21:03:22 -0400 Subject: [PATCH 08/19] extensions: Make downloads.onDeterminingFilename a persistent event Move onDeterminingFilename from an ad-hoc EventManager with an explicit register function into PERSISTENT_EVENTS so that the listener survives event-page suspension. Also stop iterating callbacks after the first one that successfully changes the target path (first suggestion wins). --- .../extensions/parent/ext-downloads.js | 88 +++++++++++-------- 1 file changed, 52 insertions(+), 36 deletions(-) diff --git a/toolkit/components/extensions/parent/ext-downloads.js b/toolkit/components/extensions/parent/ext-downloads.js index bd25ae634c8cf..a5c3a5c2492ea 100644 --- a/toolkit/components/extensions/parent/ext-downloads.js +++ b/toolkit/components/extensions/parent/ext-downloads.js @@ -720,6 +720,55 @@ this.downloads = class extends ExtensionAPIPersistent { onErased: this.downloadEventRegistrar("erase", (fire, what, item) => { fire.async(item.id); }), + + onDeterminingFilename: ({ fire }) => { + let { extension } = this; + DownloadMap.lazyInit(); + const callback = async download => { + if (!extension.privateBrowsingAllowed && download.source.isPrivate) { + return; + } + console.log( + `[ext-downloads] onDeterminingFilename fired for extension "${extension.id}", download target: ${download.target.path}` + ); + const item = + DownloadMap.byDownload.get(download) || + DownloadMap.newFromDownload(download, null); + let suggestion; + try { + suggestion = await fire.async(item.serialize()); + } catch (e) { + Cu.reportError(e); + return; + } + console.log( + `[ext-downloads] onDeterminingFilename listener returned suggestion:`, + suggestion + ); + if (suggestion) { + const newPath = await applyFilenameSuggestion( + suggestion, + download.target.path + ); + if (newPath) { + console.log( + `[ext-downloads] onDeterminingFilename changing path: ${download.target.path} -> ${newPath}` + ); + download.target.path = newPath; + download.target.partFilePath = `${newPath}.part`; + } + } + }; + DownloadIntegration._determineFilenameCallbacks.add(callback); + return { + unregister() { + DownloadIntegration._determineFilenameCallbacks.delete(callback); + }, + convert(_fire) { + fire = _fire; + }, + }; + }, }; getAPI(context) { @@ -1334,42 +1383,9 @@ this.downloads = class extends ExtensionAPIPersistent { onDeterminingFilename: new EventManager({ context, - name: "downloads.onDeterminingFilename", - register: fire => { - DownloadMap.lazyInit(); - const callback = async download => { - if (!extension.privateBrowsingAllowed && download.source.isPrivate) { - return; - } - // Ensure the download has an entry in DownloadMap even if it has - // not been added to the DownloadList yet (e.g. legacy downloads - // call start() before list.add()). - const item = - DownloadMap.byDownload.get(download) || - DownloadMap.newFromDownload(download, null); - let suggestion; - try { - suggestion = await fire.async(item.serialize()); - } catch (e) { - Cu.reportError(e); - return; - } - if (suggestion) { - const newPath = await applyFilenameSuggestion( - suggestion, - download.target.path - ); - if (newPath) { - download.target.path = newPath; - download.target.partFilePath = `${newPath}.part`; - } - } - }; - DownloadIntegration._determineFilenameCallbacks.add(callback); - return () => { - DownloadIntegration._determineFilenameCallbacks.delete(callback); - }; - }, + module: "downloads", + event: "onDeterminingFilename", + extensionApi: this, }).api(), }, }; From 67b0f8053e8083354e5cc8ca3029b8ce0b3d6090 Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 25 Jun 2026 21:03:28 -0400 Subject: [PATCH 09/19] downloads: Add logging and first-suggestion-wins to determineFilename() Log each determineFilename() call and each callback invocation for debugging. Stop iterating after the first callback that changes the download's target path so that the first listener's suggestion wins. --- .../components/downloads/DownloadIntegration.sys.mjs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/toolkit/components/downloads/DownloadIntegration.sys.mjs b/toolkit/components/downloads/DownloadIntegration.sys.mjs index eab09f573666f..fa93e0862e258 100644 --- a/toolkit/components/downloads/DownloadIntegration.sys.mjs +++ b/toolkit/components/downloads/DownloadIntegration.sys.mjs @@ -425,12 +425,22 @@ export var DownloadIntegration = { * @param {Download} download */ async determineFilename(download) { + console.log( + `[DownloadIntegration] determineFilename called for ${download.target.path} (${this._determineFilenameCallbacks.size} listener(s))` + ); + const originalPath = download.target.path; for (let callback of this._determineFilenameCallbacks) { + console.log( + `[DownloadIntegration] invoking onDeterminingFilename callback for ${download.target.path}` + ); try { await callback(download); } catch (ex) { console.error(ex); } + if (download.target.path !== originalPath) { + break; + } } }, From 1faae3d9f6f52f42d9d567371d6a5ab913c314f2 Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 25 Jun 2026 21:03:36 -0400 Subject: [PATCH 10/19] extensions: Add child-side downloads module to inject suggest callback Chrome's onDeterminingFilename API passes a suggest() callback as the second argument to listeners. Functions cannot cross the IPC boundary, so inject suggest() in the child process by wrapping each listener at addListener() time. Handles both the synchronous call pattern and the async pattern where the listener returns true and calls suggest() later. --- .../extensions/child/ext-downloads.js | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 toolkit/components/extensions/child/ext-downloads.js diff --git a/toolkit/components/extensions/child/ext-downloads.js b/toolkit/components/extensions/child/ext-downloads.js new file mode 100644 index 0000000000000..459f61083b6a7 --- /dev/null +++ b/toolkit/components/extensions/child/ext-downloads.js @@ -0,0 +1,89 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +"use strict"; + +let nextListenerId = 0; + +this.downloads = class extends ExtensionAPI { + getAPI(context) { + const childManager = context.childManager; + const path = "downloads.onDeterminingFilename"; + + return { + downloads: { + onDeterminingFilename: { + addListener(fn) { + const map = childManager.listeners.get(path); + if (map.listeners.has(fn)) { + return; + } + + const id = ++nextListenerId; + + const wrappedFn = item => { + let suggestCalled = false; + let suggestValue; + let resolveAsync; + + const suggest = Cu.exportFunction(suggestion => { + suggestCalled = true; + suggestValue = suggestion; + if (resolveAsync) { + resolveAsync(suggestion); + } + }, context.cloneScope); + + const result = context.applySafeWithoutClone(fn, [item, suggest]); + + if (result === true) { + if (suggestCalled) { + return suggestValue; + } + return new Promise(resolve => { + resolveAsync = resolve; + }); + } + if (suggestCalled) { + return suggestValue; + } + return result; + }; + + map.ids.set(id, wrappedFn); + map.listeners.set(fn, id); + + childManager.conduit.sendAddListener({ + childId: childManager.id, + listenerId: id, + path, + args: [], + }); + }, + + removeListener(fn) { + const map = childManager.listeners.get(path); + if (!map.listeners.has(fn)) { + return; + } + + const id = map.listeners.get(fn); + map.listeners.delete(fn); + map.ids.delete(id); + map.removedIds.add(id); + + childManager.conduit.sendRemoveListener({ + childId: childManager.id, + listenerId: id, + path, + }); + }, + + hasListener(fn) { + return childManager.listeners.get(path).listeners.has(fn); + }, + }, + }, + }; + } +}; From eafc2cecae73d35eeecf894a60ae866d2481200d Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 25 Jun 2026 21:03:43 -0400 Subject: [PATCH 11/19] extensions: Register child-side downloads module in ext-toolkit.js --- toolkit/components/extensions/child/ext-toolkit.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/toolkit/components/extensions/child/ext-toolkit.js b/toolkit/components/extensions/child/ext-toolkit.js index 2d8f4344d0df8..8764167e3bd1c 100644 --- a/toolkit/components/extensions/child/ext-toolkit.js +++ b/toolkit/components/extensions/child/ext-toolkit.js @@ -7,6 +7,11 @@ global.EventManager = ExtensionCommon.EventManager; extensions.registerModules({ + downloads: { + url: "chrome://extensions/content/child/ext-downloads.js", + scopes: ["addon_child"], + paths: [["downloads", "onDeterminingFilename"]], + }, backgroundPage: { url: "chrome://extensions/content/child/ext-backgroundPage.js", scopes: ["addon_child"], From fcfbaf1f72cf8812ab8c4dea23614758acd665f4 Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 25 Jun 2026 21:03:49 -0400 Subject: [PATCH 12/19] extensions: Package child/ext-downloads.js in jar.mn --- toolkit/components/extensions/jar.mn | 1 + 1 file changed, 1 insertion(+) diff --git a/toolkit/components/extensions/jar.mn b/toolkit/components/extensions/jar.mn index c889b6c6d1710..922b49df02965 100644 --- a/toolkit/components/extensions/jar.mn +++ b/toolkit/components/extensions/jar.mn @@ -52,6 +52,7 @@ toolkit.jar: content/extensions/parent/ext-webRequest.js (parent/ext-webRequest.js) content/extensions/parent/ext-webNavigation.js (parent/ext-webNavigation.js) content/extensions/child/ext-backgroundPage.js (child/ext-backgroundPage.js) + content/extensions/child/ext-downloads.js (child/ext-downloads.js) content/extensions/child/ext-contentScripts.js (child/ext-contentScripts.js) content/extensions/child/ext-declarativeNetRequest.js (child/ext-declarativeNetRequest.js) content/extensions/child/ext-extension.js (child/ext-extension.js) From 65da327233a3f70a56db29441d0aa31e71aac6ac Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 2 Jul 2026 19:37:42 -0400 Subject: [PATCH 13/19] downloads: Add determineFilenameBeforeDialog() and double-fire guard Adds determineFilenameBeforeDialog() to fire onDeterminingFilename listeners before the auto-save path or Save As dialog is chosen. The _determinedDownloads WeakSet prevents determineFilename() in Download.start() from firing the event a second time for the same download. Also adds _processedLaunchers to track which nsIHelperAppLauncher objects have already been through HelperAppDlg, so DownloadLegacy can set the guard when creating the Download object. --- .../downloads/DownloadIntegration.sys.mjs | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/toolkit/components/downloads/DownloadIntegration.sys.mjs b/toolkit/components/downloads/DownloadIntegration.sys.mjs index fa93e0862e258..c0fc1ccac56fa 100644 --- a/toolkit/components/downloads/DownloadIntegration.sys.mjs +++ b/toolkit/components/downloads/DownloadIntegration.sys.mjs @@ -417,6 +417,93 @@ export var DownloadIntegration = { */ _determineFilenameCallbacks: new Set(), + /** + * Downloads for which determineFilename() was already called early (before + * the Save As dialog) via the extension API. Download.start() checks this + * to avoid firing onDeterminingFilename a second time. + */ + _determinedDownloads: new WeakSet(), + + markFilenameAlreadyDetermined(download) { + this._determinedDownloads.add(download); + }, + + /** + * Tracks nsIHelperAppLauncher objects for which determineFilenameBeforeDialog + * has already been called, so DownloadLegacy can avoid double-firing + * onDeterminingFilename in Download.start(). + */ + _processedLaunchers: new WeakSet(), + + markLauncherProcessed(launcher) { + this._processedLaunchers.add(launcher.QueryInterface(Ci.nsISupports)); + }, + + wasLauncherProcessed(cancelable) { + try { + return this._processedLaunchers.has( + cancelable.QueryInterface(Ci.nsISupports) + ); + } catch { + return false; + } + }, + + /** + * Called from the helper-app download dialog before either the auto-save + * path or the Save As file picker is chosen, giving onDeterminingFilename + * listeners a chance to redirect the download to a different filename before + * the user sees any dialog. + * + * @param {string} url Source URL of the download. + * @param {string} tentativePath Full tentative destination path (using the + * default download directory and suggested + * filename). + * @param {string|null} mimeType MIME type of the download, if known. + * @param {boolean} isPrivate Whether this is a private-browsing download. + * @returns {Promise} Resolves to the (possibly updated) path. + */ + async determineFilenameBeforeDialog(url, tentativePath, mimeType, isPrivate) { + if (!this._determineFilenameCallbacks.size) { + return tentativePath; + } + const syntheticDownload = { + source: { url, isPrivate: !!isPrivate }, + target: { path: tentativePath, partFilePath: `${tentativePath}.part` }, + _syntheticSerialized: { + id: -1, + url, + referrer: null, + filename: PathUtils.filename(tentativePath), + incognito: !!isPrivate, + cookieStoreId: isPrivate ? "firefox-private" : "firefox-default", + danger: "safe", + mime: mimeType ?? null, + startTime: null, + endTime: null, + estimatedEndTime: null, + state: "in_progress", + paused: false, + canResume: false, + error: null, + bytesReceived: 0, + totalBytes: -1, + fileSize: -1, + exists: false, + byExtensionId: null, + byExtensionName: null, + }, + }; + console.log( + `[DownloadIntegration] determineFilenameBeforeDialog: url=${url}, tentativePath=${tentativePath}, callbacks=${this._determineFilenameCallbacks.size}` + ); + await this.determineFilename(syntheticDownload); + console.log( + `[DownloadIntegration] determineFilenameBeforeDialog result: ${syntheticDownload.target.path}` + ); + return syntheticDownload.target.path; + }, + /** * Called by Download.start() before execution begins, to give registered * callbacks (i.e. WebExtension onDeterminingFilename listeners) a chance to @@ -425,6 +512,9 @@ export var DownloadIntegration = { * @param {Download} download */ async determineFilename(download) { + if (this._determinedDownloads.has(download)) { + return; + } console.log( `[DownloadIntegration] determineFilename called for ${download.target.path} (${this._determineFilenameCallbacks.size} listener(s))` ); From ee7578ed06372b1d4a1280b6304d3f475018e147 Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 2 Jul 2026 19:37:49 -0400 Subject: [PATCH 14/19] downloads: Prevent double-firing of onDeterminingFilename in legacy path When HelperAppDlg fires onDeterminingFilename before showing the Save As dialog, mark the nsIHelperAppLauncher as processed. DownloadLegacy checks this flag when the Download object is created and calls markFilenameAlreadyDetermined() so Download.start() skips the event. --- .../downloads/DownloadLegacy.sys.mjs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/toolkit/components/downloads/DownloadLegacy.sys.mjs b/toolkit/components/downloads/DownloadLegacy.sys.mjs index a60af5ed30c6a..00e6ee59e93dd 100644 --- a/toolkit/components/downloads/DownloadLegacy.sys.mjs +++ b/toolkit/components/downloads/DownloadLegacy.sys.mjs @@ -16,6 +16,7 @@ const lazy = {}; ChromeUtils.defineESModuleGetters(lazy, { DownloadError: "resource://gre/modules/DownloadCore.sys.mjs", + DownloadIntegration: "resource://gre/modules/DownloadIntegration.sys.mjs", Downloads: "resource://gre/modules/Downloads.sys.mjs", }); @@ -347,6 +348,8 @@ DownloadLegacyTransfer.prototype = { aHttpChannel = null ) { this._cancelable = aCancelable; + this._determineFilenameCalledBeforeDialog = + lazy.DownloadIntegration.wasLauncherProcessed(aCancelable); let launchWhenSucceeded = false, contentType = null, launcherPath = null, @@ -431,6 +434,21 @@ DownloadLegacyTransfer.prototype = { aDownload.tryToKeepPartialData = true; } + // If onDeterminingFilename was already fired by HelperAppDlg before + // the Save As dialog (save-to-disk path), prevent Download.start() + // from firing it again. Downloads that bypassed HelperAppDlg (e.g. + // SetDownloadToLaunch for helper-app opens) go through normally. + if (this._determineFilenameCalledBeforeDialog) { + console.log( + `[DownloadLegacy] marking download as already determined: ${aDownload.target.path}` + ); + lazy.DownloadIntegration.markFilenameAlreadyDetermined(aDownload); + } else { + console.log( + `[DownloadLegacy] download was NOT pre-processed by HelperAppDlg; determineFilename will run in Download.start(): ${aDownload.target.path}` + ); + } + // Start the download before allowing it to be controlled. Ignore errors. aDownload.start().catch(() => {}); From 064e3fd013a510e26528eeb5fc4b1d1edd3ed1f7 Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 2 Jul 2026 19:37:55 -0400 Subject: [PATCH 15/19] extensions: Fire onDeterminingFilename before Save As dialog in downloads.download() When saveAs is true, fire onDeterminingFilename listeners on a synthetic download before showing the file picker, so extensions can redirect the filename before the user sees the dialog. Uses _syntheticSerialized to avoid adding the download to DownloadMap prematurely. Marks the download as already determined so Download.start() does not re-fire the event. --- .../extensions/parent/ext-downloads.js | 49 +++++++++++++++++-- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/toolkit/components/extensions/parent/ext-downloads.js b/toolkit/components/extensions/parent/ext-downloads.js index a5c3a5c2492ea..cf2bdcf2ef642 100644 --- a/toolkit/components/extensions/parent/ext-downloads.js +++ b/toolkit/components/extensions/parent/ext-downloads.js @@ -731,12 +731,12 @@ this.downloads = class extends ExtensionAPIPersistent { console.log( `[ext-downloads] onDeterminingFilename fired for extension "${extension.id}", download target: ${download.target.path}` ); - const item = - DownloadMap.byDownload.get(download) || - DownloadMap.newFromDownload(download, null); + const serialized = download._syntheticSerialized ?? + (DownloadMap.byDownload.get(download) || + DownloadMap.newFromDownload(download, null)).serialize(); let suggestion; try { - suggestion = await fire.async(item.serialize()); + suggestion = await fire.async(serialized); } catch (e) { Cu.reportError(e); return; @@ -936,6 +936,8 @@ this.downloads = class extends ExtensionAPIPersistent { return true; } + let determineFilenameCalledBeforeDialog = false; + async function createTarget() { if (!filename) { let uri = Services.io.newURI(options.url); @@ -1003,6 +1005,41 @@ this.downloads = class extends ExtensionAPIPersistent { return target; } + // Give onDeterminingFilename listeners a chance to change the + // filename before the user sees the Save As dialog. + { + const syntheticDownload = { + source: { url: options.url, isPrivate: options.incognito }, + target: { path: target, partFilePath: `${target}.part` }, + _syntheticSerialized: { + id: DownloadMap.currentId + 1, + url: options.url, + referrer: null, + filename: target, + incognito: !!options.incognito, + cookieStoreId: options.incognito ? PRIVATE_STORE : DEFAULT_STORE, + danger: "safe", + mime: null, + startTime: null, + endTime: null, + estimatedEndTime: null, + state: "in_progress", + paused: false, + canResume: false, + error: null, + bytesReceived: 0, + totalBytes: -1, + fileSize: -1, + exists: false, + byExtensionId: extension.id, + byExtensionName: extension.name, + }, + }; + await DownloadIntegration.determineFilename(syntheticDownload); + target = syntheticDownload.target.path; + determineFilenameCalledBeforeDialog = true; + } + // At this point we are committed to displaying the file picker. const downloadLastDir = new DownloadLastDir( null, @@ -1163,6 +1200,10 @@ this.downloads = class extends ExtensionAPIPersistent { const item = DownloadMap.newFromDownload(download, extension); list.add(download); + if (determineFilenameCalledBeforeDialog) { + DownloadIntegration.markFilenameAlreadyDetermined(download); + } + // This is necessary to make pause/resume work. download.tryToKeepPartialData = true; From aeb696ff9a7b0161d380bc9c398bdfc2b6d55394 Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 2 Jul 2026 19:38:08 -0400 Subject: [PATCH 16/19] downloads: Fire onDeterminingFilename in HelperAppDlg before save path is chosen Call determineFilenameBeforeDialog() at the start of promptForSaveToFileAsync() so onDeterminingFilename listeners can redirect the download before auto-save or the Save As file picker runs. For auto-save, create any suggested subdirectory and pass only the leaf name to validateLeafName(). For the file picker, open the picker in the suggested subdirectory and show only the leaf name as the default string, skipping the lastDir override when a subdirectory was explicitly suggested by an extension. --- .../mozapps/downloads/HelperAppDlg.sys.mjs | 104 +++++++++++++++--- 1 file changed, 90 insertions(+), 14 deletions(-) diff --git a/toolkit/mozapps/downloads/HelperAppDlg.sys.mjs b/toolkit/mozapps/downloads/HelperAppDlg.sys.mjs index 2a7ba8701c331..7cbece8c701ed 100644 --- a/toolkit/mozapps/downloads/HelperAppDlg.sys.mjs +++ b/toolkit/mozapps/downloads/HelperAppDlg.sys.mjs @@ -288,6 +288,45 @@ export class nsUnknownContentTypeDialog { } (async () => { + // Retrieve the preferred download directory once, used both for the + // extension hook below and for auto-save / file-picker defaults. + let preferredDir = await Downloads.getPreferredDownloadsDirectory(); + + // Give onDeterminingFilename listeners a chance to rename the file + // before the user sees any dialog or the auto-save path is chosen. + console.log( + `[HelperAppDlg] promptForSaveToFileAsync: aDefaultFileName=${aDefaultFileName}, preferredDir=${preferredDir}` + ); + if (aDefaultFileName) { + const isPrivate = + BrowsingContext.get(aLauncher.browsingContextId) + ?.usePrivateBrowsing ?? false; + const tentativePath = PathUtils.join(preferredDir, aDefaultFileName); + console.log( + `[HelperAppDlg] calling determineFilenameBeforeDialog: url=${aLauncher.source.spec}, tentativePath=${tentativePath}, isPrivate=${isPrivate}` + ); + const newPath = + await lazy.DownloadIntegration.determineFilenameBeforeDialog( + aLauncher.source.spec, + tentativePath, + aLauncher.MIMEInfo?.MIMEType ?? null, + isPrivate + ); + console.log( + `[HelperAppDlg] determineFilenameBeforeDialog returned: ${newPath}` + ); + lazy.DownloadIntegration.markLauncherProcessed(aLauncher); + if (newPath !== tentativePath) { + const sep = AppConstants.platform === "win" ? "\\" : "/"; + aDefaultFileName = newPath.startsWith(preferredDir + sep) + ? newPath.slice(preferredDir.length + 1) + : PathUtils.filename(newPath); + console.log( + `[HelperAppDlg] aDefaultFileName updated to: ${aDefaultFileName}` + ); + } + } + if (!aForcePrompt) { // Check to see if the user wishes to auto save to the default download // folder without prompting. Note that preference might not be set. @@ -297,17 +336,36 @@ export class nsUnknownContentTypeDialog { ); if (autodownload) { - // Retrieve the user's default download directory - let preferredDir = await Downloads.getPreferredDownloadsDirectory(); let defaultFolder = new FileUtils.File(preferredDir); try { if (aDefaultFileName) { - result = this.validateLeafName( - defaultFolder, - aDefaultFileName, - aSuggestedFileExtension - ); + const sep = AppConstants.platform === "win" ? "\\" : "/"; + const sepIdx = aDefaultFileName.lastIndexOf(sep); + if (sepIdx !== -1) { + const subdir = aDefaultFileName.slice(0, sepIdx); + const leafName = aDefaultFileName.slice(sepIdx + 1); + const subFolder = defaultFolder.clone(); + for (const part of subdir.split(sep)) { + if (part) { + subFolder.append(part); + } + } + await IOUtils.makeDirectory(subFolder.path, { + createAncestors: true, + }); + result = this.validateLeafName( + subFolder, + leafName, + aSuggestedFileExtension + ); + } else { + result = this.validateLeafName( + defaultFolder, + aDefaultFileName, + aSuggestedFileExtension + ); + } } } catch (ex) { // When the default download directory is write-protected, @@ -329,8 +387,28 @@ export class nsUnknownContentTypeDialog { Cc["@mozilla.org/filepicker;1"].createInstance(nsIFilePicker); var windowTitle = bundle.GetStringFromName("saveDialogTitle"); picker.init(parent.browsingContext, windowTitle, nsIFilePicker.modeSave); + + // If the extension suggested a subdirectory (e.g. "images/photo.png"), + // split it out so the file picker shows the leaf name only and opens in + // the right directory. + const sep = AppConstants.platform === "win" ? "\\" : "/"; + let pickerLeafName = aDefaultFileName; + let suggestedPickerDir = null; if (aDefaultFileName) { - picker.defaultString = this.getFinalLeafName(aDefaultFileName); + const sepIdx = aDefaultFileName.lastIndexOf(sep); + if (sepIdx !== -1) { + const subdir = aDefaultFileName.slice(0, sepIdx); + pickerLeafName = aDefaultFileName.slice(sepIdx + 1); + const dir = new FileUtils.File(preferredDir); + for (const part of subdir.split(sep)) { + if (part) { + dir.append(part); + } + } + await IOUtils.makeDirectory(dir.path, { createAncestors: true }); + suggestedPickerDir = dir; + } + picker.defaultString = this.getFinalLeafName(pickerLeafName); } if (aSuggestedFileExtension) { @@ -353,14 +431,12 @@ export class nsUnknownContentTypeDialog { picker.appendFilters(nsIFilePicker.filterAll); - // Default to lastDir if it is valid, otherwise use the user's default - // downloads directory. getPreferredDownloadsDirectory should always - // return a valid directory path, so we can safely default to it. - let preferredDir = await Downloads.getPreferredDownloadsDirectory(); - picker.displayDirectory = new FileUtils.File(preferredDir); + // If the extension suggested a subdirectory, open the picker there. + // Otherwise default to lastDir (if valid) or the preferred downloads dir. + picker.displayDirectory = suggestedPickerDir ?? new FileUtils.File(preferredDir); gDownloadLastDir.getFileAsync(aLauncher.source).then(lastDir => { - if (lastDir && isUsableDirectory(lastDir)) { + if (!suggestedPickerDir && lastDir && isUsableDirectory(lastDir)) { picker.displayDirectory = lastDir; } From f74b49b357eca34c573d78b33c1decfe7f34b62c Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 2 Jul 2026 19:48:48 -0400 Subject: [PATCH 17/19] extensions: Compute unique filename without creating placeholder in applyFilenameSuggestion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createNiceUniqueFile physically creates a zero-byte file to claim the name, which is correct when a download is about to start but wrong when called from applyFilenameSuggestion — the file picker hasn't run yet, so the placeholder appears in the directory and triggers a "file already exists" prompt when the user tries to save. Inline the same base(N)ext loop using splitBaseNameAndExtension to find a non-conflicting name without touching the filesystem. The actual placeholder is created later by validateLeafName (auto-save path) or the download core (file picker path). --- .../components/extensions/parent/ext-downloads.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/toolkit/components/extensions/parent/ext-downloads.js b/toolkit/components/extensions/parent/ext-downloads.js index cf2bdcf2ef642..f38f7b695b8c8 100644 --- a/toolkit/components/extensions/parent/ext-downloads.js +++ b/toolkit/components/extensions/parent/ext-downloads.js @@ -661,8 +661,19 @@ async function applyFilenameSuggestion(suggestion, currentPath) { if (await IOUtils.exists(newPath)) { if (resolvedConflictAction === "uniquify") { - return DownloadPaths.createNiceUniqueFile(new FileUtils.File(newPath)) - .path; + // Compute a unique name without creating a placeholder file, since the + // download hasn't started yet (the file picker may still run). The + // actual placeholder is created by validateLeafName or the download core. + const uniqueDir = PathUtils.parent(newPath); + let [base, ext] = DownloadPaths.splitBaseNameAndExtension( + PathUtils.filename(newPath) + ); + for (let i = 1; i < 10000; i++) { + const candidate = PathUtils.join(uniqueDir, base + "(" + i + ")" + ext); + if (!(await IOUtils.exists(candidate))) { + return candidate; + } + } } // "overwrite" — use the path as-is } From 38e2d0d31f1d24d5a938764156e83e6be503443b Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 2 Jul 2026 19:54:13 -0400 Subject: [PATCH 18/19] downloads: Remove debug logging added during development --- .../components/downloads/DownloadIntegration.sys.mjs | 12 ------------ toolkit/components/downloads/DownloadLegacy.sys.mjs | 7 ------- .../components/extensions/parent/ext-downloads.js | 10 ---------- toolkit/mozapps/downloads/HelperAppDlg.sys.mjs | 12 ------------ 4 files changed, 41 deletions(-) diff --git a/toolkit/components/downloads/DownloadIntegration.sys.mjs b/toolkit/components/downloads/DownloadIntegration.sys.mjs index c0fc1ccac56fa..8572743d404e3 100644 --- a/toolkit/components/downloads/DownloadIntegration.sys.mjs +++ b/toolkit/components/downloads/DownloadIntegration.sys.mjs @@ -494,13 +494,7 @@ export var DownloadIntegration = { byExtensionName: null, }, }; - console.log( - `[DownloadIntegration] determineFilenameBeforeDialog: url=${url}, tentativePath=${tentativePath}, callbacks=${this._determineFilenameCallbacks.size}` - ); await this.determineFilename(syntheticDownload); - console.log( - `[DownloadIntegration] determineFilenameBeforeDialog result: ${syntheticDownload.target.path}` - ); return syntheticDownload.target.path; }, @@ -515,14 +509,8 @@ export var DownloadIntegration = { if (this._determinedDownloads.has(download)) { return; } - console.log( - `[DownloadIntegration] determineFilename called for ${download.target.path} (${this._determineFilenameCallbacks.size} listener(s))` - ); const originalPath = download.target.path; for (let callback of this._determineFilenameCallbacks) { - console.log( - `[DownloadIntegration] invoking onDeterminingFilename callback for ${download.target.path}` - ); try { await callback(download); } catch (ex) { diff --git a/toolkit/components/downloads/DownloadLegacy.sys.mjs b/toolkit/components/downloads/DownloadLegacy.sys.mjs index 00e6ee59e93dd..be151fc678974 100644 --- a/toolkit/components/downloads/DownloadLegacy.sys.mjs +++ b/toolkit/components/downloads/DownloadLegacy.sys.mjs @@ -439,14 +439,7 @@ DownloadLegacyTransfer.prototype = { // from firing it again. Downloads that bypassed HelperAppDlg (e.g. // SetDownloadToLaunch for helper-app opens) go through normally. if (this._determineFilenameCalledBeforeDialog) { - console.log( - `[DownloadLegacy] marking download as already determined: ${aDownload.target.path}` - ); lazy.DownloadIntegration.markFilenameAlreadyDetermined(aDownload); - } else { - console.log( - `[DownloadLegacy] download was NOT pre-processed by HelperAppDlg; determineFilename will run in Download.start(): ${aDownload.target.path}` - ); } // Start the download before allowing it to be controlled. Ignore errors. diff --git a/toolkit/components/extensions/parent/ext-downloads.js b/toolkit/components/extensions/parent/ext-downloads.js index f38f7b695b8c8..fad811703c7ee 100644 --- a/toolkit/components/extensions/parent/ext-downloads.js +++ b/toolkit/components/extensions/parent/ext-downloads.js @@ -739,9 +739,6 @@ this.downloads = class extends ExtensionAPIPersistent { if (!extension.privateBrowsingAllowed && download.source.isPrivate) { return; } - console.log( - `[ext-downloads] onDeterminingFilename fired for extension "${extension.id}", download target: ${download.target.path}` - ); const serialized = download._syntheticSerialized ?? (DownloadMap.byDownload.get(download) || DownloadMap.newFromDownload(download, null)).serialize(); @@ -752,19 +749,12 @@ this.downloads = class extends ExtensionAPIPersistent { Cu.reportError(e); return; } - console.log( - `[ext-downloads] onDeterminingFilename listener returned suggestion:`, - suggestion - ); if (suggestion) { const newPath = await applyFilenameSuggestion( suggestion, download.target.path ); if (newPath) { - console.log( - `[ext-downloads] onDeterminingFilename changing path: ${download.target.path} -> ${newPath}` - ); download.target.path = newPath; download.target.partFilePath = `${newPath}.part`; } diff --git a/toolkit/mozapps/downloads/HelperAppDlg.sys.mjs b/toolkit/mozapps/downloads/HelperAppDlg.sys.mjs index 7cbece8c701ed..bdc8c7d08976f 100644 --- a/toolkit/mozapps/downloads/HelperAppDlg.sys.mjs +++ b/toolkit/mozapps/downloads/HelperAppDlg.sys.mjs @@ -294,17 +294,11 @@ export class nsUnknownContentTypeDialog { // Give onDeterminingFilename listeners a chance to rename the file // before the user sees any dialog or the auto-save path is chosen. - console.log( - `[HelperAppDlg] promptForSaveToFileAsync: aDefaultFileName=${aDefaultFileName}, preferredDir=${preferredDir}` - ); if (aDefaultFileName) { const isPrivate = BrowsingContext.get(aLauncher.browsingContextId) ?.usePrivateBrowsing ?? false; const tentativePath = PathUtils.join(preferredDir, aDefaultFileName); - console.log( - `[HelperAppDlg] calling determineFilenameBeforeDialog: url=${aLauncher.source.spec}, tentativePath=${tentativePath}, isPrivate=${isPrivate}` - ); const newPath = await lazy.DownloadIntegration.determineFilenameBeforeDialog( aLauncher.source.spec, @@ -312,18 +306,12 @@ export class nsUnknownContentTypeDialog { aLauncher.MIMEInfo?.MIMEType ?? null, isPrivate ); - console.log( - `[HelperAppDlg] determineFilenameBeforeDialog returned: ${newPath}` - ); lazy.DownloadIntegration.markLauncherProcessed(aLauncher); if (newPath !== tentativePath) { const sep = AppConstants.platform === "win" ? "\\" : "/"; aDefaultFileName = newPath.startsWith(preferredDir + sep) ? newPath.slice(preferredDir.length + 1) : PathUtils.filename(newPath); - console.log( - `[HelperAppDlg] aDefaultFileName updated to: ${aDefaultFileName}` - ); } } From f65c33c25e711ebae764109bcc72a2c21cc2c6e5 Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 2 Jul 2026 19:59:32 -0400 Subject: [PATCH 19/19] extensions: Add tests for double-fire guard and uniquify counter behavior - test_onDeterminingFilename_double_fire_guard: verifies that markFilenameAlreadyDetermined() prevents the listener from firing in Download.start() when the event was already dispatched earlier (e.g. by HelperAppDlg before the Save As dialog). - test_onDeterminingFilename_uniquify_increments_counter: verifies that when both file.txt and file(1).txt already exist, the suggestion resolves to file(2).txt without creating any placeholder file. - test_onDeterminingFilename_default_conflict_action_is_uniquify: verifies that omitting conflictAction defaults to "uniquify" rather than silently overwriting the existing file. --- ...test_ext_downloads_determining_filename.js | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_downloads_determining_filename.js b/toolkit/components/extensions/test/xpcshell/test_ext_downloads_determining_filename.js index 814e102d90e7c..d0850e6cf24cd 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_downloads_determining_filename.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_downloads_determining_filename.js @@ -4,6 +4,10 @@ const { Downloads } = ChromeUtils.importESModule( "resource://gre/modules/Downloads.sys.mjs" ); +const { DownloadIntegration } = ChromeUtils.importESModule( + "resource://gre/modules/DownloadIntegration.sys.mjs" +); + const gServer = createHttpServer(); gServer.registerDirectory("/data/", do_get_file("data")); @@ -431,6 +435,130 @@ add_task(async function test_onDeterminingFilename_removeListener() { await extension.unload(); }); +// --------------------------------------------------------------------------- +// Test: markFilenameAlreadyDetermined prevents listener from firing a second time +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_double_fire_guard() { + const target = PathUtils.join(downloadDir.path, "guard_test.txt"); + + let callCount = 0; + const callback = async () => { + callCount++; + }; + DownloadIntegration._determineFilenameCallbacks.add(callback); + + const download = await Downloads.createDownload({ + source: TXT_URL, + target, + }); + + // Simulate what HelperAppDlg / the saveAs path does before Download.start(). + DownloadIntegration.markFilenameAlreadyDetermined(download); + + await download.start(); + await download.whenSucceeded(); + + equal(callCount, 0, "Listener not called when download is already marked as determined"); + + DownloadIntegration._determineFilenameCallbacks.delete(callback); + + const list = await Downloads.getList(Downloads.ALL); + list.remove(download); + await IOUtils.remove(target); +}); + +// --------------------------------------------------------------------------- +// Test: uniquify counter increments past (1) when the first candidate is taken +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_uniquify_increments_counter() { + for (const name of ["file.txt", "file(1).txt"]) { + let f = downloadDir.clone(); + f.append(name); + f.create(Ci.nsIFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE); + } + + const extension = ExtensionTestUtils.loadExtension({ + manifest: { permissions: ["downloads"] }, + background() { + browser.downloads.onDeterminingFilename.addListener(() => ({ + filename: "file.txt", + conflictAction: "uniquify", + })); + browser.test.onMessage.addListener(async url => { + await browser.downloads.download({ url }); + browser.test.sendMessage("done"); + }); + browser.test.sendMessage("ready"); + }, + }); + await extension.startup(); + await extension.awaitMessage("ready"); + + extension.sendMessage(TXT_URL); + await extension.awaitMessage("done"); + await waitForDownloads(); + + ok( + fileInDownloadDir("file(2).txt").exists(), + "Download skipped to (2) when file.txt and file(1).txt were already present" + ); + equal( + fileInDownloadDir("file(2).txt").fileSize, + TXT_LEN, + "Uniquified file has the correct downloaded content" + ); + + for (const name of ["file.txt", "file(1).txt", "file(2).txt"]) { + fileInDownloadDir(name).remove(false); + } + + await extension.unload(); +}); + +// --------------------------------------------------------------------------- +// Test: omitting conflictAction defaults to "uniquify" +// --------------------------------------------------------------------------- +add_task(async function test_onDeterminingFilename_default_conflict_action_is_uniquify() { + let existing = downloadDir.clone(); + existing.append("default_conflict.txt"); + existing.create(Ci.nsIFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE); + + const extension = ExtensionTestUtils.loadExtension({ + manifest: { permissions: ["downloads"] }, + background() { + browser.downloads.onDeterminingFilename.addListener(() => ({ + filename: "default_conflict.txt", + })); + browser.test.onMessage.addListener(async url => { + await browser.downloads.download({ url }); + browser.test.sendMessage("done"); + }); + browser.test.sendMessage("ready"); + }, + }); + await extension.startup(); + await extension.awaitMessage("ready"); + + extension.sendMessage(TXT_URL); + await extension.awaitMessage("done"); + await waitForDownloads(); + + ok( + fileInDownloadDir("default_conflict(1).txt").exists(), + "File was uniquified when conflictAction was not specified" + ); + equal( + fileInDownloadDir("default_conflict.txt").fileSize, + 0, + "Original pre-existing file was not overwritten" + ); + + fileInDownloadDir("default_conflict.txt").remove(false); + fileInDownloadDir("default_conflict(1).txt").remove(false); + + await extension.unload(); +}); + // --------------------------------------------------------------------------- // Test: no listener registered — download proceeds normally // ---------------------------------------------------------------------------