399498 - use XDG downloads dir as default downloads dir

Status: VERIFIED FIXED

(Toolkit :: Downloads API, enhancement)

Description

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: proposed patch: use XDG download dir, with a fallback to the current default if the XDG user dirs are disabled

nsDownloadManager::GetDefaultDownloadsDirectory currently uses $HOME/Downloads (localised string) on linux. The XDG user directories (see URL) allow the user to customise the directory name, while still providing a localised name by default, and to maintain the directory across locale changes.

To integrate better with GNOME (and other DEs too, since it's a freedesktop.org spec) which has adopted this spec, the download manager should use the XDG Download directory as its default download directory (used when browser.download.folderList = 1).

(Note that this is not a dup of bug 259356 since that bug is about following the XDG base dir spec for the profile location etc.)

Comment 1

[Christian Persch (GNOME) (away; not receiving bug mail)]

Comment on attachment 284517 [details] [diff] [review]
proposed patch: use XDG download dir, with a fallback to the current default if the XDG user dirs are disabled

This depends on the patch in bug 399500 which adds that key to the dir service; if that doesn't get into 1.9 I'll have to come up with another approach here; maybe define a NS_UNIX_DEFAULT_DOWNLOAD_DIR key and query that before falling back to $HOME/Downloads (so I can provide the correct dir in my own directory provider).

Comment 2

[Shawn Wilsher :sdwilsh]

Comment on attachment 284517 [details] [diff] [review]
proposed patch: use XDG download dir, with a fallback to the current default if the XDG user dirs are disabled

>+#if defined(XP_UNIX)
>+  rv = dirService->Get(NS_UNIX_XDG_DOWNLOAD_DIR,
>+                       NS_GET_IID(nsILocalFile),
>+                       getter_AddRefs(downloadDir));
>+  // fallback to Home/Downloads
>+  if (NS_FAILED(rv)) {
I'm inclined to just copy/paste this code here, instead of doing this ifdef stuff because it's not immediately clear as to what's going on.  I'd rather have easy to read code than save a few bytes of source file size.

Comment 3

[Christian Persch (GNOME) (away; not receiving bug mail)]

Attachment: updated patch

This just copies the code instead of the #ifdef, as requested by the reviewer.

Also I changed the dir service key to add NS_UNIX_DEFAULT_DOWNLOAD_DIR instead of using the XDG key directly (there's precedent for this in the OSX and WIN DEFAULT_DOWNLOAD keys); this is because I don't have much hope that bug 399500 will make it into 1.9, but I still need this way to provide the right dir from my own dir service provider.

Comment 4

[Christian Persch (GNOME) (away; not receiving bug mail)]

Comment on attachment 286148 [details] [diff] [review]
updated patch

Requesting additional review from bsmedberg for the addition of the NS_UNIX_DEFAULT_DOWNLOAD_DIR define in nsDirectoryServiceDefs.h .

Comment 5

[Shawn Wilsher :sdwilsh]

Comment on attachment 286148 [details] [diff] [review]
updated patch

>+  // fallback to Home/Downloads
>+  if (NS_FAILED(rv)) {
>+    rv = dirService->Get(NS_OS_HOME_DIR,
NS_UNIX_HOME_DIR?  They may point to the same, but I think it makes more sense here

>+                        NS_GET_IID(nsILocalFile),
>+                        getter_AddRefs(downloadDir));
nit: +1 space on the start of both of these lines

r=sdwilsh with these fixed.

Comment 6

[Rimas Kudelis [use needinfo to ping]]

I hope this bugs gets fixed before the release of Fx 3.0, because...

Currently, Firefox seems to default to "~/Desktop" as a download dir, however it seems like GNOME is now localizing the name of that folder, so just "~/Desktop" no longer "just works". I'm not sure, this *COULD* be a wrong translation in lt, but it seems a little odd when Firefox tells it downloads stuff to Desktop, but downloads it to ~ instead.

Comment 7

[Christian Persch (GNOME) (away; not receiving bug mail)]

This patch is just putting the infrastructure in place for that; actually using the XDG dir is bug 399500.

The patch is waiting for r/sr on the addition of the #define in xpcom/; afterwards I'll post a patch with the nits from comment 5 adressed.

Comment 8

[Shawn Wilsher :sdwilsh]

Requesting blocking per comment 6

Comment 9

[Mike Connor [:mconnor]]

Nice to have, but not going to block on it if we don't get the support landed.  There's far too much other stuff we want to get into users' hands.

Comment 10

[Reed Loden [:reed]]

Attachment: patch for check-in

Combined with the patch from bug 399500, this completes parity of all three platforms of choosing the correct downloads directory. Without these two patches, problems like what comment #6 mentions are likely to happen on non-English Linux installs, which will greatly confuse users.

Comment 11

[Damon Sicore (:damons)]

Comment on attachment 308851 [details] [diff] [review]
patch for check-in

a1.9+=damons

Do I have to point out that time was spent on a non-blocker here?  /me looks to see if there are other blockers that sdwilsh could look at?  :)

Comment 12

[Reed Loden [:reed]]

(In reply to comment #11)
> Do I have to point out that time was spent on a non-blocker here?  /me looks to
> see if there are other blockers that sdwilsh could look at?  :)

Do note that sdwilsh gave his r+ on 2007-10-26 08:48:25 PDT, so the blockers-only speech isn't as useful here. :)

Comment 13

[Reed Loden [:reed]]

Checking in toolkit/components/downloads/src/nsDownloadManager.cpp;
/cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp,v  <--  nsDownloadManager.cpp
new revision: 1.171; previous revision: 1.170
done
Checking in xpcom/io/nsDirectoryServiceDefs.h;
/cvsroot/mozilla/xpcom/io/nsDirectoryServiceDefs.h,v  <--  nsDirectoryServiceDefs.h
new revision: 1.32; previous revision: 1.31
done

Comment 14

[Rimas Kudelis [use needinfo to ping]]

Verifying fixed with Lithuanian XDG settings (GNOME) and the following build:

Mozilla/5.0 (X11; U; Linux i686; lt; rv:1.9b5pre) Gecko/2008031304 Minefield/3.0b5pre

Comment 15

[antistress]

l18n problem :

i'm playing with Firefox 3.5~b4 within Ubuntu 9.04 Jaunty beta and my XDG_DOWNLOAD is automatically chosen by Firefox to store files that i've downloaded which is great (and might be new in 3.5 version since i didn't noticed that before)

But although my user-dirs.dirs file says :
XDG_DOWNLOAD_DIR="$HOME/Téléchargements"
Firefox 3.5 offers me the choice to "Save files to Download" whereas it should say "Save files to Téléchargements" in my case (which is the l18n version of "Download" set in the user-dirs.dirs file)

Comment 16

[Shawn Wilsher :sdwilsh]

File a new bug please.  Commenting in old resolved bugs is a surefire way to have that comment ignored.

Comment 17

[antistress]

ok, that's Bug 488588