From a49aa34d6dfcaa9052ecd365c956401fad74dac5 Mon Sep 17 00:00:00 2001 From: Neil Deakin Date: Mon, 6 Jun 2022 22:23:58 +0000 Subject: [PATCH] Bug 1761265, don't show the downloads panel when a download was started by user action that they expect will save the file, r=mhowell,necko-reviewers,kershaw The download panel should still appear when clicking on download links or those with content-disposition: attachment Differential Revision: https://phabricator.services.mozilla.com/D147875 --- .../downloads/test/browser/browser.ini | 1 + .../browser_downloads_panel_dontshow.js | 126 ++++++++++++++++++ dom/webbrowserpersist/nsWebBrowserPersist.cpp | 3 + netwerk/base/LoadInfo.cpp | 14 ++ netwerk/base/LoadInfo.h | 1 + netwerk/base/TRRLoadInfo.cpp | 10 ++ netwerk/base/nsILoadInfo.idl | 6 + toolkit/components/downloads/DownloadCore.jsm | 9 ++ .../test/browser_pdfjs_download_button.js | 31 +++-- 9 files changed, 190 insertions(+), 11 deletions(-) create mode 100644 browser/components/downloads/test/browser/browser_downloads_panel_dontshow.js diff --git a/browser/components/downloads/test/browser/browser.ini b/browser/components/downloads/test/browser/browser.ini index 35d3d9a486f8..f803f34daa98 100644 --- a/browser/components/downloads/test/browser/browser.ini +++ b/browser/components/downloads/test/browser/browser.ini @@ -45,6 +45,7 @@ skip-if = support-files = foo.txt foo.txt^headers^ +[browser_downloads_panel_dontshow.js] [browser_downloads_panel_height.js] [browser_downloads_panel_opens.js] [browser_downloads_autohide.js] diff --git a/browser/components/downloads/test/browser/browser_downloads_panel_dontshow.js b/browser/components/downloads/test/browser/browser_downloads_panel_dontshow.js new file mode 100644 index 000000000000..b6de75872390 --- /dev/null +++ b/browser/components/downloads/test/browser/browser_downloads_panel_dontshow.js @@ -0,0 +1,126 @@ +// This test verifies that the download panel opens when a +// download occurs but not when a user manually saves a page. + +let MockFilePicker = SpecialPowers.MockFilePicker; +MockFilePicker.init(window); + +async function promiseDownloadFinished(list) { + return new Promise(resolve => { + list.addView({ + onDownloadChanged(download) { + download.launchWhenSucceeded = false; + if (download.succeeded || download.error) { + list.removeView(this); + resolve(download); + } + }, + }); + }); +} + +function openTestPage() { + return BrowserTestUtils.openNewForegroundTab( + gBrowser, + `https://www.example.com/document-builder.sjs?html= + + Link1 + Link2 + + ` + ); +} + +add_task(async function download_saveas_file() { + let tab = await openTestPage(); + + for (let testname of ["save link", "save page"]) { + if (testname == "save link") { + let menu = document.getElementById("contentAreaContextMenu"); + let popupShown = BrowserTestUtils.waitForEvent(menu, "popupshown"); + BrowserTestUtils.synthesizeMouse( + "#normallink", + 5, + 5, + { type: "contextmenu", button: 2 }, + gBrowser.selectedBrowser + ); + await popupShown; + } + + let list = await Downloads.getList(Downloads.PUBLIC); + let downloadFinishedPromise = promiseDownloadFinished(list); + + let saveFile = Services.dirsvc.get("TmpD", Ci.nsIFile); + saveFile.append("testsavedir"); + if (!saveFile.exists()) { + saveFile.create(Ci.nsIFile.DIRECTORY_TYPE, 0o755); + } + + await new Promise(resolve => { + MockFilePicker.showCallback = function(fp) { + saveFile.append("sample"); + MockFilePicker.setFiles([saveFile]); + setTimeout(() => { + resolve(fp.defaultString); + }, 0); + return Ci.nsIFilePicker.returnOK; + }; + + if (testname == "save link") { + let menu = document.getElementById("contentAreaContextMenu"); + let menuitem = document.getElementById("context-savelink"); + menu.activateItem(menuitem); + } else if (testname == "save page") { + document.getElementById("Browser:SavePage").doCommand(); + } + }); + + await downloadFinishedPromise; + is( + DownloadsPanel.panel.state, + "closed", + "downloads panel closed after download link after " + testname + ); + } + + await task_resetState(); + + MockFilePicker.cleanup(); + BrowserTestUtils.removeTab(tab); +}); + +add_task(async function download_link() { + let tab = await openTestPage(); + + let list = await Downloads.getList(Downloads.PUBLIC); + let downloadFinishedPromise = promiseDownloadFinished(list); + + let panelOpenedPromise = promisePanelOpened(); + + BrowserTestUtils.synthesizeMouse( + "#downloadlink", + 5, + 5, + {}, + gBrowser.selectedBrowser + ); + + let download = await downloadFinishedPromise; + await panelOpenedPromise; + + is( + DownloadsPanel.panel.state, + "open", + "downloads panel open after download link clicked" + ); + + DownloadsPanel.hidePanel(); + + await task_resetState(); + + BrowserTestUtils.removeTab(tab); + + try { + await IOUtils.remove(download.target.path); + } catch (ex) {} +}); diff --git a/dom/webbrowserpersist/nsWebBrowserPersist.cpp b/dom/webbrowserpersist/nsWebBrowserPersist.cpp index f2c2ebf58840..7f304a77e848 100644 --- a/dom/webbrowserpersist/nsWebBrowserPersist.cpp +++ b/dom/webbrowserpersist/nsWebBrowserPersist.cpp @@ -1377,6 +1377,9 @@ nsresult nsWebBrowserPersist::SaveURIInternal( } } + nsCOMPtr loadInfo = inputChannel->LoadInfo(); + loadInfo->SetIsUserTriggeredSave(true); + // Set the referrer, post data and headers if any nsCOMPtr httpChannel(do_QueryInterface(inputChannel)); if (httpChannel) { diff --git a/netwerk/base/LoadInfo.cpp b/netwerk/base/LoadInfo.cpp index c70ae05224c3..5f5f9d051476 100644 --- a/netwerk/base/LoadInfo.cpp +++ b/netwerk/base/LoadInfo.cpp @@ -1800,6 +1800,20 @@ LoadInfo::SetAllowDeprecatedSystemRequests( return NS_OK; } +NS_IMETHODIMP +LoadInfo::GetIsUserTriggeredSave(bool* aIsUserTriggeredSave) { + *aIsUserTriggeredSave = + mIsUserTriggeredSave || + mInternalContentPolicyType == nsIContentPolicy::TYPE_SAVEAS_DOWNLOAD; + return NS_OK; +} + +NS_IMETHODIMP +LoadInfo::SetIsUserTriggeredSave(bool aIsUserTriggeredSave) { + mIsUserTriggeredSave = aIsUserTriggeredSave; + return NS_OK; +} + NS_IMETHODIMP LoadInfo::GetIsInDevToolsContext(bool* aIsInDevToolsContext) { *aIsInDevToolsContext = mIsInDevToolsContext; diff --git a/netwerk/base/LoadInfo.h b/netwerk/base/LoadInfo.h index 6452948b199c..a1624592d20d 100644 --- a/netwerk/base/LoadInfo.h +++ b/netwerk/base/LoadInfo.h @@ -330,6 +330,7 @@ class LoadInfo final : public nsILoadInfo { uint32_t mHttpsOnlyStatus = nsILoadInfo::HTTPS_ONLY_UNINITIALIZED; bool mHasValidUserGestureActivation = false; bool mAllowDeprecatedSystemRequests = false; + bool mIsUserTriggeredSave = false; bool mIsInDevToolsContext = false; bool mParserCreatedScript = false; nsILoadInfo::StoragePermissionState mStoragePermission = diff --git a/netwerk/base/TRRLoadInfo.cpp b/netwerk/base/TRRLoadInfo.cpp index 7ef044cb2517..05a8878a3f26 100644 --- a/netwerk/base/TRRLoadInfo.cpp +++ b/netwerk/base/TRRLoadInfo.cpp @@ -698,6 +698,16 @@ TRRLoadInfo::SetAllowDeprecatedSystemRequests( return NS_ERROR_NOT_IMPLEMENTED; } +NS_IMETHODIMP +TRRLoadInfo::GetIsUserTriggeredSave(bool* aIsUserTriggeredSave) { + return NS_ERROR_NOT_IMPLEMENTED; +} + +NS_IMETHODIMP +TRRLoadInfo::SetIsUserTriggeredSave(bool aIsUserTriggeredSave) { + return NS_ERROR_NOT_IMPLEMENTED; +} + NS_IMETHODIMP TRRLoadInfo::GetIsInDevToolsContext(bool* aIsInDevToolsContext) { return NS_ERROR_NOT_IMPLEMENTED; diff --git a/netwerk/base/nsILoadInfo.idl b/netwerk/base/nsILoadInfo.idl index 0a7d0c66fb3e..85ba87970298 100644 --- a/netwerk/base/nsILoadInfo.idl +++ b/netwerk/base/nsILoadInfo.idl @@ -510,6 +510,12 @@ interface nsILoadInfo : nsISupports */ [infallible] attribute boolean parserCreatedScript; + /** + * True if this request is known to have been triggered by a user + * manually requesting the URI to be saved. + */ + [infallible] attribute boolean isUserTriggeredSave; + /** * True if this request is from DevTools. */ diff --git a/toolkit/components/downloads/DownloadCore.jsm b/toolkit/components/downloads/DownloadCore.jsm index e4af8bceeea5..36275c9d22ca 100644 --- a/toolkit/components/downloads/DownloadCore.jsm +++ b/toolkit/components/downloads/DownloadCore.jsm @@ -2867,6 +2867,15 @@ DownloadLegacySaver.prototype = { this.download.source.referrerInfo = aRequest.referrerInfo; } + // Don't open the download panel when the user initiated to save a + // link or document. + if ( + aRequest instanceof Ci.nsIChannel && + aRequest.loadInfo.isUserTriggeredSave + ) { + this.download.openDownloadsListOnStart = false; + } + this.addToHistory(); }, diff --git a/toolkit/components/pdfjs/test/browser_pdfjs_download_button.js b/toolkit/components/pdfjs/test/browser_pdfjs_download_button.js index 5f21b241a223..b0040a71941f 100644 --- a/toolkit/components/pdfjs/test/browser_pdfjs_download_button.js +++ b/toolkit/components/pdfjs/test/browser_pdfjs_download_button.js @@ -12,6 +12,20 @@ MockFilePicker.returnValue = MockFilePicker.returnOK; var tempDir; +async function promiseDownloadFinished(list) { + return new Promise(resolve => { + list.addView({ + onDownloadChanged(download) { + download.launchWhenSucceeded = false; + if (download.succeeded || download.error) { + list.removeView(this); + resolve(download); + } + }, + }); + }); +} + function createPromiseForFilePicker() { return new Promise(resolve => { MockFilePicker.showCallback = fp => { @@ -77,10 +91,7 @@ add_task(async function test_downloading_pdf_nonprivate_window() { let filePickerShown = createPromiseForFilePicker(); - let downloadsPanelPromise = BrowserTestUtils.waitForEvent( - DownloadsPanel.panel, - "popupshown" - ); + let downloadFinishedPromise = promiseDownloadFinished(downloadList); info("Clicking on the download button..."); await SpecialPowers.spawn(browser, [], () => { @@ -89,14 +100,13 @@ 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 - // and the dl panel opened - info("Waiting for download panel to open when the download is complete"); - await downloadsPanelPromise; + // check that resulted in a download being added to the list. The + // download panel should not open. + await downloadFinishedPromise; is( DownloadsPanel.panel.state, - "open", - "Check the download panel state is 'open'" + "closed", + "Check the download panel state is 'closed'" ); downloadList = await Downloads.getList(Downloads.PUBLIC); const allDownloads = await downloadList.getAll(); @@ -120,7 +130,6 @@ add_task(async function test_downloading_pdf_nonprivate_window() { tabCount, "No new tab was opened to view the downloaded PDF" ); - await closeDownloadsPanel(); } ); });