Open Bug 814000 Opened 9 years ago Updated 2 years ago

Setting a pre-defined download folder for attachments is inconsequential - almost never used in attachments UI (Attachment options > Save files to [folder] aka Browser.download.dir pref)

Categories

(Thunderbird :: Untriaged, defect)

17 Branch
x86
Windows Vista
defect
Not set
normal

Tracking

(Not tracked)

UNCONFIRMED

People

(Reporter: anjeyelf, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [dupeme?])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.0; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121024073032

Steps to reproduce:

Windows vista.
Thunderbird 17.0
Selecting a file to auto save attachments.
Tools > Options > attachments > Incoming Tab
select : Save files to
click on 'browse'
choose file and click on 'Select file'
Select email and open the attachment.


Actual results:

After:
click on 'browse'
choose file so it is displayed in the 'folder' text box line and click on 'Select file'
The filename selected is incorrect:
C:\Users\User\Desktop\test doc dump\test doc dump

if I do not select a folder to display in the 'folder' text box then the filename displayed in the 'Incoming tab' is correct.

however, when I open the attachment it is not save to the selected location.
It is saved to \Appdata\local\temp.

I also tried: closing and reopening Thunderbird to see if this helped. It did not.

this is in Config Editor:
browser.download.dir;C:\Users\User\Desktop\test doc dump
browser.download.downloadDir;C:\Users\User\Desktop\test doc dump
browser.download.folderList;2
browser.download.lastDir;C:\Users\User\Desktop\test doc dump
editor.lastFileLocation.image;C:\Users\User\Desktop\test doc dump

checked..nothing has the appdata\local\temp





Expected results:

I would expect to select the folder name and it should display in the 'folder' text box.
I would expect this to then display correctly in the Incoming tab.
when I open an attachment I would expect the attachment to be save in the correct folder as shown in the incoming tab.

Attachment of images provided.
Sorry forgot to mention that the correct folder should be;

