182274 - [IMAP] Need UI for server trash folder name

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Ere Maijala (slow)]

In bug 24064 a back end was created for specifying the trash folder name on an
IMAP server. This needs a front end in the preferences.

Comment 1

[Asa Dotzler [:asa]]

Please do not set + on flags unless you are a driver. If you would like to
nominate the bug use ?.

Comment 2

[R.K.Aa.]

*** Bug 185494 has been marked as a duplicate of this bug. ***

Comment 3

[Daniel Küstner]

From bug 185494:

> I propose to do this as a selectable list in Server Settings -> Advanced.

Comment 4

[Ronald van Kuijk]

I started hacking on the gui (xul/js) a little and I have an initial version
here. The only thing is, it does not store anything in the prefs.js. In examples
I examined (e.g. prefs in the same xul/js file) I do not see anything special in
the storing of preferences. Does something need to be implemented in the C code
as well?

Does somebody have a link to information on how to store preferences in mozilla?

Comment 5

[Henry Jia]

Ronald,

You may refer to nsPref & nsPrefBranch for store preference settings.

http://lxr.mozilla.org/seamonkey/source/modules/libpref/src/nsPref.cpp
http://lxr.mozilla.org/seamonkey/source/modules/libpref/src/nsPrefBranch.cpp

Good luck.

Henry

Comment 6

[timeless]

look at the preference panel xul files, there are magic attributes which do the
work for average cases.

Comment 7

[Ronald van Kuijk]

I got a first version working. At the moment it is only a textbox where you can
enter the name u want. The preference is stored and if that folder does not
exist it is created (great thing is that I did not have to do anything for
that...cudos to mozilla-developers)

There seem to be some problems however with refreshing the folderpane (as is the
case with the bcc, drafts folders if you change them). E.g. sometimes two
folders are selected as "trash" for a while, or when you change a name, the
wrong one stays selected for a while.

Is there a way I can force this update? 

If I create a static menulist instead of a textbox, the active one is
automatically selected, if it is in the list.... again...cudos to
mozilla-developers.

Now I'll try to make a dynamic menulist with the folders of that server in it.
I've seen example-code in am-copies.xul/js, but are a bit in the dark if I need
those observers and broadcasters (and what they do, but that's my own problem ;-) )

Comment 8

[Ronald van Kuijk]

I think it should also be possible to block this preference, just like the
others (for customized installations). Who should make a decision on this?

Comment 9

[Ronald van Kuijk]

I've got a version working at the moment that uses a list of all folders that
are available. Unfortunately the back-end is not implemented using a uri
(imap://rtvkuijk@10.1.5.103/Sent items) but just the name of the folder (Sent
items). Furthermore the backend allows only the use of a folder on the imap
server that you are configuring, not on another one (hence no need for a full uri)

The preference manipulation is therefor not like that for other folders (drafts,
bcc etc) but needs additional checks like
- is the folder that is selected on the server that we are configuring. 
- Strip the foldername from the uri
- create a full uri from the plain foldername to have the right folder displayed.

I'm currently looking into hiding all but the current server in the folder
selection box, so no errors have to be displayed if a wrong server is selected

Plz assign this bug to me, I'll add some patches in a few days

The pref on the advanced tab of the imap server btw. If It should go into the
copies & folders menu please let me know asap, so I can still switch

Comment 10

[Henry Jia]

=> [e-mail]

Comment 11

[Ere Maijala (slow)]

The backend is actually done so that it will create the folder if it doesn't
exist. Therefore it's not necessary to check if the folder exists. And I think
Server settings is the right place for it. It's different from drafts and others
in that it's tied to the server functionality (for example delete model, empty
on exit). If it was a full URI, we would need to rethink the other settings too. 

Comment 12

[Ronald van Kuijk]

OK, one step further... current features:

- Uses the folder selection like drafts etc.
- If a folder on another server than the current one is selected the user get's
a warning and the current value is restored
- If a subfolder is selected, the user gets a response that only top-level
folders are usable as trash

Problems:
------------
- Selecting a folder which contains a folder (e.g. my current 'Deleted items' in
exchange contains a deleted folder) does not work yet
- I need to know how to construct a full uri from for the folderPicker.
Currently I do

