465783 - Fennec default download location inaccessible to users

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[cmtalbert]

The default location for downloads in fennec is /home/user/Downloads however the user only has access to the file system inside /home/user/MyDocs/ (from the file manager in the phone)
== expected ==
The user should be able to find downloaded files using the file manager

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

We need to establish where we want to default to on Maemo and Win Mobile.

I guess /home/user/MyDocs is the maemo location. Is that a "special" folder for Maemo or will we need to expicitly set that using the download preferences?

Use preprocessing defines in mobile.js to keep Maemo and WinMo separate.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

We could also just fix the directory service and/or nsDownloadManager::GetDefaultDownloadsDirectory to return the right directories for those platforms.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #2)
> We could also just fix the directory service and/or
> nsDownloadManager::GetDefaultDownloadsDirectory to return the right directories
> for those platforms.

I like that idea the best

Comment 4

[Shawn Wilsher :sdwilsh]

(In reply to comment #2)
> We could also just fix the directory service and/or
> nsDownloadManager::GetDefaultDownloadsDirectory to return the right directories
> for those platforms.
Fixing the directory service should basically fix nsDownloadManager

Comment 5

[Antonio Gomes (tonikitoo)]

this is what is happening:

* in nsDownloadManager::GetDefaultDownloadsDirectory method body there is

(...)
#elif defined(XP_UNIX)
  rv = dirService->Get(NS_UNIX_DEFAULT_DOWNLOAD_DIR,
                       NS_GET_IID(nsILocalFile),
                       getter_AddRefs(downloadDir));
  // fallback to Home/Downloads
  if (NS_FAILED(rv)) {
    rv = dirService->Get(NS_UNIX_HOME_DIR,
                         NS_GET_IID(nsILocalFile),
                         getter_AddRefs(downloadDir));
    NS_ENSURE_SUCCESS(rv, rv);
    rv = downloadDir->Append(folderName);
    NS_ENSURE_SUCCESS(rv, rv);
  }
(...)

fennec (maemo) goes to the fallback implementation because "dirService->Get(NS_UNIX_DEFAULT_DOWNLOAD_DIR," fails: basically maemo has no "xdg-user-dirs" data/info available (see ~/.config/user-dirs.dirs in your linux desktop to know what I am talking about) and so the fallback code comes in scene. 

In fallback implementation, "$(HOME)/Downloads" turns to be the default folder for downloads (see http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties#120 )

the simpler approach would be to make the fallback use "MyDocs" instead of "Downloads" for maemo.

taking ...

Comment 6

[Antonio Gomes (tonikitoo)]

Attachment: v0.1 - simpler appoach

patch basically makes the fallback implementation to end up in /home/user/MyDocs folder in maemo.

Comment 7

[Shawn Wilsher :sdwilsh]

Comment on attachment 362531 [details] [diff] [review]
v0.1 - simpler appoach

Can we just get the users doc directory with the directory service instead please?

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

I think the directory service _and_ download manager will need patches. Maemo does not have a "download" directory. It does have a "documents" directory, much like WindowsXP.

I think we should add code to directory service so suppport NS_UNIX_XDG_DOCUMENTS_DIR (which might already work, I haven't tested it):

http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsDirectoryService.cpp#1179

We should also add code to the download manager to use the "documents" folder as a fallback, like we do for Windows, except we should not create/append the "Downloads" folder.

Comment 9

[Antonio Gomes (tonikitoo)]

> I think we should add code to directory service so suppport
> NS_UNIX_XDG_DOCUMENTS_DIR (which might already work, I haven't tested it):

All UNIX_XDG_BLABLA falls in the same treatment:

(...)
        case Unix_XDG_Desktop:
        case Unix_XDG_Documents:
        case Unix_XDG_Download:
        case Unix_XDG_Music:
        case Unix_XDG_Pictures:
        case Unix_XDG_PublicShare:
        case Unix_XDG_Templates:
        case Unix_XDG_Videos:
            return GetUnixXDGUserDirectory(aSystemSystemDirectory, aFile);
(...)

however in "GetUnixXDGUserDirectory" ~/.config/user-dirs.dirs is needed otherwise it goes to a fallback (maemo case). So we can handle 'Unix_XDG_Documents' in that fallback for maemo.

Comment 10

[Antonio Gomes (tonikitoo)]

Attachment: v0.2 - make DirService to support Unix_XDG_Documents for maemo, and calling it from nsDownloadManager also for that platform

mark, shawn, what about this? (see comment 9 above)

Comment 11

[Antonio Gomes (tonikitoo)]

Attachment: v0.3 - make GetUnixXDGUserDirectory @ SpecialSystemDirectories to fallback in "$MYDOCSDIR/.documents" for Unix_XDG_Documents.

basically instead of /home/user/MyDocs default location is /home/user/MyDocs/.documents , as per mark's suggestion on irc.

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 362646 [details] [diff] [review]
v0.3 - make GetUnixXDGUserDirectory @ SpecialSystemDirectories to fallback in "$MYDOCSDIR/.documents" for Unix_XDG_Documents.

From a Fennec point of view, I'm OK with the way this works.

Comment 13

[Shawn Wilsher :sdwilsh]

Comment on attachment 362646 [details] [diff] [review]
v0.3 - make GetUnixXDGUserDirectory @ SpecialSystemDirectories to fallback in "$MYDOCSDIR/.documents" for Unix_XDG_Documents.

>+#if defined(NS_OSSO)
>+    rv = dirService->Get(NS_UNIX_XDG_DOCUMENTS_DIR,
>+                       NS_GET_IID(nsILocalFile),
>+                       getter_AddRefs(downloadDir));
>+#else
nit: fix the indenting here (two spaces too many), and add a comment explaining why we do this.

>diff -r 04ab0748f650 xpcom/io/SpecialSystemDirectory.cpp
you'll need sr for this from bsmedberg

Comment 14

[Antonio Gomes (tonikitoo)]

Attachment: v0.4 - make GetUnixXDGUserDirectory @ SpecialSystemDirectories to fallback in "$MYDOCSDIR/.documents" for Unix_XDG_Documents.

same patch as previous but added comment as requested.

carrying mfinkle and sdwilsh r+ and reasking for sr=benjamin for the xpcom part.

Comment 15

[Benjamin Smedberg]

Comment on attachment 363050 [details] [diff] [review]
v0.4 - make GetUnixXDGUserDirectory @ SpecialSystemDirectories to fallback in "$MYDOCSDIR/.documents" for Unix_XDG_Documents.

Boy, hardcoding ".documents" feels hacky, but if that's the official way of doing things...

Comment 16

[Antonio Gomes (tonikitoo)]

fixed

1b3948b3562c581ddbb525d427721b3bce7c9682

Comment 17

[Joel Maher ( :jmaher ) (UTC -8)]

I tried this today on the maemo device with the beta1 rc1 build, and it downloads to /home/user/MyDocs/.documents.

When I use the maemo device, I cannot access .documents from a file input tag when using the "browse" button:

https://developer.mozilla.org/samples/installjar.html


It could be we need to fix the file tag to work like other applications, but this is still confusing.

Comment 18

[Joel Maher ( :jmaher ) (UTC -8)]

Verified with 20091001 1.9.2 beta4 candidate on my n810