From 2816a6e799e31d7a49ad578e89caa44a06884175 Mon Sep 17 00:00:00 2001 From: Sam Foster Date: Mon, 29 Nov 2021 20:07:57 +0000 Subject: [PATCH] Bug 1740135 - Treat download button like save action to avoid opening on completion. r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D131945 --- .../pdfjs/content/PdfStreamConverter.jsm | 12 +- toolkit/components/pdfjs/test/browser.ini | 1 + .../test/browser_pdfjs_download_button.js | 117 ++++++++++++++++++ .../pdfjs/test/browser_pdfjs_saveas.js | 9 -- toolkit/components/pdfjs/test/head.js | 29 +++++ 5 files changed, 157 insertions(+), 11 deletions(-) create mode 100644 toolkit/components/pdfjs/test/browser_pdfjs_download_button.js diff --git a/toolkit/components/pdfjs/content/PdfStreamConverter.jsm b/toolkit/components/pdfjs/content/PdfStreamConverter.jsm index 82424b86471d..36ddcee3e131 100644 --- a/toolkit/components/pdfjs/content/PdfStreamConverter.jsm +++ b/toolkit/components/pdfjs/content/PdfStreamConverter.jsm @@ -356,8 +356,16 @@ class ChromeActions { var blobUri = NetUtil.newURI(blobUrl); // If the download was triggered from the ctrl/cmd+s or "Save Page As" - // launch the "Save As" dialog. - if (data.sourceEventType == "save") { + // or the download button, launch the "Save As" dialog. + const saveOnDownload = getBoolPref( + "browser.download.improvements_to_download_panel", + false + ); + + if ( + data.sourceEventType == "save" || + (saveOnDownload && data.sourceEventType == "download") + ) { let actor = getActor(this.domWindow); actor.sendAsyncMessage("PDFJS:Parent:saveURL", { blobUrl, diff --git a/toolkit/components/pdfjs/test/browser.ini b/toolkit/components/pdfjs/test/browser.ini index 2bb4c77fac81..db31a7b8460b 100644 --- a/toolkit/components/pdfjs/test/browser.ini +++ b/toolkit/components/pdfjs/test/browser.ini @@ -3,6 +3,7 @@ support-files = file_pdfjs_test.pdf head.js +[browser_pdfjs_download_button.js] [browser_pdfjs_fill_login.js] support-files = file_pdfjs_form.pdf diff --git a/toolkit/components/pdfjs/test/browser_pdfjs_download_button.js b/toolkit/components/pdfjs/test/browser_pdfjs_download_button.js new file mode 100644 index 000000000000..9711209bd88c --- /dev/null +++ b/toolkit/components/pdfjs/test/browser_pdfjs_download_button.js @@ -0,0 +1,117 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +const RELATIVE_DIR = "toolkit/components/pdfjs/test/"; +const TESTROOT = "http://example.com/browser/" + RELATIVE_DIR; + +var MockFilePicker = SpecialPowers.MockFilePicker; +MockFilePicker.init(window); +MockFilePicker.returnValue = MockFilePicker.returnOK; + +var tempDir; + +function createPromiseForFilePicker() { + return new Promise(resolve => { + MockFilePicker.showCallback = fp => { + let destFile = tempDir.clone(); + destFile.append(fp.defaultString); + if (destFile.exists()) { + destFile.remove(false); + } + MockFilePicker.setFiles([destFile]); + MockFilePicker.filterIndex = 0; // kSaveAsType_Complete + resolve(); + }; + }); +} + +add_task(async function setup() { + tempDir = createTemporarySaveDirectory(); + MockFilePicker.displayDirectory = tempDir; + + registerCleanupFunction(async function() { + MockFilePicker.cleanup(); + await cleanupDownloads(); + tempDir.remove(true); + }); +}); + +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 + */ +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]], + }); + + await BrowserTestUtils.withNewTab( + { gBrowser, url: "about:blank" }, + async function(browser) { + await waitForPdfJS(browser, pdfUrl); + + const tabCount = gBrowser.tabs.length; + 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(); + + let downloadsPanelPromise = BrowserTestUtils.waitForEvent( + DownloadsPanel.panel, + "popupshown" + ); + + info("Clicking on the download button..."); + await SpecialPowers.spawn(browser, [], () => { + content.document.querySelector("#download").click(); + }); + 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; + is( + DownloadsPanel.panel.state, + "open", + "Check the download panel state is 'open'" + ); + downloadList = await Downloads.getList(Downloads.PUBLIC); + let currentDownloadCount = (await downloadList.getAll()).length; + is( + currentDownloadCount, + initialDownloadCount + 1, + "A download was added when we clicked download" + ); + + is( + gBrowser.tabs.length, + tabCount, + "No new tab was opened to view the downloaded PDF" + ); + await closeDownloadsPanel(); + } + ); +}); diff --git a/toolkit/components/pdfjs/test/browser_pdfjs_saveas.js b/toolkit/components/pdfjs/test/browser_pdfjs_saveas.js index 03ee7012b951..0df10a39c477 100644 --- a/toolkit/components/pdfjs/test/browser_pdfjs_saveas.js +++ b/toolkit/components/pdfjs/test/browser_pdfjs_saveas.js @@ -15,15 +15,6 @@ Services.scriptloader.loadSubScript( this ); -function createTemporarySaveDirectory() { - var saveDir = Services.dirsvc.get("TmpD", Ci.nsIFile); - saveDir.append("testsavedir"); - if (!saveDir.exists()) { - saveDir.create(Ci.nsIFile.DIRECTORY_TYPE, 0o755); - } - return saveDir; -} - function createPromiseForTransferComplete(expectedFileName, destFile) { return new Promise(resolve => { MockFilePicker.showCallback = fp => { diff --git a/toolkit/components/pdfjs/test/head.js b/toolkit/components/pdfjs/test/head.js index 96a762fa4030..94e8d0f59195 100644 --- a/toolkit/components/pdfjs/test/head.js +++ b/toolkit/components/pdfjs/test/head.js @@ -74,3 +74,32 @@ function changeMimeHandler(preferredAction, alwaysAskBeforeHandling) { return oldAction; } + +function createTemporarySaveDirectory() { + var saveDir = Services.dirsvc.get("TmpD", Ci.nsIFile); + saveDir.append("testsavedir"); + if (!saveDir.exists()) { + saveDir.create(Ci.nsIFile.DIRECTORY_TYPE, 0o755); + } + return saveDir; +} + +async function cleanupDownloads(listId = Downloads.PUBLIC) { + info("cleaning up downloads"); + let downloadList = await Downloads.getList(listId); + for (let download of await downloadList.getAll()) { + await download.finalize(true); + try { + 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); + } catch (error) { + info("The file " + download.target.path + " is not removed, " + error); + } + + await downloadList.remove(download); + await download.finalize(); + } +}