From cb852bdc510e5691c9af569a35e10316fd340bd3 Mon Sep 17 00:00:00 2001 From: Ava Katushka ava8katushka Date: Wed, 11 Aug 2021 13:06:14 +0000 Subject: [PATCH] Bug 1714107 - File opened by application is saved to user configured dir. r=mtigley,Gijs Differential Revision: https://phabricator.services.mozilla.com/D122005 --- .../exthandler/nsExternalHelperAppService.cpp | 120 +++++++++--------- .../exthandler/tests/mochitest/browser.ini | 4 +- .../browser_launched_app_save_directory.js | 52 +++++++- 3 files changed, 112 insertions(+), 64 deletions(-) diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index 88120a8d1b07..06eea47f3394 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -282,72 +282,77 @@ static bool GetFilenameAndExtensionFromChannel(nsIChannel* aChannel, */ static nsresult GetDownloadDirectory(nsIFile** _directory, bool aSkipChecks = false) { - nsCOMPtr dir; +#if defined(ANDROID) + return NS_ERROR_FAILURE; +#endif + bool usePrefDir = + StaticPrefs::browser_download_improvements_to_download_panel(); #ifdef XP_MACOSX - // On OS X, we first try to get the users download location, if it's set. - switch (Preferences::GetInt(NS_PREF_DOWNLOAD_FOLDERLIST, -1)) { - case NS_FOLDER_VALUE_DESKTOP: - (void)NS_GetSpecialDirectory(NS_OS_DESKTOP_DIR, getter_AddRefs(dir)); - break; - case NS_FOLDER_VALUE_CUSTOM: { - Preferences::GetComplex(NS_PREF_DOWNLOAD_DIR, NS_GET_IID(nsIFile), - getter_AddRefs(dir)); - if (!dir) break; + usePrefDir = true; +#endif - // If we're not checking for availability we're done. - if (aSkipChecks) { - dir.forget(_directory); - return NS_OK; - } - - // We have the directory, and now we need to make sure it exists - bool dirExists = false; - (void)dir->Exists(&dirExists); - if (dirExists) break; - - nsresult rv = dir->Create(nsIFile::DIRECTORY_TYPE, 0755); - if (NS_FAILED(rv)) { - dir = nullptr; + nsCOMPtr dir; + nsresult rv; + if (usePrefDir) { + // Try to get the users download location, if it's set. + switch (Preferences::GetInt(NS_PREF_DOWNLOAD_FOLDERLIST, -1)) { + case NS_FOLDER_VALUE_DESKTOP: + (void)NS_GetSpecialDirectory(NS_OS_DESKTOP_DIR, getter_AddRefs(dir)); break; - } - } break; - case NS_FOLDER_VALUE_DOWNLOADS: - // This is just the OS default location, so fall out - break; + case NS_FOLDER_VALUE_CUSTOM: { + Preferences::GetComplex(NS_PREF_DOWNLOAD_DIR, NS_GET_IID(nsIFile), + getter_AddRefs(dir)); + if (!dir) break; + + // If we're not checking for availability we're done. + if (aSkipChecks) { + dir.forget(_directory); + return NS_OK; + } + + // We have the directory, and now we need to make sure it exists + bool dirExists = false; + (void)dir->Exists(&dirExists); + if (dirExists) break; + + nsresult rv = dir->Create(nsIFile::DIRECTORY_TYPE, 0755); + if (NS_FAILED(rv)) { + dir = nullptr; + break; + } + } break; + case NS_FOLDER_VALUE_DOWNLOADS: + // This is just the OS default location, so fall out + break; + } } if (!dir) { - // If not, we default to the OS X default download location. - nsresult rv = NS_GetSpecialDirectory(NS_OSX_DEFAULT_DOWNLOAD_DIR, - getter_AddRefs(dir)); +#ifdef XP_MACOSX + // Default to the OS X default download location. + rv = NS_GetSpecialDirectory(NS_OSX_DEFAULT_DOWNLOAD_DIR, + getter_AddRefs(dir)); +#elif defined(XP_WIN) + rv = NS_GetSpecialDirectory( + StaticPrefs::browser_download_improvements_to_download_panel() + ? NS_WIN_DEFAULT_DOWNLOAD_DIR + : NS_OS_TEMP_DIR, + getter_AddRefs(dir)); +#elif defined(XP_UNIX) + rv = NS_GetSpecialDirectory( + StaticPrefs::browser_download_improvements_to_download_panel() + ? NS_UNIX_DEFAULT_DOWNLOAD_DIR + : NS_OS_TEMP_DIR, + getter_AddRefs(dir)); +#else + // On all other platforms, we default to the systems temporary directory. + rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(dir)); +#endif + NS_ENSURE_SUCCESS(rv, rv); } -#elif defined(ANDROID) - return NS_ERROR_FAILURE; -#elif defined(XP_WIN) - // On windows, use default downloads directory if pref is set - const char* directory_to_save_file = - StaticPrefs::browser_download_improvements_to_download_panel() - ? NS_WIN_DEFAULT_DOWNLOAD_DIR - : NS_OS_TEMP_DIR; - nsresult rv = - NS_GetSpecialDirectory(directory_to_save_file, getter_AddRefs(dir)); - NS_ENSURE_SUCCESS(rv, rv); -#elif defined(XP_UNIX) - // On unix, use default downloads directory if pref is set - const char* directory_to_save_file = - StaticPrefs::browser_download_improvements_to_download_panel() - ? NS_UNIX_DEFAULT_DOWNLOAD_DIR - : NS_OS_TEMP_DIR; - nsresult rv = - NS_GetSpecialDirectory(directory_to_save_file, getter_AddRefs(dir)); - NS_ENSURE_SUCCESS(rv, rv); -#else - // On all other platforms, we default to the systems temporary directory. - nsresult rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(dir)); - NS_ENSURE_SUCCESS(rv, rv); -# if defined(XP_UNIX) +#if defined(XP_UNIX) // Ensuring that only the current user can read the file names we end up // creating. Note that Creating directories with specified permission only // supported on Unix platform right now. That's why above if exists. @@ -415,7 +420,6 @@ static nsresult GetDownloadDirectory(nsIFile** _directory, } } -# endif #endif NS_ASSERTION(dir, "Somehow we didn't get a download directory!"); diff --git a/uriloader/exthandler/tests/mochitest/browser.ini b/uriloader/exthandler/tests/mochitest/browser.ini index 0552762f98e0..16655f0d6b9c 100644 --- a/uriloader/exthandler/tests/mochitest/browser.ini +++ b/uriloader/exthandler/tests/mochitest/browser.ini @@ -41,8 +41,8 @@ support-files = file_as.exe^headers^ [browser_launched_app_save_directory.js] # This test checks the save destination of the - # open with app download on Windows and Linux. -skip-if = (os == 'mac') || (os == 'android') + # open with app download on Windows, Linux and OS X. +skip-if = (os == 'android') support-files = file_pdf_application_pdf.pdf [browser_shows_where_to_save_dialog.js] diff --git a/uriloader/exthandler/tests/mochitest/browser_launched_app_save_directory.js b/uriloader/exthandler/tests/mochitest/browser_launched_app_save_directory.js index 1d0acceb4cf4..a5e16dd8b554 100644 --- a/uriloader/exthandler/tests/mochitest/browser_launched_app_save_directory.js +++ b/uriloader/exthandler/tests/mochitest/browser_launched_app_save_directory.js @@ -11,17 +11,25 @@ const TEST_PATH = getRootDirectory(gTestPath).replace( "https://example.com" ); -// This test ensures that a file downloaded with "open with app" option -// is actually saved in default Downloads directory. -add_task(async function aDownloadLaunchedWithAppIsSavedInDownloadsFolder() { +add_task(async function setup() { await SpecialPowers.pushPrefEnv({ set: [["browser.download.improvements_to_download_panel", true]], }); + const allowDirectoriesVal = DownloadIntegration.allowDirectories; + DownloadIntegration.allowDirectories = true; + registerCleanupFunction(() => { + DownloadIntegration.allowDirectories = allowDirectoriesVal; + Services.prefs.clearUserPref("browser.download.dir"); + Services.prefs.clearUserPref("browser.download.folderList"); + }); +}); +async function aDownloadLaunchedWithAppIsSavedInFolder(downloadDir) { let publicList = await Downloads.getList(Downloads.PUBLIC); registerCleanupFunction(async () => { await publicList.removeFinished(); }); + let downloadFinishedPromise = promiseDownloadFinished(publicList); let initialTabsCount = gBrowser.tabs.length; @@ -38,7 +46,6 @@ add_task(async function aDownloadLaunchedWithAppIsSavedInDownloadsFolder() { gBrowser.removeCurrentTab(); BrowserTestUtils.removeTab(loadingTab); - let downloadDir = await DownloadIntegration.getSystemDownloadsDirectory(); ok( download.target.path.startsWith(downloadDir), "Download should be placed in default download directory: " + @@ -62,4 +69,41 @@ add_task(async function aDownloadLaunchedWithAppIsSavedInDownloadsFolder() { } catch (ex) { info("The file " + download.target.path + " is not removed, " + ex); } +} + +add_task(async function aDownloadLaunchedWithAppIsSavedInCustomDir() { + //Test the temp dir. + let time = new Date().getTime(); + let tempDir = Services.dirsvc.get("TmpD", Ci.nsIFile); + tempDir.append(time); + Services.prefs.setComplexValue("browser.download.dir", Ci.nsIFile, tempDir); + let downloadDir = await DownloadIntegration.getPreferredDownloadsDirectory(); + Assert.notEqual(downloadDir, ""); + Assert.equal(downloadDir, tempDir.path); + Assert.ok(await IOUtils.exists(downloadDir)); + registerCleanupFunction(async () => { + await IOUtils.remove(tempDir.path, { recursive: true }); + }); + await aDownloadLaunchedWithAppIsSavedInFolder(downloadDir); +}); + +add_task(async function aDownloadLaunchedWithAppIsSavedInDownloadsDir() { + // Test the system downloads directory. + Services.prefs.setIntPref("browser.download.folderList", 1); + let systemDir = await DownloadIntegration.getSystemDownloadsDirectory(); + let downloadDir = await DownloadIntegration.getPreferredDownloadsDirectory(); + Assert.notEqual(downloadDir, ""); + Assert.equal(downloadDir, systemDir); + + await aDownloadLaunchedWithAppIsSavedInFolder(downloadDir); +}); + +add_task(async function aDownloadLaunchedWithAppIsSavedInDesktopDir() { + // Test the desktop directory. + Services.prefs.setIntPref("browser.download.folderList", 0); + let downloadDir = await DownloadIntegration.getPreferredDownloadsDirectory(); + Assert.notEqual(downloadDir, ""); + Assert.equal(downloadDir, Services.dirsvc.get("Desk", Ci.nsIFile).path); + + await aDownloadLaunchedWithAppIsSavedInFolder(downloadDir); });