if I do not select a folder to display in the 'folder' text box then the filename displayed in the 'Incoming tab' is correct.
C:\Users\User\Desktop\test doc dump
duplicate of bug 633182 or bug 92146
(In reply to Wayne Mery (:wsmwk) from comment #2)
> duplicate of bug 633182 or bug 92146

anje, what do you think?
Flags: needinfo?(anjeyelf)
Whiteboard: [dupeme?]
I'm not certain this is completely the same.

email received with one attachment.

first there is some display error when selecting folder on where to save.
Will attach an image to show more clearly.

These are the confif entries showing the selected folder  -

browser.download.dir;C:\Users\User\Desktop\test doc dump
browser.download.downloadDir;C:\Users\User\Desktop\test doc dump
browser.download.folderList;2
browser.download.lastDir;C:\Users\User\Desktop\test doc dump
editor.lastFileLocation.image;C:\Users\User\Desktop\test doc dump

However, dispite the fact that I have selected the correct place where to auto save attachments.
when I select one attachment and click on 'Save' button located bottom right.

The 'Save files to' which I previously set, is not used.
Instead the window opens on a different file location.
C:\Users\User\Photos\Family\Family Name\Ancestors

In the Config Editor, I searched to locate any reference to this file location.
this is the only line I could find.

messenger.save.dir;  Value = C:\Users\User\Photos\Family\Family Name\Ancestors

So when I select to Save one attachment, it seems to be using the entry saved in:
messenger.save.dir;

which is incorrect.
Flags: needinfo?(anjeyelf)
Images shows the two methods of selecting folder which in one method does not display correctly.
(In reply to Wayne Mery (:wsmwk) from comment #3)
> (In reply to Wayne Mery (:wsmwk) from comment #2)
> > duplicate of bug 633182 or bug 92146
> 
> anje, what do you think?

Bug 633182 - does not express the same issues.
they ask for the ability to save attachments with a different filename from the actual attached filename, so that it shows which email sent the attachment.
I can see their point, but I do not want this.
My bug has nothing to do with the attachment filename itself.

bug 92146 - - does not express the same issues.
they would like to see an option where in a folder, they select several emails which all have attachments and then extract all attachments from all emails in one go into a specified folder. Currently, they can save all attachments only for one, currently viewed message. 

My bug has nothing to do with wanting the functionality of selecting multiple emails, nor extracting multiple attachments.
Poor Anje, you're absolutely right. Saving attachments to central location is totally broken in many ways, and I've commented on this elsewhere although I don't think all the bugs have been filed.

At a quick glance, here's a start:

1) A normal bug here:

From Options, changing the central location of "Save file to" folder setting (browser.download.dir pref) is broken. As my current download dir, options show:

Save files to: [# Desktop] [Browse...]

Clicking on "Browse" shows an entirely unrelated folder (not Desktop), and I doubt I've ever set that as my central download location. However, browser.download.dir pref in config editor shows that other location too (not Desktop).

2) A big design problem here:

Setting the central download folder for attachments (as opposed to: "Always ask me where to save files") is almost completely inconsequential. In fact, I didn't even know I have central download folder active, as it's never used whatever you do in the UI. You have to change a lot of settings in weird ways or otherwise try really, really hard to make use of that folder from the main attachment UI, which kinda breaks the whole concept of pre-defining a central download folder. To begin with, for user who have central download folder defined, "Save all attachments" on attachment header bar should just save to that folder without asking. Otherwise, what's the point of pre-defined target folder? "Save as..." should then be optional in the dropdown. For other users, vice versa. It's not all that easy to get the UI right without too much clutter, but there's a lot of room for improvement in this corner.
(In reply to Thomas D. from comment #7)
> Poor Anje, you're absolutely right. Saving attachments to central location
> is totally broken in many ways, and I've commented on this elsewhere
> although I don't think all the bugs have been filed.
> 
> At a quick glance, here's a start:
> 
> 1) A normal bug here:
> 
> From Options, changing the central location of "Save file to" folder setting
> (browser.download.dir pref) is broken. As my current download dir, options
> show:
> 
> Save files to: [# Desktop] [Browse...]
> 
> Clicking on "Browse" shows an entirely unrelated folder (not Desktop), and I
> doubt I've ever set that as my central download location. However,
> browser.download.dir pref in config editor shows that other location too
> (not Desktop).

Problem 1) seems volatile, changing folders updated everything and seems stable, but I don't have time for systematic research. But even for a once-only, there should never be discrepancy between download.dir shown in options and the actual download.dir.
Summary: auto save attachments → Setting a pre-defined download folder for attachments is inconsequential - almost never used in attachments UI (Attachment options > Save files to [folder] aka Browser.download.dir pref)
Plus, due to bug 408647 and several other bugs in this corner, the entire attachment download experience out of the box is really bad - think of saving your attachments into your specific target folder, TB will save them there but there's no link to get into the folder - so most users will just open their file managers and navigate to the very same folder again. Grrr. For that one, Bug 907732 would be a giant step forward but TB volunteer manpower isn't made for giant steps.
(In reply to Thomas D. from comment #7)

> 1) A normal bug here:

We probably need a new bug for the central download folder UI mismatch problem in options as mentioned in comment 7.

> 2) A big design problem here:
> 
> Setting the central download folder for attachments (as opposed to: "Always
> ask me where to save files") is almost completely inconsequential.

I dwelled a bit more on that in Bug 539198, of which this might be a dupe.
Perhaps we need to break up the general design problem into smaller, actionable bugs.
See Also: → 539198
See Also: → 549719
Hi,

While I was trying to fix the silent failure of TB when we try to save an attachment to
a write-protected folder for the last few years (it is more or less fixed now. If not, please let us know),
I came to realize that there was two code paths that
were invoked when we "save" attachments
1. - by clicking on the attachment, and
2. - by clicking on the save button near the lower corner of the mail body text pane (or 
     context-menu shown by the right-click on the attachment).

Bug 549719
 Unify/Merge two code paths for saving an attachment in a message (different save directory in "double click/save file" vs "context menu/save as") 

See especially, comment 17: https://bugzilla.mozilla.org/show_bug.cgi?id=549719#c17
 for details of the problems (as of May 2010).

But note that files mentioned there especially nsHelperDlg.js has changed much since then.
That said, however, as I look it up again, the problem seems to be still there.

[Note that I am not that familiar with the attachment handling in IMAP case. My experience is
purely with POP3.]

Now,  there is a problem because
in one of the code paths (I think the case 1 above) picked up the
default folder for the browser for saving (aka downloading), 
which is NOT quite correct for a TB usage IMHO.
(But then today I realize there is NO permanent TB-specific save location in user preference.)
 
I am not sure what the UI for setting the default is doing, but
I think there is a confusion of which default save folder (for browser, or mail client)
is used for
setting (config editor)
and 
during saving (the real save operation)

I have thought the problem somehow disappeared long time ago, but I probably was not
that keen on this aspect of the problem.


Let us see the current code, for example.:
(the case) this is used for saving each attachment by clicking, I think.

http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/mozapps/downloads/nsHelperAppDlg.js#96
96 const PREF_BD_USEDOWNLOADDIR = "browser.download.useDownloadDir"

http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/mozapps/downloads/nsHelperAppDlg.js#231
231           autodownload = prefs.getBoolPref(PREF_BD_USEDOWNLOADDIR);

So, browser.download.useDownloadDir is still used for the downloading (when user does not specify explicitly  the target folder.)

Remember, we are in TB, but still refer to "browser.download.useDownloadDir".

But this code is *shared* between TB, Firefox, and Spidermonkey, and what have you there.
So we need some indicator of within WHAT application this code is used (!) so that
we can pick up the proper user setting.

Q-1: Is there such global flag or function that returns the application name within JavaScript?


Whereas, the code that handles context menu (Right-clicking path), and I *think* this is also the path used by hitting "save" button on the lower right corner is like this: 
mailnews/base/src/nsMessenger.cpp

http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMessenger.cpp#105

105 #define MESSENGER_SAVE_DIR_PREF_NAME "messenger.save.dir"

Note the different variable name, "messenger.save.dir".

2020 nsresult
2021 nsMessenger::GetLastSaveDirectory(nsIFile **aLastSaveDir)
2022 {
2023   nsresult rv;
2024   nsCOMPtr<nsIPrefBranch> prefBranch = do_GetService(NS_PREFSERVICE_CONTRACTID, &rv);
2025   NS_ENSURE_SUCCESS(rv,rv);
2026 
2027   // this can fail, and it will, on the first time we call it, as there is no default for this pref.
2028   nsCOMPtr <nsIFile> localFile;
2029   rv = prefBranch->GetComplexValue(MESSENGER_SAVE_DIR_PREF_NAME, NS_GET_IID(nsIFile), getter_AddRefs(localFile));
2030   if (NS_SUCCEEDED(rv)) {
2031     NS_IF_ADDREF(*aLastSaveDir = localFile);
2032   }
2033   return rv;
2034 }

Well, from the comment above, it seems worse than I originally thought.
There is no default for "messenger.save.dir". (There is no such user configurable setting, I think it means.)
My reading of the code is as follows:
On the first try, it will prompt for the directory and message file, and
subsequently, the directory portion of the last directory is remembered and used.
(But it would disappear once TB is terminated and started from scratch??? I am not sure about this, but
re-reading suggests it is the case.).

The first case handled by the JavaScript coede
would remember the default position permanently as per user setting.

From: https://bugzilla.mozilla.org/show_bug.cgi?id=539198#c7
>E.g., why doesn't the save button use that setting?

Because the code does NOT as it turns out!

Any thought about how to fix this split personality?

I would go for a user setting in config file, messenger.save.dir, and use that in nsMessenger.cpp path,
and  also use it in nsHelperDlg.js by
selecting the appropriate user config variable 
according to which application the code path is invoked.

NO, no, no. come to think of it, these days, TB has a built-in web browsing capability.
I have no idea what happens when we want to download something from URL within the web page
shown in a tab within TB (ugh.) So we may need to even specify explicitly which
download directory we may want to use.
Oh well, then, I think fixing the nsMessenger.cpp to use
the default directory of "browser.download.useDownloadDir" seems much easier option.

Again, any suggestions?

Oh, there is one more thing in the original post.

>however, when I open the attachment it is not save to the selected location.
>It is saved to \Appdata\local\temp.

For unknown reasons, this is the way how opening an attachment by application has been done.
(I think other mailers under windows seem to follow suit.)
An attachment is saved to a TEMP directory when it is "opened" by an application 
instead of explicit "Saving".

I suspect that this behavior is meant for quickly checking the content of attachment, and if it is
not important to work on later, saving is not done explicitly by the user(and presumably the temporary file saved in TEMP directory will be deleted eventually outside TB.)
Which, of course, sometimes cause the user to have an illusion on working on an explicitly saved copy,
and after the user's modifying it and later not finding it in regular working directory,
the user searches for the modified desperately under wrong directories. Only searching for a file 
modified/created very lately will help him/her.

Come to think of it, we *MAY* want to have an option for someone like the original poster to
want to save to a user-specified directory even for opening the attachment temporarily, but I digress.
This is another bugzilla entry. 
If the original poster still follows this bugzilla, he/she may want to request this feature.
OTOH,  if the saving of an attachment to a temporary folder to be opened by an application
is recorded in save directory listing, that may certainly help and we may not need such a user preference.
But recording this in the save directory listing is beyond my skill. Anyone?
Duplicate of this bug: 480899
See Also: → 480899
See Also: 480899
Duplicate of this bug: 1433753
You need to log in before you can comment on or make changes to this bug.