diff --git a/toolkit/components/contentanalysis/tests/browser/browser.toml b/toolkit/components/contentanalysis/tests/browser/browser.toml index 5cd1af9469fd..31842a4476fe 100644 --- a/toolkit/components/contentanalysis/tests/browser/browser.toml +++ b/toolkit/components/contentanalysis/tests/browser/browser.toml @@ -150,3 +150,9 @@ support-files = [ "browser_print_pdf_content_analysis_impl.js", "file_pdf.pdf", ] + +["browser_save_page_complete_content_analysis.js"] +support-files = [ + "save_page_complete.css", + "save_page_complete.html", +] diff --git a/toolkit/components/contentanalysis/tests/browser/browser_save_page_complete_content_analysis.js b/toolkit/components/contentanalysis/tests/browser/browser_save_page_complete_content_analysis.js new file mode 100644 index 000000000000..5d63ec458e4a --- /dev/null +++ b/toolkit/components/contentanalysis/tests/browser/browser_save_page_complete_content_analysis.js @@ -0,0 +1,117 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +let MockFilePicker = SpecialPowers.MockFilePicker; +let mockCA; + +const TEST_URL = + "https://example.com/browser/toolkit/components/contentanalysis/tests/browser/save_page_complete.html"; + +let tempDir; + +add_setup(async function test_setup() { + mockCA = await mockContentAnalysisService(makeMockContentAnalysis()); + + tempDir = Services.dirsvc.get("TmpD", Ci.nsIFile); + tempDir.append("test-save-page-complete-dir"); + tempDir.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0o755); + + MockFilePicker.init(); + MockFilePicker.returnValue = MockFilePicker.returnOK; + MockFilePicker.displayDirectory = tempDir; + + registerCleanupFunction(async function () { + MockFilePicker.cleanup(); + let list = await Downloads.getList(Downloads.ALL); + await list.removeFinished(); + tempDir.remove(true); + }); +}); + +function createPromiseForFilePicker() { + return new Promise(resolve => { + MockFilePicker.showCallback = fp => { + let destFile = tempDir.clone(); + destFile.append(fp.defaultString); + let filesFolder = tempDir.clone(); + filesFolder.append(fp.defaultString.replace(/\.[^.]*$/, "") + "_files"); + MockFilePicker.setFiles([destFile]); + MockFilePicker.filterIndex = 0; // kSaveAsType_Complete + resolve({ destFile, filesFolder }); + }; + }); +} + +function promiseDownloadFinished(list) { + return new Promise(resolve => { + list.addView({ + onDownloadChanged(download) { + download.launchWhenSucceeded = false; + if (download.succeeded || download.error) { + list.removeView(this); + resolve(download); + } + }, + }); + }); +} + +async function testSavePageComplete(allow) { + mockCA.setupForTest(allow); + await SpecialPowers.pushPrefEnv({ + set: [ + ["browser.contentanalysis.interception_point.download.enabled", true], + ], + }); + + await BrowserTestUtils.withNewTab({ gBrowser, url: TEST_URL }, async () => { + let downloadList = await Downloads.getList(Downloads.ALL); + let filePickerShown = createPromiseForFilePicker(); + let downloadFinishedPromise = promiseDownloadFinished(downloadList); + + saveBrowser(gBrowser.selectedBrowser); + + info("Waiting for a filename to be picked from the file picker"); + let { destFile, filesFolder } = await filePickerShown; + let download = await downloadFinishedPromise; + + is(mockCA.calls.length, 1, "Content Analysis called once"); + is(download.target.path, destFile.path, "Download saved to picked file"); + is( + download.target.filesFolderPath, + filesFolder.path, + "Download knows about its files folder" + ); + is( + await IOUtils.exists(download.target.path), + allow, + "Target file existence" + ); + is(await IOUtils.exists(filesFolder.path), allow, "Files folder existence"); + + if (allow) { + ok(!download.error, "Download should not have an error"); + await IOUtils.remove(download.target.path); + await IOUtils.remove(filesFolder.path, { recursive: true }); + } else { + ok( + download.error.becauseBlockedByContentAnalysis, + "Download blocked by content analysis" + ); + } + + await downloadList.removeFinished(); + }); + + await SpecialPowers.popPrefEnv(); +} + +add_task(async function testSavePageCompleteAllow() { + await testSavePageComplete(true); +}); + +add_task(async function testSavePageCompleteBlock() { + await testSavePageComplete(false); +}); diff --git a/toolkit/components/contentanalysis/tests/browser/save_page_complete.css b/toolkit/components/contentanalysis/tests/browser/save_page_complete.css new file mode 100644 index 000000000000..11e462d90d75 --- /dev/null +++ b/toolkit/components/contentanalysis/tests/browser/save_page_complete.css @@ -0,0 +1,4 @@ +p { + /* stylelint-disable-next-line stylelint-plugin-mozilla/use-design-tokens */ + color: red; +} diff --git a/toolkit/components/contentanalysis/tests/browser/save_page_complete.html b/toolkit/components/contentanalysis/tests/browser/save_page_complete.html new file mode 100644 index 000000000000..9df2a7b4c5d7 --- /dev/null +++ b/toolkit/components/contentanalysis/tests/browser/save_page_complete.html @@ -0,0 +1,11 @@ + + + + + Test save page complete + + + +

This page has an external stylesheet that is saved alongside it.

+ + diff --git a/toolkit/components/downloads/DownloadCore.sys.mjs b/toolkit/components/downloads/DownloadCore.sys.mjs index 1889146a5531..77dff83e2700 100644 --- a/toolkit/components/downloads/DownloadCore.sys.mjs +++ b/toolkit/components/downloads/DownloadCore.sys.mjs @@ -1883,6 +1883,14 @@ DownloadTarget.prototype = { */ partFilePath: null, + /** + * String containing the path of the directory holding the additional files + * of a download that involves multiple files, like a complete web page saved + * to disk, or null if the download has no such directory. When the data of + * the download is removed, this directory is removed as well. + */ + filesFolderPath: null, + /** * Indicates whether the target file exists. * @@ -1964,11 +1972,19 @@ DownloadTarget.prototype = { */ toSerializable() { // Simplify the representation if we don't have other details. - if (!this.partFilePath && !this._unknownProperties) { + if ( + !this.partFilePath && + !this.filesFolderPath && + !this._unknownProperties + ) { return this.path; } - let serializable = { path: this.path, partFilePath: this.partFilePath }; + let serializable = { + path: this.path, + partFilePath: this.partFilePath, + filesFolderPath: this.filesFolderPath, + }; serializeUnknownProperties(this, serializable); return serializable; }, @@ -1984,6 +2000,9 @@ DownloadTarget.prototype = { * { * path: String containing the path of the target file. * partFilePath: optional string containing the part file path. + * filesFolderPath: optional string containing the path of the + * directory holding the additional files of the + * download. * } * * @return The newly created DownloadTarget object. @@ -2003,11 +2022,17 @@ DownloadTarget.fromSerializable = function (aSerializable) { if ("partFilePath" in aSerializable) { target.partFilePath = aSerializable.partFilePath; } + if ("filesFolderPath" in aSerializable) { + target.filesFolderPath = aSerializable.filesFolderPath; + } deserializeUnknownProperties( target, aSerializable, - property => property != "path" && property != "partFilePath" + property => + property != "path" && + property != "partFilePath" && + property != "filesFolderPath" ); } return target; @@ -2954,9 +2979,9 @@ DownloadCopySaver.prototype = { */ async removeData(canRemoveFinalTarget = false) { // Defined inline so removeData can be shared with DownloadLegacySaver. - async function _tryToRemoveFile(path) { + async function _tryToRemoveFile(path, recursive = false) { try { - await IOUtils.remove(path); + await IOUtils.remove(path, { recursive }); } catch (ex) { // On Windows we may get an access denied error instead of a no such // file error if the file existed before, and was recently deleted. This @@ -2978,6 +3003,12 @@ DownloadCopySaver.prototype = { (await isPlaceholder(this.download.target.path)) ) { await _tryToRemoveFile(this.download.target.path); + // A download that saved a complete web page also created a directory + // holding the additional files of the page, that has to be removed + // along with the main file. + if (this.download.target.filesFolderPath) { + await _tryToRemoveFile(this.download.target.filesFolderPath, true); + } } this.download.target.exists = false; this.download.target.size = 0; diff --git a/toolkit/components/downloads/DownloadLegacy.sys.mjs b/toolkit/components/downloads/DownloadLegacy.sys.mjs index a60af5ed30c6..d76e7eb4bae9 100644 --- a/toolkit/components/downloads/DownloadLegacy.sys.mjs +++ b/toolkit/components/downloads/DownloadLegacy.sys.mjs @@ -265,7 +265,8 @@ DownloadLegacyTransfer.prototype = { aIsPrivate, aDownloadClassification, aReferrerInfo, - aOpenDownloadsListOnStart + aOpenDownloadsListOnStart, + aFilesFolder ) { return this._nsITransferInitInternal( aSource, @@ -279,7 +280,8 @@ DownloadLegacyTransfer.prototype = { aIsPrivate, aDownloadClassification, aReferrerInfo, - aOpenDownloadsListOnStart + aOpenDownloadsListOnStart, + aFilesFolder ); }, @@ -321,6 +323,7 @@ DownloadLegacyTransfer.prototype = { aDownloadClassification, aReferrerInfo, aOpenDownloadsListOnStart, + null, userContextId, browsingContextId, aHandleInternally, @@ -340,7 +343,8 @@ DownloadLegacyTransfer.prototype = { isPrivate, aDownloadClassification, referrerInfo, - openDownloadsListOnStart = true, + openDownloadsListOnStart, + filesFolder, userContextId = 0, browsingContextId = 0, handleInternally = false, @@ -388,6 +392,7 @@ DownloadLegacyTransfer.prototype = { target: { path: aTarget.QueryInterface(Ci.nsIFileURL).file.path, partFilePath: aTempFile && aTempFile.path, + filesFolderPath: filesFolder && filesFolder.path, }, saver: "legacy", launchWhenSucceeded, diff --git a/toolkit/components/downloads/test/unit/head.js b/toolkit/components/downloads/test/unit/head.js index 48097b318615..6b2e22f0d9d5 100644 --- a/toolkit/components/downloads/test/unit/head.js +++ b/toolkit/components/downloads/test/unit/head.js @@ -338,6 +338,8 @@ function promiseStartLegacyDownload(aSourceUrl, aOptions) { persist, isPrivate, classification, + null, + false, null ); persist.progressListener = transfer; diff --git a/toolkit/content/contentAreaUtils.js b/toolkit/content/contentAreaUtils.js index 067528d0e8ec..18194456df15 100644 --- a/toolkit/content/contentAreaUtils.js +++ b/toolkit/content/contentAreaUtils.js @@ -491,6 +491,25 @@ function internalPersist(persistArgs) { // Find the URI associated with the target file var targetFileURL = makeFileURI(persistArgs.targetFile); + // The local directory into which to save the files associated with a + // document, or null when only a single file is written. + var filesFolder = null; + if ( + persistArgs.sourceDocument && + persistArgs.targetContentType != "text/plain" + ) { + filesFolder = persistArgs.targetFile.clone(); + + var nameWithoutExtension = getFileBaseName(filesFolder.leafName); + // Given the minimal benefits, the "_files" suffix is intentionally not + // localized. Localizing it introduces complexity in handling OS filename + // length limits (e.g. bug 1959738) and risks breaking the folder-linking + // feature if an unsupported suffix is used. + var filesFolderLeafName = nameWithoutExtension + "_files"; + + filesFolder.leafName = filesFolderLeafName; + } + // Create download and initiate it (below) var tr = Cc["@mozilla.org/transfer;1"].createInstance(Ci.nsITransfer); tr.init( @@ -504,7 +523,9 @@ function internalPersist(persistArgs) { persist, persistArgs.isPrivate, Ci.nsITransfer.DOWNLOAD_ACCEPTABLE, - persistArgs.sourceReferrerInfo + persistArgs.sourceReferrerInfo, + false /* aOpenDownloadsListOnStart */, + filesFolder ); persist.progressListener = new DownloadListener(window, tr); const { saveCompleteCallback } = persistArgs; @@ -517,21 +538,6 @@ function internalPersist(persistArgs) { if (persistArgs.sourceDocument) { // Saving a Document, not a URI: - var filesFolder = null; - if (persistArgs.targetContentType != "text/plain") { - // Create the local directory into which to save associated files. - filesFolder = persistArgs.targetFile.clone(); - - var nameWithoutExtension = getFileBaseName(filesFolder.leafName); - // Given the minimal benefits, the "_files" suffix is intentionally not - // localized. Localizing it introduces complexity in handling OS filename - // length limits (e.g. bug 1959738) and risks breaking the folder-linking - // feature if an unsupported suffix is used. - var filesFolderLeafName = nameWithoutExtension + "_files"; - - filesFolder.leafName = filesFolderLeafName; - } - var encodingFlags = 0; if (persistArgs.targetContentType == "text/plain") { encodingFlags |= nsIWBP.ENCODE_FLAGS_FORMATTED; diff --git a/uriloader/base/nsITransfer.idl b/uriloader/base/nsITransfer.idl index 80034f4c5b19..95633008de37 100644 --- a/uriloader/base/nsITransfer.idl +++ b/uriloader/base/nsITransfer.idl @@ -64,9 +64,14 @@ interface nsITransfer : nsIWebProgressListener2 { * * @param aReferrerInfo The Referrer this download is started with * - * @param aOpenDownloadsListOnStart true (default) - Open downloads panel. + * @param aOpenDownloadsListOnStart true - Open downloads panel. * false - Only show an icon indicator. - * This parameter is optional. + * + * @param aFilesFolder The directory holding the additional files of a + * transfer that writes more than one file, like a + * complete web page saved to disk. It is removed along + * with the target file when the data of the download is + * removed. May be null. */ void init(in nsIURI aSource, in nsIURI aSourceOriginalURI, @@ -79,7 +84,8 @@ interface nsITransfer : nsIWebProgressListener2 { in boolean aIsPrivate, in long aDownloadClassification, in nsIReferrerInfo aReferrerInfo, - [optional] in boolean aOpenDownloadsListOnStart); + in boolean aOpenDownloadsListOnStart, + in nsIFile aFilesFolder); /** * Same as init, but allows for passing the browsingContext diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index 601070dcca72..ccebdcc8fee7 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -2442,10 +2442,10 @@ nsresult nsExternalAppHandler::CreateTransfer() { 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, !mDialogShowing); + rv = transfer->Init( + mSourceUrl, nullptr, target, u""_ns, mMimeInfo, mTimeDownloadStarted, + mTempFile, this, channel && NS_UsePrivateBrowsing(channel), + mDownloadClassification, referrerInfo, !mDialogShowing, nullptr); } mDialogShowing = false; @@ -2539,7 +2539,7 @@ nsresult nsExternalAppHandler::CreateFailedTransfer() { rv = transfer->Init(mSourceUrl, nullptr, pseudoTarget, u""_ns, mMimeInfo, mTimeDownloadStarted, mTempFile, this, channel && NS_UsePrivateBrowsing(channel), - mDownloadClassification, referrerInfo, true); + mDownloadClassification, referrerInfo, true, nullptr); } NS_ENSURE_SUCCESS(rv, rv);