78.17 KB, image/png
36.21 KB, image/png
188.39 KB, patch
Magnus Melin: ui-review+
|Details | Diff | Splinter Review|
98.12 KB, image/jpeg
Created attachment 330461 [details] [diff] [review] proposed fix Following the discussion in bug 286094 comment 30 (and some before), the Thunderbird Attachments Folder options are really confusing. "That "Save As..." and "Save All..." are not influenced by the preference "Save all attachments to this folder:" is perfectly fine, since those commands throw a dialog at you in which you are asked where to save the attachment files. That's not a bug. If you merely do a "Save", e.g. by double-clicking the attachment symbol at the bottom of the E-mail message and then choose option "Save to disk", only then the preference matters. The problem I have been describing has rather to do with the poor formulation of the preference, which leads easily to misunderstandings." --- This patch tries to clarify what this option really does. It's all a little messy to try fit this in... Also fixes the related Choose folder dialog title.
If above attachment is a proposed fix, I still have problems with it. Putting myself in the footsteps of an ordinary user, I have the following question looking at this dialog window: What is the "'Save File' option"? An attachment preference? A global one? Probably. Then you expect this to be part of handling attachments, right? Thus you expect also all options of TB concerning attachments to be handled by this tab, not by any other. So where is the "'Save File' option" on this tab defined? Nowhere. Moreover, 'Save File' is less clear than my proposed 'When saving attachments to disk'. Now, I admit it would be good to keep texts short. Studying this dialog window I offer several improvements, which just occurred to me: First, it might be preferable to use word 'action' instead of 'option'. Then the texts should use the exact same wording as in the action settings dialog. TB uses now 'Save to disk'. That's why I have also used this term in my first suggestion. Furthermore, it might be very helpful to reverse the sequence in this window. First define "Download Actions" and only second preference for "Attachments Folder". Because the attachments folder is only of relevance for certain actions and is not global (as I understand the behavior of TB). Finally I suggest to avoid the word file. We talk about attachments in this tab. Thus 'Always ask me where to save the file' becomes 'Always ask me where to save attachments' and 'Save the file to' becomes 'Save attachments to' (also remember, this pref applies to single and multiple attachments). The dialogs from top to bottom would then read: "Download Actions" "Shredder can automatically save or open attachments of certain types." Button "View & Edit Actions..." "Attachments Folder" "When saving attachments by 'Save to Disk' action" "Always ask me where to save attachments" "Save attachments to" <folder field> button "Browse..." I believe this would all be much clearer and lead to much less confusion of users. It took me, an experienced programmer, years before I understood what the programmers of TB had in mind. A large reason for that having been only this confusing dialog window. Plese consider my suggestion. Andreas
"action" might be better than "option", yes. At least on trunk the options in the save file dialog are "Open with " [ ] and "Save File", not "Save to Disk" - that's why I used it. I agree Download Actions could be more logical to have on top.
Created attachment 334073 [details] [diff] [review] proposed fix, v2 Better fix. The idea is that you read it from top. First we explain that there tb can handle attachments by open or save, and then, for the save action get the folder option.
Excellent this proposed fix, except for two small points: 1) I understand that the context menu for attachments has a "Save All..." command. My understanding was that a "Save File" action preference would also affect the "Save All..." command and would all save in the specified folder. Therefore the plural would be indicated, i.e. "attachments", perhaps "attachment(s)" instead of "the attachment". That was my thinking behind my proposal for this dialog. 2) Moreover, why again talking about a "Save File" action? Do you rename throughout the "Save to disk" action to "Save File"? If not, please use the exact same wording as in the action specification or users won't understand that you talk about the same thing. All following the general rule for clarity: "Always use exactly the same wording for the same thing!"
1) It only affects what happens in the dialog you (may) get when you double click and attachment. Save All... context menu is not involved. 2) The dialog has two options, "Open with " [ ] and "Save File". I use that wording exactly because it should be the same wording for the same thing.
I am not sure we talk really about the same thing. Have you now changed all wording in all dialogs? Since in the current version of TB (version 220.127.116.11 (20080707)) the dialog what to do with an attachment uses the wording 'Save to disk' and the edit action dialog also uses the wording 'Save to disk'.
Created attachment 334086 [details] Current opening an attachment dialog uses 'Save to Disk' wording The dialog TB version 18.104.22.168 (20080707) throws at you if you have double-clicked an attachment of a certain type and TB does not know what to do with it unless you make a choice or edit TB's actions for that type of attachment.
Created attachment 334087 [details] Action editing dialog (clicking button "View & Edit Actions") - Preferences tab Attachments) The preference dialog of TB version 22.214.171.124 (20080707) visible if you wish to edit actions TB should take while handling attachments. It also uses the wording 'Save to disk' not 'Save File'.
We talk about the same thing, but that wording has already changed since tb2 (compare the dialog in firefox3).
Comment on attachment 334073 [details] [diff] [review] proposed fix, v2 r=me in terms of the code, lovely as always. I'll leave the UI and wordsmithing parts to clarkbw.
Comment on attachment 334073 [details] [diff] [review] proposed fix, v2 A couple of string changes and it looks good to me. Attachment Handling Actions $TB can automatically open attachments of certain types Attachment Saving When saving an attachment (o) Always ask me where to save attachments ( ) Save attachments to: Let me know if that looks good to you and then I think we're good to go.
I like the "Attachment Saving" header, but the other changes would still make it unclear that it's only about when attachments are double clicked - not if you right-click and do save/save all. > $TB can automatically open attachments of certain types Thunderbird can also *save* automatically when you double click. That's what makes all of this so confusing. The file handling dialog UI isn't really very suited for mail usage... but I guess we're a bit stuck with it :(
Therfore the following might be a preferable wording (note the "open or save" as I've suggested in Comment #2): Attachment Handling Actions On Double-Click $TB can automatically open or save attachments of certain types Attachment Saving When saving an attachment (o) Always ask me where to save attachments ( ) Save attachments to:
Well, no, I said double click, but obviously you get the same effect by "Open" in the context menu, hitting enter when the attachment is selected and so on.
Ok, then: Attachment Handling Actions On Double-Click/Open $TB can automatically open or save attachments of certain types Attachment Saving When saving an attachment (o) Always ask me where to save attachments ( ) Save attachments to:
andreas: i think it's the second part (about saving) that needs clarification, not the first one. Bryan: see comment 14 ->
Ah, right I suppose it is still awkward. If we stick with what you had originally for the handling section and keep the updated saving section then ui-r+ can carry over with those changes.
Magnus: I know that it is the second one that needs clarification. That's why I proposed to change the first one. The second one can then be better understood. Remember, the 2nd one is a subsection or specification subordinated under the first one. It specifies only what needs to be done in case of certain actions. My proposed changes for the 1st part are meant to help clarifying which actions can be controlled (Save as, Save all are not affected). 2nd part specify preferences for the save to disk actions. Hope this explains where I'm coming from. Andreas
Created attachment 351931 [details] propose fix v3 (screenshot) I think were we got lost here is that there is no reason to have it under two different headers. Keeping it under one makes it much more clear that the attachment folder is about what happens in the actions. Also make it a bit more consistent with firefox. What you do think?
Sorry, I don't think I got lost here. I still believe my additional text helps to understand this complex behavior. Therefore I'm not quite happy with your design and I still propose: Attachment Handling Actions On double-click or open: <View & Edit Actions> When saving an attachment: ( o ) Always ask me where to save attachments ( ) Save attachments to [ Desktop ] <Browse...> That's clearly my preference to make it really clear. Why repeating a mistake from FireFox?
Created attachment 352629 [details] attachment prefs mockup Just as a quick change of POV I did this mockup to see what it would look like if we took even more of the current firefox system and used that for Thunderbird's attachment handling. I kind of like the way this ends up looking.
Yeah that's one more part we need to port from firefox. Hm, perhaps the Attachment Actions (Applications) first, and where to save after though. To signify that that's where the folder comes in.
Is the plan now to keep Attachments as a main tab? Or is it moving somewhere?
I don't have a plan to move it anywhere. Though the current amount of preferences doesn't really warrant an entire tab IMO; unless we were to flatten out the options in the way FF has done.
This is in response to comment #23 and the discussion following from there. Frankly speaking, I am flabbergasted how this discussion is going. I have to say that I fear it is going in a very bad direction, given first all what we have discussed so far and secondly given that this issue relates to other issues users complain about since years (see also bug 286094, bug 446291, bug 374184, bug 243975, bug 238789). I was hoping we would now finally have a chance to solve this problem for good, but I'm afraid with what you suggest under comment #23, all is going only backwards. The good way to handle this is the carefully designed dialog structure I provided under comment #22. AFAI can see anything else is confusing and invites again the same problems we are plagued with since years. If FF is making the same mistakes, this is no reason to copy them over to TB. BTW I have also already provided a graphic of the preference dialog, which someone has deleted for reasons I did not get. The issue has been extensively discussed and I wish not to repeat the entire argument (see allegedly resolved bug 286094 and bug 446291). It is all well documented here and in the bugs listed above. What is important now, I guess, is to remember that TB knows no folder for permanent attachments, and that the so-called attachment folder serves only the storing of temporary copies of the attachment files. Such temporary files are only involved in some of the actions, notably 'save to disk' action. Therefore the dialog must make clear the relationship of subordination of attachment folder being used only for some actions. However, the design Bryan suggests under comment #23 invites the idea of permanent attachment storing. The latter confuses people (it took me as an experienced programmer months to understand that) and can raise many issues (see bugs listed above). For all these reason we are not free to list the dialog items in any sequence. The dialogs have to respect the hierarchy: First come the actions. Only if actions such as 'save to disk' are used do we need a folder. That folder stores only temporary files. Moreover, the latter means also this folder MUST NOT default to the desktop. Instead under Mac OS X its default is best set to /private/tmp (see also comment #26 under allegedly resolved bug 286094). PLEEEEAAAASE consider these aspects and I beg you to implement the dialog as I have suggested under comment #22. Thanks for your help and cooperation.
Comment on attachment 351931 [details] propose fix v3 (screenshot) Giving this a plus because it's a good step forward. However I think the longer term plan should be along the lines of attachment 352629 [details], the FF model.
Just wanted to note that bug 480899 is looking at how this pref actually works. Apparently part of it never has.
Created attachment 379595 [details] [diff] [review] proposed fix, v4 - WIP WIP, i'm using this bug to port the firefox helper applications prefs then. This patch works mostly I think, at least on linux. It's almost completely copying over what firefox has and making it work (originally implemented in ff bug 377784, sm bug 417590). Still to do, at least o rip out the feed preview / live bookmark stuff o add (back) the download folder pref to this pane
(aside: it'd be nice if there was a way to get to the "download manager" window from the menus, as in firefox -- probably a different bug, though)
A different bug named "bug 450861".
(In reply to comment #31) > This patch works mostly I think, at least on linux. It's almost completely > copying over what firefox has and making it work (originally implemented in ff > bug 377784, sm bug 417590). Still to do, at least > o rip out the feed preview / live bookmark stuff > o add (back) the download folder pref to this pane I tried this out on Linux yesterday and it seems to work pretty well, I'll try to give it a go on Windows when I get a chance.
Created attachment 380628 [details] [diff] [review] proposed fix, v5 Out with the old, in with the new. This should be it. I incorporated a few of the sm changes but not all. Thinking about future development changes are more likely to come from ff changes so I'm not to keen on changing stuff that's not wrong but a matter of opinion. I didn't have many types in my list, you can see all the plugins too if you set browser.download.show_plugins_in_list but I think that should be false for tb. Other notes: - the doxygen tag is @return, not @returns - ff doesn't use saveFile.png for pinstripe (in applications.css) - i made tb use it, not sure that's wanted or not Bryan: not sure we need the grouping (screenshot coming up)
This can go in as a later bug if you want, but what do you think about better names for http & https "content types", like "Web pages" and "Web pages (https)".
Created attachment 380631 [details] [diff] [review] proposed fix, v5.1 Applies to hg tip.
davida: yeah would be good with better protocol handler descriptions
Comment on attachment 380631 [details] [diff] [review] proposed fix, v5.1 Can you add the same thin separator below the richlist that exists above it. The padding looks off right now between the list and the radio button group. With that change this looks good to me.
Created attachment 380881 [details] [diff] [review] proposed fix, v5.2 Thin separator added. Carrying fwd ui-r=clarkbw
I wish to ask here the programmers, whether they have considered ALL what was stated on this bug. In particular, do you now honor the difference between temporary and permanent storage? I have to admit I doubt it, given the dialog as it seems to be proposed now (proposed fix v5 https://bug446291.bugzilla.mozilla.org/attachment.cgi?id=380629). Programmers, please read bug 311292, comment 55 (https://bugzilla.mozilla.org/show_bug.cgi?id=311292#c55). Is this issue resolved with what you propose now? I doubt, since I see no preference to set and reset the temporary storage location in your fix. I get the impression we will have to continue to use my workaround with what you are implementing now, isn't it? Programmers, are you aware that the concept to resolve this bug and many others have been provided by me under bug 311292, comment 109 (https://bugzilla.mozilla.org/show_bug.cgi?id=311292#c109)? Have you read this proposal and made good use of it while attempting to resolve this bug 446291 and many others? Be aware that my solution is resolving all related bugs, i.e. bug 286094, bug 446291, bug 374184, bug 243975, bug 238789, and bug 311292 at the same time! Please make sure you understood what I wrote, otherwise I fear we will make little or perhaps even no progress at all. And the issue is more crucial than you might think (given the many complaints since years, see many users comments in above bugs). If we fail here again, I fear it will become a clear indication that open source is not working. Would be a pity. Andreas
Bug 311292 and the other issues related to mac tmp folder doesn't really have anything to do with this bug. (Yes, i'm sure bug 311292 is a pita on the mac.)
Created attachment 383504 [details] Attachments preference dialog proposal for Mac by Andreas Fischlin Proposed fix for this bug (see comment 22 and 28 for explanations, but consider also comment 43 and the there listed related bugs)
Sorry, can't agree at all. Have you really read my solution as I propose it under bug 311292, comment 109? If you implement all what I write there, this bug and all the others I listed in comment #43 are solved in one go and the implementation is not even much work. To see the connection between this bug 446291 and the others I mention, please read again my proposal how to structure the dialog box (comment 22) and the issues behind this dialog box (comment 28). I believe my argumentation under comment 22 makes this connection clear. To help as much as possible I attach once again my proposal as attached (comment 45). I am a very experienced programmer myself and believe having provided an elegant and very carefully designed solution (bug 311292, comment 109) that resolves this issue we debate here for good. Please note also, some contributors start considering this debate to become absurd (bug 311292, comment 132). I'm afraid I feel I have soon to agree, which would be a pity, given the popularity of FF and TB.
Created attachment 383512 [details] Attachments preference dialog proposal for Mac by Andreas Fischlin Proposed fix for this bug (see comment 22 and 28 for explanations, but consider also comment 43 and the there listed related bugs, in particular bug 311292, comment 109, 4th last item starting with "- What is needed in addition to above improvement with preference setting dialogs is the following: TB (as well as FF) should introduce the control for the 2nd preference setting, i.e. the location for temporary storage. ...")
Sorry for having first attached the wrong old dialog box I prepared a long time ago. The new one is the one you find under comment #48. This is the preference dialog I actually propose (based on bug 311292, comment 109 explanations) and which should now make the connection between this bug 446291 and all the other bugs I listed under comment 43 very clear. For Magnus: The actual setting for radio button "Default system location" should be set using environment variable TMPDIR (bug 311292, comment 80 and some thereafter). I guess also http://lists.apple.com/archives/darwin-dev/2008/Oct/msg00063.html and http://lists.apple.com/archives/darwin-dev/2008/Oct/msg00067.html may be helpful in this context.
To repeat myself: this bug has nothing to do with bug 311292. This is only about the UI change. What you want to do is all backend changes, shared with firefox.
Sorry Magnus, to repeat myself too: It has lots to do with each other. Have you looked at the dialog box I have provided (comment 48)? That makes it clear, doesn't it? Or where do you think would be a better place to offer the preference control where to store temporary attachments you merely open? Or where do you see you pick up my argument that users do not understand what TB is doing with attachments, because the sequence in the dialog is not as I propose (this is relevant irrespective of the control over temporary storage location preference)? Or why do you think the text given in the dialog as you have proposed latest AFAIK should be not misleading in the way I described in several comments (comment 22 and later)? And once more, sorry having to repeat myself, have you read bug 311292, comment 109? You have never referred to any of the arguments I am making there, thus I do not see whether you see a flaw in my argumentation or not. Is this the case, then please tell me or I have not the slightest idea why you disagree with me. BTW, this argument has all to do with user models. Good software follows an easy to understand user model and supports the user hereby in using the software. The underlying implementation, the programing etc. needs only to follow the user model strictly but in any other respect, it's merely the programer's business, not the user's. TB and FF would profit from agreeing FIRST on the user model with respect to the handling of attachments, THEN to implement it. The UI has to present clearly the user model, this is currently not the case. Then the implementation has to support the user model, nothing else. This all means you can never separate an UI issue from the underlying algorithm. My suspicion is that this is precisely what is not happening and therefore we have these seemingly never ending debates. My arguments have lots to do with software engineering that increases the probability of high software quality. I hope that these explanations help you to better understand where I am coming from.
Sorry, but I don't understand this in v5.2: +# LOCALIZATION NOTE (usePluginIn): +# %1$S = plugin name (for example "QuickTime Plugin-in 7.2") +# %2$S = brandShortName from brand.properties (for example "Minefield") +usePluginIn=Use %S (in %S) + +# LOCALIZATION NOTE (typeDescriptionWithType): +# %1$S = type description (for example "Portable Document Format") +# %2$S = type (for example "application/pdf") +typeDescriptionWithType=%S (%S) In the LOCALIZATION NOTE a %1$S and a %2$S is mentioned, but there is only a %S used in the string.
It refers to the first and second arguments. I guess it's better to use the %1$S form since it let's localizers define the order. I'll attach a new patch.
(In reply to comment #51) You're missing the point. There's no backend support for what you want to do (or well, sure, on some level it's of course doable). It's not about clarity of UI, I just can't/won't add UI when there's no backend support (=bug 311292 isn't fixed).
Created attachment 384085 [details] [diff] [review] proposed fix, v5.4 unbitrotted and improved the localization issue noted above
In reply to comment #54 Please let's not argue in such a way. As I explained in bug #311292, comment #109, there must be some backend available: TB distinguishes already the two preferences and already supports the functionality fully. The only thing missing is the control over the UI to that functionality (as I've explained in bug #311292, comment #109). Admitted, I do not know the source of TB (nor FF) and I do not plan to study it. Therefore you might have a point if you are talking about missing algorithms to maintain the UI and linking that functionality with preference control. But I argue that there is plenty of proof for my arguments that the functionality is there, or the fix I propose for bug #311292, comment #55, would not work.
from Bug 499551: please regard URL scheme preferences (e.g. for AIM, see adreess book) can't be changed via preferences atm!!!! Clicking ScreenName link in adress book, a third variant of this dialog appears!!! Nice to see: - ability to _add_ entries without opening an attachment - ability to change what to do clicking "mailto" links (new mail in Tab, new mail in new window, ...)
Created attachment 384524 [details] Screenshot for v5.4 on Mac OS X On Mac OS X it looks like this if you use patch v5.4. But there is something curious. In the en-us build it works as desired. But in a localized (german) build I don't see the content types I see in the en-us build. And I couldn't find out how to add new content types and their action. First I was searching for an option like "Add new content type and an action", but couldn't find one. Than I've tried to add a new content type by dragging an pdf file into the field, doesn't work. Than I've opened a mail with pdfs in the attachment, selected Adobes pdf Reader to open it and marked "Do this automatically for files like this from now on" (but in german). Than I've seen a message that says me "Settings can be changed using the Attachments tab in Thunderbird's Preferences" (but in german) and I've clicked OK. So I switched hopeful back to the attachments tab, but there was no new content type (also not after a restart of Thunderbird). Why this? Did I something wrong?
Anything of interest in the error console? (for the german build)
(In reply to comment #60) > Anything of interest in the error console? (for the german build) Yes (hopefully): Fehler: [Exception... "Component returned failure code: 0x80470006 (NS_BASE_STREAM_BAD_CONVERSION) [nsIStringBundle.GetStringFromName]" nsresult: "0x80470006 (NS_BASE_STREAM_BAD_CONVERSION)" location: "JS frame :: XStringBundle :: getString :: line 17" data: no] Quelldatei: chrome://global/content/bindings/preferences.xml Zeile: 406 Fehler: uncaught exception: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIStringBundle.formatStringFromName]" nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)" location: "JS frame :: XStringBundle :: getFormattedString :: line 33" data: no] And I only see this in the german build and I see this everytime I switch to the attachments tab.
Sounds like some missing localization key/s for that build then. (Not sure where you get a german build to begin with, as this hasn't landed yet.)
(In reply to comment #62) > Sounds like some missing localization key/s for that build then. (Not sure > where you get a german build to begin with, as this hasn't landed yet.) Hm, I've made the localized files by my own and double checked it that there is no missing string. And normally if there is a string missing I get an error like "missing entity xy". But OK, maybe this was an fault from my side. I let you know if I will see the errors also in the official (german) version after your patch checked in...
> (comment #31) It's almost completely copying over what firefox has and > making it work (originally implemented in ff bug 377784, sm bug 417590). If this follows the SM implementation (in turn based on the FF dialog), this would also inherit the issues described in bug 462629, correct? Thus, it would not be possible to identify or edit MIME-type and file-extension associations with that dialog (unless I've missed some detail reading the comments here).
This landed last night, and the Win32 nightly build 20090629032928 shows indeed the known behavior that wrong MIME-type/file-extension definitions can be easily picked up when opening an attachment with incorrect definition. It does not seem possible to remove such an entry, though at least the MIME type is shown along with the description which helps somewhat to identify those.
changeset: 2965:76a9c718b6fa http://hg.mozilla.org/comm-central/rev/76a9c718b6fa ->FIXED
I have filed bug 501163 to follow up on comment #58 (adding an entry) and comment #64 (verification, editing, removing an entry). It would be good to have those cases covered as well, given the frequent MIME-type reports.