Bug 1714107 - File opened by application is saved to user configured dir. r=mtigley,Gijs

Differential Revision: https://phabricator.services.mozilla.com/D122005

uriloader/exthandler/nsExternalHelperAppService.cpp

@@ -282,72 +282,77 @@ static bool GetFilenameAndExtensionFromChannel(nsIChannel* aChannel,
  */
 static nsresult GetDownloadDirectory(nsIFile** _directory,
                                      bool aSkipChecks = false) {
-  nsCOMPtr<nsIFile> 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<nsIFile> 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!");

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]

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