[tor-commits] [tor-browser] 50/73: Bug 1739348 - Don't open downloads panel after download dialogs. r=NeilDeakin, a=RyanVM
gitolite role
git at cupani.torproject.org
Wed Sep 21 20:17:43 UTC 2022
This is an automated email from the git hooks/post-receive script.
richard pushed a commit to branch geckoview-102.3.0esr-12.0-1
in repository tor-browser.
commit 226da410d935623597a0b535222d4386031b1610
Author: Shane Hughes <shmediaproductions at gmail.com>
AuthorDate: Mon Jul 18 20:45:41 2022 +0000
Bug 1739348 - Don't open downloads panel after download dialogs. r=NeilDeakin, a=RyanVM
This is a medium sized patch to legacy download construction. It takes
advantage of the new property added in Bug 1762033 to prevent the
downloads panel from being automatically shown when a download is added
after an interaction with the unknown content type dialog or the file
picker dialog. I chose to not do the same for failed transfers since I
thought it might serve some use, but that might be wrong. I don't know
if there's a way to test the dialog that appears when you download an
executable without going through the same path I adjusted with the
patch. It seems like it's covered but I could be wrong. Also add a test
to cover these changes from the bottom up. Thanks and apologies for my
sloppy C++, though I'm sure I'll learn a lot more from the review 😅
Differential Revision: https://phabricator.services.mozilla.com/D145312
---
browser/components/downloads/DownloadsCommon.jsm | 14 +-
.../components/downloads/test/browser/browser.ini | 5 +
.../test/browser/browser_downloads_panel_opens.js | 236 ++++++++++++++++++++-
.../downloads/test/browser/browser_tempfilename.js | 38 +++-
toolkit/components/downloads/DownloadLegacy.jsm | 10 +-
.../pdfjs/test/browser_pdfjs_download_button.js | 43 +---
toolkit/mozapps/downloads/HelperAppDlg.jsm | 3 +-
uriloader/base/nsITransfer.idl | 9 +-
.../exthandler/nsExternalHelperAppService.cpp | 27 ++-
uriloader/exthandler/nsExternalHelperAppService.h | 8 +-
.../exthandler/nsIExternalHelperAppService.idl | 5 +-
.../browser_open_internal_choice_persistence.js | 4 +-
12 files changed, 344 insertions(+), 58 deletions(-)
diff --git a/browser/components/downloads/DownloadsCommon.jsm b/browser/components/downloads/DownloadsCommon.jsm
index 422be505a3156..11e119a0155ea 100644
--- a/browser/components/downloads/DownloadsCommon.jsm
+++ b/browser/components/downloads/DownloadsCommon.jsm
@@ -1003,14 +1003,18 @@ DownloadsDataCtor.prototype = {
browserWin == Services.focus.activeWindow &&
gAlwaysOpenPanel;
+ // For new downloads after the first one, don't show the panel
+ // automatically, but provide a visible notification in the topmost browser
+ // window, if the status indicator is already visible. Also ensure that if
+ // openDownloadsListOnStart = false is passed, we always skip opening the
+ // panel. That's because this will only be passed if the download is started
+ // without user interaction or if a dialog was previously opened in the
+ // process of the download (e.g. unknown content type dialog).
if (
- this.panelHasShownBefore &&
aType != "error" &&
- !shouldOpenDownloadsPanel
+ ((this.panelHasShownBefore && !shouldOpenDownloadsPanel) ||
+ !openDownloadsListOnStart)
) {
- // For new downloads after the first one, don't show the panel
- // automatically, but provide a visible notification in the topmost
- // browser window, if the status indicator is already visible.
DownloadsCommon.log("Showing new download notification.");
browserWin.DownloadsIndicatorView.showEventNotification(aType);
return;
diff --git a/browser/components/downloads/test/browser/browser.ini b/browser/components/downloads/test/browser/browser.ini
index f803f34daa988..6098bd3caa7c6 100644
--- a/browser/components/downloads/test/browser/browser.ini
+++ b/browser/components/downloads/test/browser/browser.ini
@@ -48,6 +48,11 @@ support-files =
[browser_downloads_panel_dontshow.js]
[browser_downloads_panel_height.js]
[browser_downloads_panel_opens.js]
+skip-if =
+ os == "linux" && verify && !debug # For some reason linux opt verify builds time out.
+support-files =
+ foo.txt
+ foo.txt^headers^
[browser_downloads_autohide.js]
[browser_go_to_download_page.js]
[browser_pdfjs_preview.js]
diff --git a/browser/components/downloads/test/browser/browser_downloads_panel_opens.js b/browser/components/downloads/test/browser/browser_downloads_panel_opens.js
index e56b24fd0e3bf..fb1e6f76dabb9 100644
--- a/browser/components/downloads/test/browser/browser_downloads_panel_opens.js
+++ b/browser/components/downloads/test/browser/browser_downloads_panel_opens.js
@@ -1,6 +1,10 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */
+let { MockFilePicker } = SpecialPowers;
+MockFilePicker.init(window);
+registerCleanupFunction(() => MockFilePicker.cleanup());
+
/**
* Check that the downloads panel opens when a download is spoofed.
*/
@@ -85,6 +89,175 @@ function clickCheckbox(checkbox) {
checkbox.parentElement.hidePopup();
}
+/**
+ * Test that the downloads panel correctly opens or doesn't open based on
+ * whether the download triggered a dialog already. If askWhereToSave is true,
+ * we should get a file picker dialog. If preferredAction is alwaysAsk, we
+ * should get an unknown content type dialog. If neither of those is true, we
+ * should get no dialog at all, and expect the downloads panel to open.
+ * @param {boolean} [expectPanelToOpen] true - fail if panel doesn't open
+ * false (default) - fail if it opens
+ * @param {number} [preferredAction] Default download action:
+ * 0 (default) - save download to disk
+ * 1 - open UCT dialog first
+ * @param {boolean} [askWhereToSave] true - open file picker dialog
+ * false (default) - use download dir
+ */
+async function testDownloadsPanelAfterDialog({
+ expectPanelToOpen = false,
+ preferredAction,
+ askWhereToSave = false,
+} = {}) {
+ const { saveToDisk, alwaysAsk } = Ci.nsIHandlerInfo;
+ if (![saveToDisk, alwaysAsk].includes(preferredAction)) {
+ preferredAction = saveToDisk;
+ }
+ const openUCT = preferredAction === alwaysAsk;
+ const TEST_PATH = getRootDirectory(gTestPath).replace(
+ "chrome://mochitests/content",
+ "https://example.com"
+ );
+ const MimeSvc = Cc["@mozilla.org/mime;1"].getService(Ci.nsIMIMEService);
+ const HandlerSvc = Cc["@mozilla.org/uriloader/handler-service;1"].getService(
+ Ci.nsIHandlerService
+ );
+ let publicList = await Downloads.getList(Downloads.PUBLIC);
+
+ for (let download of await publicList.getAll()) {
+ await publicList.remove(download);
+ }
+
+ // We need to test the changes from bug 1739348, where the helper app service
+ // sets a flag based on whether a file picker dialog was opened, and this flag
+ // determines whether the downloads panel will be opened as the download
+ // starts. We need to actually hit "Save" for the download to start, but we
+ // can't interact with the real file picker dialog. So this temporarily
+ // replaces it with a barebones component that plugs into the helper app
+ // service and tells it to start saving the file to the default path.
+ if (askWhereToSave) {
+ MockFilePicker.returnValue = MockFilePicker.returnOK;
+ MockFilePicker.showCallback = function(fp) {
+ // Get the default location from the helper app service.
+ let testFile = MockFilePicker.displayDirectory.clone();
+ testFile.append(fp.defaultString);
+ info("File picker download path: " + testFile.path);
+ MockFilePicker.setFiles([testFile]);
+ MockFilePicker.filterIndex = 0; // kSaveAsType_Complete
+ MockFilePicker.showCallback = null;
+ // Confirm that saving should proceed. The helper app service uses this
+ // value to determine whether to invoke launcher.saveDestinationAvailable
+ return MockFilePicker.returnOK;
+ };
+ }
+
+ await SpecialPowers.pushPrefEnv({
+ set: [
+ ["browser.download.useDownloadDir", !askWhereToSave],
+ ["browser.download.always_ask_before_handling_new_types", openUCT],
+ ["browser.download.improvements_to_download_panel", true],
+ ["security.dialog_enable_delay", 0],
+ ],
+ });
+
+ // Configure the handler for the file according to parameters.
+ let mimeInfo = MimeSvc.getFromTypeAndExtension("text/plain", "txt");
+ let existed = HandlerSvc.exists(mimeInfo);
+ mimeInfo.alwaysAskBeforeHandling = openUCT;
+ mimeInfo.preferredAction = preferredAction;
+ HandlerSvc.store(mimeInfo);
+ registerCleanupFunction(async () => {
+ // Reset the handler to its original state.
+ if (existed) {
+ HandlerSvc.store(mimeInfo);
+ } else {
+ HandlerSvc.remove(mimeInfo);
+ }
+ await publicList.removeFinished();
+ BrowserTestUtils.removeTab(loadingTab);
+ });
+
+ let dialogWindowPromise = BrowserTestUtils.domWindowOpenedAndLoaded();
+ let downloadFinishedPromise = new Promise(resolve => {
+ publicList.addView({
+ onDownloadChanged(download) {
+ info("Download changed!");
+ if (download.succeeded || download.error) {
+ info("Download succeeded or failed.");
+ publicList.removeView(this);
+ resolve(download);
+ }
+ },
+ });
+ });
+ let panelOpenedPromise = expectPanelToOpen ? promisePanelOpened() : null;
+
+ // Open the tab that will trigger the download.
+ let loadingTab = await BrowserTestUtils.openNewForegroundTab({
+ gBrowser,
+ opening: TEST_PATH + "foo.txt",
+ waitForLoad: false,
+ waitForStateStop: true,
+ });
+
+ // Wait for a UCT dialog if the handler was set up to open one.
+ if (openUCT) {
+ let dialogWindow = await dialogWindowPromise;
+ is(
+ dialogWindow.location.href,
+ "chrome://mozapps/content/downloads/unknownContentType.xhtml",
+ "Should have seen the unknown content dialogWindow."
+ );
+ let doc = dialogWindow.document;
+ let dialog = doc.getElementById("unknownContentType");
+ let radio = doc.getElementById("save");
+ let button = dialog.getButton("accept");
+
+ await TestUtils.waitForCondition(
+ () => !button.disabled,
+ "Waiting for the UCT dialog's Accept button to be enabled."
+ );
+ ok(!radio.hidden, "The Save option should be visible");
+ // Make sure we aren't opening the file.
+ radio.click();
+ ok(radio.selected, "The Save option should be selected");
+ button.disabled = false;
+ dialog.acceptDialog();
+ }
+
+ info("Waiting for download to finish.");
+ let download = await downloadFinishedPromise;
+ ok(!download.error, "There should be no error.");
+ is(
+ DownloadsPanel.isPanelShowing,
+ expectPanelToOpen,
+ `Panel should${expectPanelToOpen ? " " : " not "}be showing.`
+ );
+ if (DownloadsPanel.isPanelShowing) {
+ await panelOpenedPromise;
+ let hiddenPromise = BrowserTestUtils.waitForPopupEvent(
+ DownloadsPanel.panel,
+ "hidden"
+ );
+ DownloadsPanel.hidePanel();
+ await hiddenPromise;
+ }
+ if (download?.target.exists) {
+ try {
+ info("Removing test file: " + download.target.path);
+ if (Services.appinfo.OS === "WINNT") {
+ await IOUtils.setPermissions(download.target.path, 0o600);
+ }
+ await IOUtils.remove(download.target.path);
+ } catch (ex) {
+ /* ignore */
+ }
+ }
+ for (let dl of await publicList.getAll()) {
+ await publicList.remove(dl);
+ }
+ BrowserTestUtils.removeTab(loadingTab);
+}
+
/**
* Make sure the downloads panel opens automatically with a new download.
*/
@@ -93,6 +266,7 @@ add_task(async function test_downloads_panel_opens() {
set: [
["browser.download.improvements_to_download_panel", true],
["browser.download.always_ask_before_handling_new_types", false],
+ ["browser.download.alwaysOpenPanel", true],
],
});
await checkPanelOpens();
@@ -103,6 +277,7 @@ add_task(async function test_customizemode_doesnt_wreck_things() {
set: [
["browser.download.improvements_to_download_panel", true],
["browser.download.always_ask_before_handling_new_types", false],
+ ["browser.download.alwaysOpenPanel", true],
],
});
@@ -114,7 +289,7 @@ add_task(async function test_customizemode_doesnt_wreck_things() {
gCustomizeMode.enter();
await customizationReadyPromise;
- info("try to open the panel (will not work, in customize mode)");
+ info("Try to open the panel (will not work, in customize mode)");
let promise = promisePanelOpened();
DownloadsCommon.getData(window)._notifyDownloadEvent("start");
await TestUtils.waitForCondition(
@@ -134,10 +309,16 @@ add_task(async function test_customizemode_doesnt_wreck_things() {
gCustomizeMode.exit();
await afterCustomizationPromise;
+ // Avoid a failure on Linux where the window isn't active for some reason,
+ // which prevents the window's downloads panel from opening.
+ if (Services.focus.activeWindow != window) {
+ info("Main window is not active, trying to focus.");
+ await SimpleTest.promiseFocus(window);
+ is(Services.focus.activeWindow, window, "Main window should be active.");
+ }
DownloadsCommon.getData(window)._notifyDownloadEvent("start");
- is(
- DownloadsPanel.isPanelShowing,
- true,
+ await TestUtils.waitForCondition(
+ () => DownloadsPanel.isPanelShowing,
"Panel state should indicate a preparation to be opened"
);
await promise;
@@ -456,3 +637,50 @@ add_task(async function test_alwaysOpenPanel_menuitem() {
await checkPanelOpens();
});
+
+/**
+ * Verify that the downloads panel opens if the download did not open a file
+ * picker or UCT dialog
+ */
+add_task(async function test_downloads_panel_after_no_dialogs() {
+ await testDownloadsPanelAfterDialog({ expectPanelToOpen: true });
+ ok(true, "Downloads panel opened because no dialogs were opened.");
+});
+
+/**
+ * Verify that the downloads panel doesn't open if the download opened an
+ * unknown content type dialog (e.g. action = always ask)
+ */
+add_task(async function test_downloads_panel_after_UCT_dialog() {
+ await testDownloadsPanelAfterDialog({
+ expectPanelToOpen: false,
+ preferredAction: Ci.nsIHandlerInfo.alwaysAsk,
+ });
+ ok(true, "Downloads panel suppressed after UCT dialog.");
+});
+
+/**
+ * Verify that the downloads panel doesn't open if the download opened a file
+ * picker dialog (e.g. useDownloadDir = false)
+ */
+add_task(async function test_downloads_panel_after_file_picker_dialog() {
+ await testDownloadsPanelAfterDialog({
+ expectPanelToOpen: false,
+ preferredAction: Ci.nsIHandlerInfo.saveToDisk,
+ askWhereToSave: true,
+ });
+ ok(true, "Downloads panel suppressed after file picker dialog.");
+});
+
+/**
+ * Verify that the downloads panel doesn't open if the download opened both
+ * dialogs (e.g. default action = always ask AND useDownloadDir = false)
+ */
+add_task(async function test_downloads_panel_after_both_dialogs() {
+ await testDownloadsPanelAfterDialog({
+ expectPanelToOpen: false,
+ preferredAction: Ci.nsIHandlerInfo.alwaysAsk,
+ askWhereToSave: true,
+ });
+ ok(true, "Downloads panel suppressed after UCT and file picker dialogs.");
+});
diff --git a/browser/components/downloads/test/browser/browser_tempfilename.js b/browser/components/downloads/test/browser/browser_tempfilename.js
index b995f078895f1..dcef4016a6fbe 100644
--- a/browser/components/downloads/test/browser/browser_tempfilename.js
+++ b/browser/components/downloads/test/browser/browser_tempfilename.js
@@ -16,6 +16,27 @@ add_task(async function test_tempfilename() {
};
list.addView(view);
});
+
+ await SpecialPowers.pushPrefEnv({
+ set: [
+ ["browser.download.improvements_to_download_panel", true],
+ ["browser.download.always_ask_before_handling_new_types", false],
+ ],
+ });
+
+ const MimeSvc = Cc["@mozilla.org/mime;1"].getService(Ci.nsIMIMEService);
+ const HandlerSvc = Cc["@mozilla.org/uriloader/handler-service;1"].getService(
+ Ci.nsIHandlerService
+ );
+ let mimeInfo = MimeSvc.getFromTypeAndExtension(
+ HandlerSvc.getTypeFromExtension("txt"),
+ "txt"
+ );
+ let existed = HandlerSvc.exists(mimeInfo);
+ mimeInfo.alwaysAskBeforeHandling = false;
+ mimeInfo.preferredAction = Ci.nsIHandlerInfo.saveToDisk;
+ HandlerSvc.store(mimeInfo);
+
serveInterruptibleAsDownload();
mustInterruptResponses();
await BrowserTestUtils.withNewTab(
@@ -27,7 +48,21 @@ add_task(async function test_tempfilename() {
},
async () => {
let download = await downloadStarted;
- registerCleanupFunction(() => download.finalize());
+ registerCleanupFunction(async () => {
+ if (existed) {
+ HandlerSvc.store(mimeInfo);
+ } else {
+ HandlerSvc.remove(mimeInfo);
+ }
+ await download.finalize(true);
+ if (Services.appinfo.OS === "WINNT") {
+ // We need to make the file writable to delete it on Windows.
+ await IOUtils.setPermissions(download.target.path, 0o600);
+ }
+ await IOUtils.remove(download.target.path);
+ await download.finalize();
+ await list.removeFinished();
+ });
let { partFilePath } = download.target;
Assert.stringContains(
@@ -51,7 +86,6 @@ add_task(async function test_tempfilename() {
!(await IOUtils.exists(download.target.partFilePath)),
"Temp file should be gone."
);
- await IOUtils.remove(download.target.path);
}
);
});
diff --git a/toolkit/components/downloads/DownloadLegacy.jsm b/toolkit/components/downloads/DownloadLegacy.jsm
index 14b9b8b8780fd..7ff95b53190c6 100644
--- a/toolkit/components/downloads/DownloadLegacy.jsm
+++ b/toolkit/components/downloads/DownloadLegacy.jsm
@@ -274,7 +274,8 @@ DownloadLegacyTransfer.prototype = {
aCancelable,
aIsPrivate,
aDownloadClassification,
- aReferrerInfo
+ aReferrerInfo,
+ aOpenDownloadsListOnStart
) {
return this._nsITransferInitInternal(
aSource,
@@ -287,7 +288,8 @@ DownloadLegacyTransfer.prototype = {
aCancelable,
aIsPrivate,
aDownloadClassification,
- aReferrerInfo
+ aReferrerInfo,
+ aOpenDownloadsListOnStart
);
},
@@ -303,6 +305,7 @@ DownloadLegacyTransfer.prototype = {
aIsPrivate,
aDownloadClassification,
aReferrerInfo,
+ aOpenDownloadsListOnStart,
aBrowsingContext,
aHandleInternally,
aHttpChannel
@@ -327,6 +330,7 @@ DownloadLegacyTransfer.prototype = {
aIsPrivate,
aDownloadClassification,
aReferrerInfo,
+ aOpenDownloadsListOnStart,
userContextId,
browsingContextId,
aHandleInternally,
@@ -346,6 +350,7 @@ DownloadLegacyTransfer.prototype = {
isPrivate,
aDownloadClassification,
referrerInfo,
+ openDownloadsListOnStart = true,
userContextId = 0,
browsingContextId = 0,
handleInternally = false,
@@ -397,6 +402,7 @@ DownloadLegacyTransfer.prototype = {
contentType,
launcherPath,
handleInternally,
+ openDownloadsListOnStart,
};
// In case the Download was classified as insecure/dangerous
diff --git a/toolkit/components/pdfjs/test/browser_pdfjs_download_button.js b/toolkit/components/pdfjs/test/browser_pdfjs_download_button.js
index b0040a71941f6..fa6b6d6c978a9 100644
--- a/toolkit/components/pdfjs/test/browser_pdfjs_download_button.js
+++ b/toolkit/components/pdfjs/test/browser_pdfjs_download_button.js
@@ -52,22 +52,6 @@ add_setup(async function() {
});
});
-async function closeDownloadsPanel() {
- if (DownloadsPanel.panel.state !== "closed") {
- let hiddenPromise = BrowserTestUtils.waitForEvent(
- DownloadsPanel.panel,
- "popuphidden"
- );
- DownloadsPanel.hidePanel();
- await hiddenPromise;
- }
- is(
- DownloadsPanel.panel.state,
- "closed",
- "Check that the download panel is closed"
- );
-}
-
/**
* Check clicking the download button saves the file and doesn't open a new viewer
*/
@@ -75,7 +59,10 @@ add_task(async function test_downloading_pdf_nonprivate_window() {
const pdfUrl = TESTROOT + "file_pdfjs_test.pdf";
await SpecialPowers.pushPrefEnv({
- set: [["browser.download.improvements_to_download_panel", true]],
+ set: [
+ ["browser.download.improvements_to_download_panel", true],
+ ["browser.download.always_ask_before_handling_new_types", false],
+ ],
});
await BrowserTestUtils.withNewTab(
@@ -87,7 +74,6 @@ add_task(async function test_downloading_pdf_nonprivate_window() {
info(`${tabCount} tabs are open at the start of the test`);
let downloadList = await Downloads.getList(Downloads.PUBLIC);
- const initialDownloadCount = (await downloadList.getAll()).length;
let filePickerShown = createPromiseForFilePicker();
@@ -100,23 +86,16 @@ add_task(async function test_downloading_pdf_nonprivate_window() {
info("Waiting for a filename to be picked from the file picker");
await filePickerShown;
- // check that resulted in a download being added to the list. The
- // download panel should not open.
await downloadFinishedPromise;
- is(
- DownloadsPanel.panel.state,
- "closed",
- "Check the download panel state is 'closed'"
- );
- downloadList = await Downloads.getList(Downloads.PUBLIC);
- const allDownloads = await downloadList.getAll();
- let currentDownloadCount = allDownloads.length;
- is(
- currentDownloadCount,
- initialDownloadCount + 1,
- "A download was added when we clicked download"
+ ok(true, "A download was added when we clicked download");
+
+ // See bug 1739348 - don't show panel for downloads that opened dialogs
+ ok(
+ !DownloadsPanel.isPanelShowing,
+ "The download panel did not open, since the file picker was shown already"
);
+ const allDownloads = await downloadList.getAll();
const dl = allDownloads.find(dl => dl.source.originalUrl === pdfUrl);
ok(!!dl, "The pdf download has the correct url in source.originalUrl");
diff --git a/toolkit/mozapps/downloads/HelperAppDlg.jsm b/toolkit/mozapps/downloads/HelperAppDlg.jsm
index 325de51c0c5fe..0a5a4a460bf38 100644
--- a/toolkit/mozapps/downloads/HelperAppDlg.jsm
+++ b/toolkit/mozapps/downloads/HelperAppDlg.jsm
@@ -409,7 +409,8 @@ nsUnknownContentTypeDialog.prototype = {
}
}
}
- aLauncher.saveDestinationAvailable(result);
+ // Don't pop up the downloads panel redundantly.
+ aLauncher.saveDestinationAvailable(result, true);
});
});
})().catch(Cu.reportError);
diff --git a/uriloader/base/nsITransfer.idl b/uriloader/base/nsITransfer.idl
index ba701c982d7a0..828e4c4a0f7bc 100644
--- a/uriloader/base/nsITransfer.idl
+++ b/uriloader/base/nsITransfer.idl
@@ -62,7 +62,12 @@ interface nsITransfer : nsIWebProgressListener2 {
*
* @param aDownloadClassification Indicates wheter the download is unwanted,
* should be considered dangerous or insecure.
+ *
* @param aReferrerInfo The Referrer this download is started with
+ *
+ * @param aOpenDownloadsListOnStart true (default) - Open downloads panel.
+ * false - Only show an icon indicator.
+ * This parameter is optional.
*/
void init(in nsIURI aSource,
in nsIURI aSourceOriginalURI,
@@ -74,7 +79,8 @@ interface nsITransfer : nsIWebProgressListener2 {
in nsICancelable aCancelable,
in boolean aIsPrivate,
in long aDownloadClassification,
- in nsIReferrerInfo aReferrerInfo);
+ in nsIReferrerInfo aReferrerInfo,
+ [optional] in boolean aOpenDownloadsListOnStart);
/**
* Same as init, but allows for passing the browsingContext
@@ -97,6 +103,7 @@ interface nsITransfer : nsIWebProgressListener2 {
in boolean aIsPrivate,
in long aDownloadClassification,
in nsIReferrerInfo aReferrerInfo,
+ [optional] in boolean aOpenDownloadsListOnStart,
in BrowsingContext aBrowsingContext,
in boolean aHandleInternally,
in nsIHttpChannel aHttpChannel);
diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp
index adb05b3c46b85..f5b0e2e5bc95c 100644
--- a/uriloader/exthandler/nsExternalHelperAppService.cpp
+++ b/uriloader/exthandler/nsExternalHelperAppService.cpp
@@ -1280,6 +1280,7 @@ nsExternalAppHandler::nsExternalAppHandler(
mIsFileChannel(false),
mShouldCloseWindow(false),
mHandleInternally(false),
+ mDialogShowing(false),
mReason(aReason),
mTempFileIsExecutable(false),
mTimeDownloadStarted(0),
@@ -1855,6 +1856,9 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
// this will create a reference cycle (the dialog holds a reference to us as
// nsIHelperAppLauncher), which will be broken in Cancel or CreateTransfer.
nsCOMPtr<nsIInterfaceRequestor> dialogParent = GetDialogParent();
+ // Don't pop up the downloads panel since we're already going to pop up the
+ // UCT dialog for basically the same effect.
+ mDialogShowing = true;
rv = mDialog->Show(this, dialogParent, mReason);
// what do we do if the dialog failed? I guess we should call Cancel and
@@ -2351,14 +2355,15 @@ nsresult nsExternalAppHandler::CreateTransfer() {
rv = transfer->InitWithBrowsingContext(
mSourceUrl, target, u""_ns, mMimeInfo, mTimeDownloadStarted, mTempFile,
this, channel && NS_UsePrivateBrowsing(channel),
- mDownloadClassification, referrerInfo, mBrowsingContext,
- mHandleInternally, nullptr);
+ mDownloadClassification, referrerInfo, !mDialogShowing,
+ mBrowsingContext, mHandleInternally, nullptr);
} else {
rv = transfer->Init(mSourceUrl, nullptr, target, u""_ns, mMimeInfo,
mTimeDownloadStarted, mTempFile, this,
channel && NS_UsePrivateBrowsing(channel),
- mDownloadClassification, referrerInfo);
+ mDownloadClassification, referrerInfo, !mDialogShowing);
}
+ mDialogShowing = false;
NS_ENSURE_SUCCESS(rv, rv);
@@ -2443,13 +2448,13 @@ nsresult nsExternalAppHandler::CreateFailedTransfer() {
rv = transfer->InitWithBrowsingContext(
mSourceUrl, pseudoTarget, u""_ns, mMimeInfo, mTimeDownloadStarted,
mTempFile, this, channel && NS_UsePrivateBrowsing(channel),
- mDownloadClassification, referrerInfo, mBrowsingContext,
+ mDownloadClassification, referrerInfo, true, mBrowsingContext,
mHandleInternally, httpChannel);
} else {
rv = transfer->Init(mSourceUrl, nullptr, pseudoTarget, u""_ns, mMimeInfo,
mTimeDownloadStarted, mTempFile, this,
channel && NS_UsePrivateBrowsing(channel),
- mDownloadClassification, referrerInfo);
+ mDownloadClassification, referrerInfo, true);
}
NS_ENSURE_SUCCESS(rv, rv);
@@ -2459,11 +2464,16 @@ nsresult nsExternalAppHandler::CreateFailedTransfer() {
return NS_OK;
}
-nsresult nsExternalAppHandler::SaveDestinationAvailable(nsIFile* aFile) {
- if (aFile)
+nsresult nsExternalAppHandler::SaveDestinationAvailable(nsIFile* aFile,
+ bool aDialogWasShown) {
+ if (aFile) {
+ if (aDialogWasShown) {
+ mDialogShowing = true;
+ }
ContinueSave(aFile);
- else
+ } else {
Cancel(NS_BINDING_ABORTED);
+ }
return NS_OK;
}
@@ -2735,6 +2745,7 @@ NS_IMETHODIMP nsExternalAppHandler::Cancel(nsresult aReason) {
// Break our reference cycle with the helper app dialog (set up in
// OnStartRequest)
mDialog = nullptr;
+ mDialogShowing = false;
mRequest = nullptr;
diff --git a/uriloader/exthandler/nsExternalHelperAppService.h b/uriloader/exthandler/nsExternalHelperAppService.h
index ce142a4f5b6b0..439be1645f4fc 100644
--- a/uriloader/exthandler/nsExternalHelperAppService.h
+++ b/uriloader/exthandler/nsExternalHelperAppService.h
@@ -376,6 +376,12 @@ class nsExternalAppHandler final : public nsIStreamListener,
*/
bool mHandleInternally;
+ /**
+ * True if any dialog (e.g. unknown content type or file picker) is shown —
+ * can stop downloads panel from opening, to avoid redundant interruptions.
+ */
+ bool mDialogShowing;
+
/**
* One of the REASON_ constants from nsIHelperAppLauncherDialog. Indicates the
* reason the dialog was shown (unknown content type, server requested it,
@@ -522,7 +528,7 @@ class nsExternalAppHandler final : public nsIStreamListener,
const nsString& path);
/**
- * Set in nsHelperDlgApp.js. This is always null after the user has chosen an
+ * Set in HelperAppDlg.jsm. This is always null after the user has chosen an
* action.
*/
nsCOMPtr<nsIWebProgressListener2> mDialogProgressListener;
diff --git a/uriloader/exthandler/nsIExternalHelperAppService.idl b/uriloader/exthandler/nsIExternalHelperAppService.idl
index 3554c69aaced1..307e6196a89df 100644
--- a/uriloader/exthandler/nsIExternalHelperAppService.idl
+++ b/uriloader/exthandler/nsIExternalHelperAppService.idl
@@ -150,8 +150,11 @@ interface nsIHelperAppLauncher : nsICancelable
* Callback invoked by nsIHelperAppLauncherDialog::promptForSaveToFileAsync
* after the user has chosen a file through the File Picker (or dismissed it).
* @param aFile The file that was chosen by the user (or null if dialog was dismissed).
+ * @param aDialogWasShown Optional boolean - false by default. Pass true if a
+ * dialog was opened in the process of reaching this file result. If true, we
+ * suppress the opening of the downloads panel to avoid redundancy.
*/
- void saveDestinationAvailable(in nsIFile aFile);
+ void saveDestinationAvailable(in nsIFile aFile, [optional] in boolean aDialogWasShown);
/**
* The following methods are used by the progress dialog to get or set
diff --git a/uriloader/exthandler/tests/mochitest/browser_open_internal_choice_persistence.js b/uriloader/exthandler/tests/mochitest/browser_open_internal_choice_persistence.js
index 11f9e24092053..e6dffa8df4ab4 100644
--- a/uriloader/exthandler/tests/mochitest/browser_open_internal_choice_persistence.js
+++ b/uriloader/exthandler/tests/mochitest/browser_open_internal_choice_persistence.js
@@ -33,11 +33,13 @@ function waitForAcceptButtonToGetEnabled(doc) {
}
add_setup(async function() {
- // Remove the security delay for the dialog during the test.
await SpecialPowers.pushPrefEnv({
set: [
+ // Remove the security delay for the dialog during the test.
["security.dialog_enable_delay", 0],
["browser.helperApps.showOpenOptionForViewableInternally", true],
+ // Make sure we don't open a file picker dialog somehow.
+ ["browser.download.useDownloadDir", true],
],
});
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the tor-commits
mailing list