Bug 1739348 - Don't open downloads panel after download dialogs. r=NeilDeakin

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

@@ -1008,14 +1008,18 @@ DownloadsDataCtor.prototype = {
       browserWin == Services.focus.activeWindow &&
       lazy.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;

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]

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.");
+});

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);
     }
   );
 });

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

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");
 

toolkit/mozapps/downloads/HelperAppDlg.jsm

@@ -408,7 +408,8 @@ nsUnknownContentTypeDialog.prototype = {
               }
             }
           }
-          aLauncher.saveDestinationAvailable(result);
+          // Don't pop up the downloads panel redundantly.
+          aLauncher.saveDestinationAvailable(result, true);
         });
       });
     })().catch(Cu.reportError);

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);

uriloader/exthandler/nsExternalHelperAppService.cpp

@@ -1285,6 +1285,7 @@ nsExternalAppHandler::nsExternalAppHandler(
       mIsFileChannel(false),
       mShouldCloseWindow(false),
       mHandleInternally(false),
+      mDialogShowing(false),
       mReason(aReason),
       mTempFileIsExecutable(false),
       mTimeDownloadStarted(0),
@@ -1859,6 +1860,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
@@ -2353,14 +2357,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);
 
@@ -2445,13 +2450,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);
 
@@ -2461,11 +2466,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;
 }
@@ -2737,6 +2747,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;
 

uriloader/exthandler/nsExternalHelperAppService.h

@@ -373,6 +373,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,
@@ -519,7 +525,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;

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

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],
     ],
   });