imapServer.trashFolderUri =
document.getElementById("server.type").getAttribute("value")+"://"+document.getElementById("server.userName").value+"@"+document.getElementById("server.realHostName").value+"/"+imapServer.trashFolderName;

but the server.userName does not seem present, only the realUserName, but that
is not the one used in uri's. I do not take imaps or non-default ports into
account yet. I could just put the port behind it, but do not know if
imap://user@host:default-port/folder is the same as imap://user@host/folder. And
will imap:// become imaps:// if security is selected? Things to try out.

- Refreshing the foldertree is a real problem. On many occasions if you change
the folder, the old folder has its icon removed when you open and close the tree
, but the new folder does not get a changed icon....

enhancements
------------
The ability to create a new folder from the selection list could be interesting.
Currently not needed because there is a workaround. Just create a new folder in
the normal way and select it here.

Comment 13

[Ronald van Kuijk]

could somebody tell me what the difference between the
mail.server.serverX.username and mail.server.serverX.realuserName is? Both are
in my preference file, but only the realUsername can be changed through the
preferences. username however is the one that is used to construct uri's
(strange, because with that uid i cannot login to my server so the use of it is
purely cosmetic.)

Fixed:
- Selecting folders that contain subfolders (typo :-( )
- Returning correct error when selecting a subfolder

New problems:
- utf-8 encoded (i think) foldernames 

Comment 14

[Ronald van Kuijk]

Ok, on the issue of using username and realusername, as wel as hostname and
realhostname (the last one is not in my prefs since i never changed the name of
my mailhost) I found bug #131176. This bug explains it a little and references
bug #14295. 

According to this bug it is normal bevavior, but it is strange that old info on
user/host is kept and new prefs are added (why not add both initially?). There
is probably good reason for that, but it makes it difficult to construct a uri
(which is needed to select the current trash folder in the folder-picker).  

It would be nice if there was a method available like
GetUriFromFolderAndServer(folder, server) or GetUriFromFolderAndAccount(folder,
account).

I'm just getting to grips with XUL, and it could probably be retrieved from and
RDF source, but not by me (at least not for the moment).

Comment 15

[Ronald van Kuijk]

digging a little more into other files, especially am-copies.js/xul, I found
some functions that gave me the serverURI. So that problem is solved. 

In fact, I think it is all working fine now, I'll look into the patch tools and
create patches later this evening. 

Comment 16

[Steffen Wilberg]

*** Bug 211303 has been marked as a duplicate of this bug. ***

Comment 17

[Steffen Wilberg]

*** Bug 211303 has been marked as a duplicate of this bug. ***

Comment 18

[Wouter Van Hemel]

So... is there anybody working on this? It seemed as if we were close to a solution.

Comment 19

[Matthew (lilmatt) Willis]

I'm willing to begin work on this if it has stagnated. 
I'm wondering where the functionality should be found within Account Manager.

I see comments below stating it should be in "Server Settings" (note to self:
mozilla/mailnews/base/prefs/resources/content/am-server.xul).

I personally feel if it would be better to move the delete options and such to
"Copies & Folders" (note to self:
mozilla/mailnews/base/prefs/resources/content/am-copies.xul), since users are
already selecting folders for Drafts, Templates, and Sent Items there.

Thoughts before I begin mocking this up?

Comment 20

[Matthew (lilmatt) Willis]

Attachment: comp of UI for Trash folder selection

This is a first cut at a comp, putting controls, similar to the "Drafts" and
"Templates" folder selection, in "Copies & Folders" in the Account Manager.

After comping this up, my thoughts are that this UI should go beneath the "When
I delete a message:" UI under the "Server Settings". Then "Move it to the Trash
folder" could be changed to "Move it to a folder", which would un-grey the
folder selection UI, similar to what is used for Drafts and Templates.

I'm going to try comping that up in XUL now, since it makes more sense to keep
all the deleted messages stuff together, and hopefully it will fit. 

I'm not worrying about localization or wiring it in to the prefs yet. Once I
have a UI built that people (mscott) agree is acceptable, I'll do the rest.

Comment 21

[Matthew (lilmatt) Willis]

Attachment: comp 2 of UI for Trash folder selection

This is an XUL comp of the trash folder selection UI added under the "When I
delete a message UI". 

I'd appreciate any comments or suggestions on this comp.

Comment 22

[Wouter Van Hemel]

The first one, "Copies and Folders", feels more intuitive, defining it together
with the other special folders - you have all the path definitions of the
abstractions of logical functionality in one place.

The second one, "when I delete a message", is WHAT should be done with a message
one doesn't want anymore. It makes more sense to me to have it simply say "put
in Trash", for which the abstraction then can be specified in the "Copies and
Folders" screen. Doing it in one go, "put it in Trash AND Trash is this" makes
the GUI more complicated for people who don't necessary want to change any
folder settings themselves, especially since there is already a place to define
abstracted locations such as "Trash" and "Sent". It seems most logical to me to
have the GUI setting there.

Btw, I'm glad someone picks this bug up. I hope this can make it into 1.0, since
I think it's an oversight not to be able to configure (and localise) this folder
like the other ones.

Comment 23

[Matthew (lilmatt) Willis]

After pondering it, I think you're right. I'm going to build it like comp 1, and
if we don't like it there, well, we can move it, since it will be built.

IMHO, It's more important to have this in 1.0 than to not have it because we
couldn't figure out under which pane to put it.

Comment 24

[Tuukka Tolvanen (sp3000)]

*** Bug 222798 has been marked as a duplicate of this bug. ***

Comment 25

[Matthew (lilmatt) Willis]

Attachment: revised comp for Trash folder selection UI

I found what I feel is a more intuitive way to handle this.

The UI is only displayed for IMAP accounts, and the folder selection menulist
only ungreys when the "Move it to this folder" radio button is selected.

I've got most of it wired in and working. I am still working on getting data
out of the menulist. After that it should be ready as a first cut.

Comment 26

[Wouter Van Hemel]

(In reply to comment #25)
> I found what I feel is a more intuitive way to handle this.
> 

That does look intuitive.

I'm still a bit bugged about why the backend allows this only for imap accounts,
though. It's inconsistent. I would argue that the user should have the freedom
to set any folder names as s/he pleases, especially in regard to localisation.

Comment 27

[Matthew (lilmatt) Willis]

(In reply to comment #26)
> I'm still a bit bugged about why the backend allows this only for imap accounts,
> though. It's inconsistent. I would argue that the user should have the freedom
> to set any folder names as s/he pleases, especially in regard to localisation.

I would agree.

That being said, I can't do all the backend work necessary to add it, nor do I
understand fully the implications of enabling nameable trash folders to pop,
movemail... accounts. 

Maybe file a bug?

In the meantime, I'm doing a final build here before I diff up a first cut
patch. If all goes well, I should have it up in the next hour.

Comment 28

[Matthew (lilmatt) Willis]

Attachment: Initial cut at adding UI for IMAP trash folder selection

Ok, so it took me more than an hour, but here's the first cut.
It should work, but I'm not sure what all the edge cases are.

I plan to make a version of it for AVIARY as well if I have time tonight.

Comment 29

[Matthew (lilmatt) Willis]

I didn't end up with enough time, but it's just as well.
I'll hold off on making an Aviary patch until the code is finalized.

Comment 30

[David :Bienvenu]

does this allow you to pick a folder in an account other than the current
account? If so, does that work? I'd be surprised if it worked that well,
especially for deleting folders.

Comment 31

[Matthew (lilmatt) Willis]

(In reply to comment #30)
> does this allow you to pick a folder in an account other than the current
> account? 

It shouldn't allow picking except for the current account, unless I've botched
something in the file picker rdf part, which is entirely possible since that's
the part I understood the least about. 

Since "mail.server.%serverkey%.trash_folder_name" is only for IMAP accounts, and
it's tied to a particular serverkey, it should only let you choose from that
server's folders.

I'll check now and see if that's the case. If not, I'll need to take a closer
look at the rdf stuff.

Comment 32

[Tuukka Tolvanen (sp3000)]

*** Bug 225440 has been marked as a duplicate of this bug. ***

Comment 33

[Matthew (lilmatt) Willis]

Attachment: Patch adds UI for selecting trash folder for an IMAP account

Revised patch. 
Now limits folder choices to the selected account's folders and doesn't show
other accounts.

Comment 34

[Matthew (lilmatt) Willis]

Attachment: Screenshot of rev1 patch

In fixing the code to only let a user choose from the current IMAP server's
folder, I realized that there was no reason to add "in [e-mail]"
to the folder name in the dropdown. Therefore, I dropped it. It was easier to
code it that way anyway :)

This screenshot shows the new code, with the abbreviated folder dropdown. It
works well. On to reviews!

Comment 35

[David :Bienvenu]

thx for doing this...some comments:

are there tabs in your file?  selectImapDeleteModel(choice) looks like it picked
up some tabs.

re the localization of Trash:

+  if (!trashFolderName) {
+	var trashFolderName = "Trash"; // XXX localize this 


I'm trying to remember - do we display the trash folder with a localized name
for IMAP? My suspicion is that we don't, so we should not localize that in the
js until we do...

Comment 36

[Matthew (lilmatt) Willis]

Attachment: rev 2 - Patch addresses bienvenu's comments

You're right. I pulled the tabs out and removed the "// localise this"

Comment 37

[David :Bienvenu]

+  var trashFolderName =
document.getElementById("imap.trashFolderName").getAttribute("value");
+  // if the preference hasn't been set, set it to a sane default
+  if (!trashFolderName) {
+    var trashFolderName = "Trash";
+   
document.getElementById("imap.trashFolderName").setAttribute("value",trashFolderName);
+  }

shouldn't redeclare var trashFoldername here

+            <!-- XXX localize this -->
+            <radio id="modelMoveToTrash"


this is already localized, right?

Comment 38

[Matthew (lilmatt) Willis]

Attachment: rev3 - Patch addresses bienvenu's comments

Ok. Those things are fixed. 

I removed the variable redeclaration and the "localize this" comment. That was
originally just a reminder to myself to add and entity to the .dtd.

Comment 39

[Matthew (lilmatt) Willis]

Attachment: AVIARY version of rev3 patch

Now that the patch is finalized and r+, here's a version for AVIARY

Comment 40

[Stefan Borggraefe]

Comment on attachment 161414 [details] [diff] [review]
rev3 - Patch addresses bienvenu's comments

> <!ENTITY downloadOnBiff.label "Automatically download new messages">
>-<!ENTITY downloadOnBiff.accesskey "m">
>+<!ENTITY downloadOnBiff.accesskey "A">
> <!ENTITY modelDeleteImmediately.label "Remove it immediately">
>+<!ENTITY modelDeleteImmediately.accesskey "R">

I think this conflicts with the accesskeys of the "Add Account..." and "Remove
Account" buttons.

Comment 41

[Matthew (lilmatt) Willis]

Attachment: rev4 - addresses Stefan's accesskey comments

Stefan,
You're right. I believe I've fixed the issue. It tests okay now.

moving bienvenu's r+ to this patch

Comment 42

[Matthew (lilmatt) Willis]

Attachment: rev4 - AVIARY version - addresses Stefan's accesskey comments

rev4 patch diffed against Aviary tree

Comment 43

[Matthew (lilmatt) Willis]

Comment on attachment 161636 [details] [diff] [review]
rev4 - AVIARY version - addresses Stefan's accesskey comments

Moving bienvenu's r+ from rev3 patch

Comment 44

[Matthew (lilmatt) Willis]

Taking this bug

Comment 45

[Asa Dotzler [:asa]]

restoring nomination.

Comment 46

[Marcia Knous [:marcia]]

Won't make the 1.0 train, not enough time. Minusing for 1.0.

Comment 47

[A. Sapuntzakis]

(In reply to comment #46)
> Won't make the 1.0 train, not enough time. Minusing for 1.0.

So to use Thunderbird 1.0 with Exchange or non-English server folder names will 
require updating prefs manually? That seems like a significant shortcoming ...

Comment 48

[Mike Cowperthwaite]

*** Bug 253309 has been marked as a duplicate of this bug. ***

Comment 49

[Mike Cowperthwaite]

*** Bug 271919 has been marked as a duplicate of this bug. ***

Comment 50

[Matthew (lilmatt) Willis]

Nominating for av1.1/1.8a6

Comment 51

[Matthew (lilmatt) Willis]

Any chance someone can SR this?
It's been 6 months.

Comment 52

[Adam Guthrie]

*** Bug 301899 has been marked as a duplicate of this bug. ***

Comment 53

[Neil Parks]

Suggest changing "Product" on this bug from "Moz App Suite" to "TBird".  

Comment 54

[Ray Booysen]

Blocking 2.0? 

Comment 55

[Daniel Küstner]

AFAICS, this bug has patches against Seamonkey _and_ TB. When you are changing the product, what's happening to the seamonkey patch?

Comment 56

[Ray Booysen]

My apologies, moving this to Core.

Comment 57

[Thomas Stache]

Could Thunderbird 2 include this fix? Would improve the situation for users of non-english IMAP providers, like GMX in Germany.

Comment 58

[Matthew (lilmatt) Willis]

Attachment: rev5 - Patch brought up to date. Also ran past beltzner for UI suggestions.

Carrying over bienvenu's r+

Comment 59

[Matthew (lilmatt) Willis]

Attachment: rev6 - this one works [checked in]

Fixes some dumb mistakes. Works against trunk

Comment 60

[(not reading, please use [e-mail] instead)]

Comment on attachment 225900 [details] [diff] [review]
rev6 - this one works [checked in]

sr=sspitzer, acting mailnews super reviewer while mscott is away.

Comment 61

[David :Bienvenu]

lets try this on the trunk before we land it on the branch.

Comment 62

[Matthew (lilmatt) Willis]

rev6 patch checked in on trunk.

Comment 63

[[e-mail]]

Comment on attachment 225900 [details] [diff] [review]
rev6 - this one works [checked in]

>+            <radio id="modelMarkDeleted"
>+                   value="0" label="&modelMarkDeleted.label;"
>+                   oncommand="selectImapDeleteModel(value)"
>+                   accesskey="&modelMarkDeleted.accesskey;"/>
Radio buttons don't work like this. Or at least, if anyone else is abusing them like this, then please file bugs.
What you're supposed to do is to put an onselect handler on the radiogroup element. Note also that the radiogroup also updates its value so you don't need to. Finally using value as a shorthand for this.value is bad style.

Comment 64

[David :Bienvenu]

this probably explains why when you go back to this dialog after selecting a folder, the drop down is only a couple pixels tall...we'll need this fixed on the trunk before landing on 1.8.1 branch.

Comment 65

[Matthew (lilmatt) Willis]

(In reply to comment #63)
> What you're supposed to do is to put an onselect handler on the radiogroup
> element.
I moved the oncommand to the radiogroup element. I also used oncommand instead of onselect to be consistent with the other radiogroup in this xul file.

I changed to this.value.

(In reply to comment #64)
> this probably explains why when you go back to this dialog after selecting a
> folder, the drop down is only a couple pixels tall

I'm not seeing this behaviour on Mac. I'm building on Linux now to try and replicate it.

Comment 66

[Matthew (lilmatt) Willis]

(In reply to comment #65)
> (In reply to comment #64)
> > this probably explains why when you go back to this dialog after selecting a
> > folder, the drop down is only a couple pixels tall
> 
> I'm not seeing this behaviour on Mac. I'm building on Linux now to try and
> replicate it.

I was unable to replicate that behavior.
I'll attach screenshots of what I'm seeing. 

Comment 67

[Matthew (lilmatt) Willis]

Attachment: Mac OS X screenshot of rev6+

Linux is similar.

Comment 68

[[e-mail]]

(In reply to comment #65)
>I moved the oncommand to the radiogroup element. I also used oncommand instead
>of onselect to be consistent with the other radiogroup in this xul file.
Yeah, I see that now, I'd forgotten radios had oncommand support :-[

Comment 69

[Matthew (lilmatt) Willis]

Comment on attachment 225900 [details] [diff] [review]
rev6 - this one works [checked in]

Re-requesting branch permission after addressing Neil's concerns.

Comment 70

[David :Bienvenu]

has your new patch landed on the trunk yet? I'd like to see that it fixes the problem I saw...

Comment 71

[Matthew (lilmatt) Willis]

(In reply to comment #70)
> has your new patch landed on the trunk yet? I'd like to see that it fixes the
> problem I saw...

Yes. Today's nightly should have it.

If it's not fixed, please give me some more information to try and reproduce it with, as I was unable to up to this point, on both Linux and Mac.

Comment 72

[Matthew (lilmatt) Willis]

(In reply to comment #71)
> If it's not fixed, please give me some more information to try and reproduce it
> with, as I was unable to up to this point, on both Linux and Mac.

I was also unable to reproduce your error in today's nightly using WinXP.

I'm not sure how you'd like to proceed.

Comment 73

[Thomas Stache]

What is the status with the trunk error? Can the patch be approved for the 1.8.1 branch?

Comment 74

[Scott MacGregor]

Comment on attachment 225900 [details] [diff] [review]
rev6 - this one works [checked in]

I think this has baked long enough, do you agree david?  If so, approving for landing on the branch.

Comment 75

[David :Bienvenu]

I think it still has an issue for servers that don't have a trash folder - Matt was able to reproduce it last time I talked to him. I had some brilliant suggestion for him for fixing it, but I don't remember exactly what it was. I think it had to do with checking if the Trash folder existed using FindSubFolder before trying to set the menu drop down to a folder that doesn't exist...and not doing GetResource, because that'll create a bogus folder.

Comment 76

[Scott MacGregor]

clearing the blocking flag. Would still be interested in the patch which I already approved if we fix the issue David is talking about. David, should we clear the approval flag here?

Comment 77

[Matthew (lilmatt) Willis]

(In reply to comment #76)
> clearing the blocking flag. Would still be interested in the patch which I
> already approved if we fix the issue David is talking about.

My plan is to take a look at this soon after we get Sunbird/Lightning 0.3 shipped. David, I should be in MV in 2 weeks. Maybe we can chat (again) about the best way to fix this then?

Comment 78

[David :Bienvenu]

Matt, yes, we can talk about it after Lightning .3 ships. 

Comment 79

[Magnus Melin [:mkmelin]]

*** Bug 352657 has been marked as a duplicate of this bug. ***

Comment 80

[Risto Vaaraniemi]

(In reply to comment #12)

> - If a subfolder is selected, the user gets a response that only top-level
> folders are usable as trash

Is this a general (technical) limitation or is it just based on a decision?

Anyhow, I think that it might be a bit restrictive. For example I am using my IMAP box with Pine, Thunderbird and Squirrelmail. The default location for my mail is the ~/mail/ directory (default IMAP location is probably ~/). Everything else is there except the Thuderbird Trash folder, which is stuck in my home directory. Syncing the TB Trash with Squirrelmail does not therefore work. If I could set TB to store the deleted items to ~/mail/Trash, everything would be hunky-dory.

Should I just settle for this situation or keep my hopes up? ;-)

Best Regards,

Risto

Comment 81

[Wayne Mery (:wsmwk)]

does patch allow destination=local folder?
bug 279141
bug 206277
bug 277854
bug 221946

Comment 82

[Matthew (lilmatt) Willis]

(In reply to comment #75)
> I think it still has an issue for servers that don't have a trash folder - Matt
> was able to reproduce it last time I talked to him. I had some brilliant
> suggestion for him for fixing it, but I don't remember exactly what it was. I
> think it had to do with checking if the Trash folder existed using
> FindSubFolder before trying to set the menu drop down to a folder that doesn't
> exist...and not doing GetResource, because that'll create a bogus folder.

Sunbird and Lightning 0.3 are out the door, so I'm taking a look at this, so that we can hopefully get it in for Tb2.
 
Right now, the default delete model for IMAP is "Move to Trash".  I wonder if instead it makes sense for IMAP to by default simply mark a message as deleted ("big red X"). That would prevent trouble with folks with only an INBOX in their account. In addition, if I then didn't set a default of "Trash" as the trash_folder_name pref, but instead didn't set the menulist at all, it would simply be blank, and when clicked would show you only the folders you're subscribed to, rather than throwing some goofy error.

David and mscott:
Thoughts on making such a change?
I believe it's a matter of changing:
http://lxr.mozilla.org/mozilla/source/mailnews/imap/src/nsImapMailFolder.cpp#2195

I also wonder if it would help to change the default delete model so we don't run into trouble making "Trash" folders by default for people that don't speak English.

Comment 83

[David :Bienvenu]

No, sorry,m we're not going to make the imap delete model the default - it's not very user friendly.

Comment 84

[Matthew (lilmatt) Willis]

(In reply to comment #83)
It was worth a shot. :)

So back to the other way to fix this.
What is the current default behaviour when you delete a message if you only have an INBOX?  Do we attempt to create the "Trash" folder?  If so, what do we do if that fails?

I was thinking that instead of assuming the trash folder is named "Trash", I'd check to see if a "Trash" folder exists. If so, great. We'll assume that's where you want deleted messages to go, and preselect that in the dropdown for you. If not, we would not preselect anything. You need to pick the folder. However, I'm not sure what would happen if a user both didn't have a "Trash" folder, and _didn't_ pick one.

I'd test this myself, but I don't have a scrap IMAP account handy anymore. I'll work on that tomorrow.

Comment 85

[Thomas Stache]

Any update on this? Feels like it slipped from 2.0 for good.

Comment 86

[Scott MacGregor]

Comment on attachment 225900 [details] [diff] [review]
rev6 - this one works [checked in]

removing the approval request, we've passed the string freeze.

Comment 87

[Steffen Wilberg]

Comment on attachment 225900 [details] [diff] [review]
rev6 - this one works [checked in]

This has been checked in on 2006-06-17 06:17. Can this be marked fixed?
Or does the problem mentioned in comment 64 still occur?

Comment 88

[David :Bienvenu]

I think the problem in comment 64 still occurs 

Comment 90

[Martijn Reuvers]

Okay, I'm really not into the details of this particular bug. So perhaps I'm thinking too lightly about this bug. 

But I downloaded TB 2.0 today and still people have to manually rename their Trashcan through the config editor. Still something as simple as renaming your trashcan in the Front End isn't possible in TB. Why?

To me this would seem like a very easy to solve bug: simply put a Trashcan area in Acount Settings -> Copies & Folders. All the other default folders are defined there as well, so why not simply put the Trashcan there?

Comment 91

[Steffen Wilberg]

I just tested a trunk (3.0a1pre 2007071206) build.
In Account Settings -> Server Settings -> When I delete a message, the "Move it to this folder" radio button is selected, and the folder picker displays "Trash".
I then set it to another folder, "Papierkorb", and click OK.
When opening Account Settings again, the "Trash" folder is selected again, and the folder works as such (displaying the trash icon, having Empty Trash in the context menu, and being the folder to which deleted messages are moved).

The error console doesn't display anything. The pref javascript.options.showInConsole is set to true, being the default.

I even have this in my prefs.js, which works fine with branch builds, but doesn't with trunk builds:
user_pref("mail.server.server2.trash_folder_name", "Papierkorb");
When using the trunk nightly for the first time, the Trash folder got created; it didn't exist before.

Comment 92

[Wayne Mery (:wsmwk)]

might want to test if the UI method suffers the problem described in bug 374638.


(In reply to comment #90)
> I downloaded TB 2.0 today and still people have to manually rename their
> Trashcan through the config editor. Still something as simple as renaming your
> trashcan in the Front End isn't possible in TB. Why?

Martijn, this fixed on trunk and is not likely to be delivered in an update to v2, there are rules and considerations which govern such things.

Comment 95

[Steffen Wilberg]

Comment 91 is bug 374638, which is a recent (2007-05-09 18:06) trunk regression from bug 379070.

Comment 96

[Patrick Lathem]

(In reply to comment #92)
> Martijn, this fixed on trunk and is not likely to be delivered in an update to
> v2, there are rules and considerations which govern such things.

Could this be nominated for blocking TB3? Or is that reserved for critical bugs only? 

Comment 97

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9a9pre) Gecko/2007110514 Thunderbird/3.0a1pre ID:2007110514

I run a test with my debug version on OS X and there are still issues left. The problem mentioned in comment 64 isn't visible for me. But after selecting a different trash folder I deleted one message. This message wasn't moved into the new folder but in Trash. I had to restart Thunderbird to get this working.

Further the new folder gets assigned the trash icon. After I revert the changes and reset the trash folder the trash icon still remains for the new folder. It has to be reset. I think the same applies to "Trash" when it's no trash folder anymore. It should be handled as a normal folder and should get its icon.

Comment 98

[Magnus Melin [:mkmelin]]

I don't see comment 64 either, but comment 75 still applies. (Problem with the dropdown if the server do not have a Trash folder, or the Trash folder is not subscribed.)

Comment 99

[Marco Zehe (:MarcoZ)]

There is also a problem with the new IMAP support in gmail. The Trash folder, by default, is located in [Google Mail]/Trash (or the localized word for Trash if you're using a different language version). I can set it via the UI, but only the "Trash" portion is saved to the prefs.js file. Editing it by hand actually solves the problem, meaning the [Google Mail]/Trash folder is then recognized as the Trash folder on next start of TB3.
So, it would be nice if the whole folder hierarchy would be saved to prefs.js, as is the case with Drafts, Sent and Junk folders, which *are* properly saved.

Comment 101

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #99)

As I wrote in Bug 404217 Comment #7, Tb trunk's UI for IMAP Trash is already changed(improved) to "choice of folder to move mail when delete".
So user can choose any IMAP mail folder(==label when Gmail) as "trash", including [Gmail]/Trash when Gmail IMAP, by clicking folder in list.
Sorry but I don't know when / by which patch the UI was changed.

Comment 102

[WADA:World Anti-bad-Duping Agency]

UI improvement by trunk doesn't work currently if subfolder([Gmail]/Trash when Gmail IMAP). I've opened Bug 427311 for this issue on trunk. 

Comment 107

[WADA:World Anti-bad-Duping Agency]

As I wrote Comment #101, UI for IMAP "trash bin" choice is already improved on Tb trunk. So, this bug usually is to be closed as WORKSFORME (not FIXED, because improved by unknown patch).

To Matthew Willis(assignee) & all problem reporters(who requested UI improvement):

Does "this bug is still kept as open" mean that current UI improvement on trunk is not sufficient or inappropriate for you?
Or trying to find bug/patch who improved the UI?
Or simply waiting for fix of Bug 427311 in improved UI?
(sorry but I don't know Seamonkey trunk case)   

Comment 108

[WADA:World Anti-bad-Duping Agency]

To Magnus Melin(who puts "remaining problem" in Whiteboard): 

> Whiteboard:
> [remaining problem occurs with the dropdown if the server do not have a Trash folder, or the Trash folder is not subscribed]

As you say, non-exsistent or non-subscribed IMAP folder is not listed in folder selection list for "trash bin". Is bug for the issue already opened?

I think this is better to be improved too. But this is same on IMAP folder choice for Sent, Drafts, Temprates, Junk folder, isn't it?
What do you think about best way to improve this issue?

Comment 110

[Dan Mosedale (:dmosedale, :dmose)]

As WADA says, the core bug here has been fixed.  It appears to me to have been fixed in this bug, even, so resolving as FIXED.  Please file issues with the long-since-landed UI as separate bugs.  And since it's fixed, it can't block. Removing the blocking-thunderbird 3 request.

Comment 111

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Dan, which bug fixed this one?

Comment 112

[Magnus Melin [:mkmelin]]

This bug fixed this one.

Comment 113

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Magnus, the latest patch on this bug is from 2006 but the TM is set to mozilla1.9a1. Do I miss something?

Comment 114

[Steffen Wilberg]

That's correct. This bug was fixed 2006-06-17 06:17 on cvs trunk (HEAD), but never made it to the MOZILLA_1_8_BRANCH (which already branched 2005-08-12) due to comment 64 and comment 75, so it's not part of Thunderbird 2.

Comment 115

[Magnus Melin [:mkmelin]]

Yep, mozilla1.9a1 was December 8, 2006, and this landed in time for that, but never on any branch. Long release cycle:) 

Comment 116

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Duh, I forgot our switch to 1.9.1 for Thunderbird 3. You both are true, sorry.

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1a2pre) Gecko/20080813030132 Shredder/3.0b1pre ID:20080813030132