Closed
Bug 24064
Opened 25 years ago
Closed 22 years ago
Need ability to specify Trash folder
Categories
(MailNews Core :: Backend, enhancement, P3)
MailNews Core
Backend
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: BenB, Assigned: emaijala+moz)
References
Details
(Keywords: helpwanted, imap-interop)
Attachments
(1 file, 9 obsolete files)
11.41 KB,
patch
|
Henry.Jia
:
review+
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
I can specify a folder per server for us as special folder, like "Drafts" etc. In 4.7, this included the trash folder, but I can't find UI in Mozilla for it. Currently, Mailnews just creates "~/Trash" on the server (without the possibility to change it). Is there already a (hidden) pref?
Comment 1•25 years ago
|
||
There's a 4.x pref for the Trash folder? Are you sure? I wasn't aware of that. In mozilla, if we create the Trash folder when we're in "IMAP delete model", that's a bug, but I don't think we have a plan to make the Trash folder configurable. So, please educate me about 4.x, or clarify this bug. If the bug is that you want a configurable Trash folder, and 4.x didn't have one, this will be a [help wanted] bug.
Reporter | ||
Comment 2•25 years ago
|
||
Phil, hm, you right. There is none. The reason, why Mozilla bugs me and 4.7 doesn't, is bug #20878. However, I don't see a reason, why it shouldn't be configurable, while "Sent" and "Draft" are. > So, please [...] clarify this bug. What do you need to know?
Updated•25 years ago
|
Assignee: phil → jglick
Comment 3•25 years ago
|
||
I see your point about consistency, but it doesn't seem really high-value to me. Reassigning to jglick since this would be a spec change.
Comment 4•25 years ago
|
||
To be clear, I don't think the temporary existence of bug 20878 is a good reason to add configurable trash.
Reporter | ||
Comment 5•25 years ago
|
||
> I don't think the temporary existence of bug 20878 is a good reason
> to add configurable trash
Agreed, otherwise I wold have marked it as a DUP. But I think, it's "the right
thing" (tm).
IMHO, I don't think we gain much by having a configurable trash. In fact, it might even confuse some less experienced folks. And if a less experienced user accidentally changes the Trash to a different folder... I see the inconsistency with Sent, Drafts and Stationery but i'm not sure if that is enough of a reason to change Trash. Sent, Drafts and Stationery are generally things the user is saving and keeping for a reason, and hence it might be nice to change the default location. But Trash is something you are getting rid of. Why would you want to save/move it to a different folder? If we did this, some potential implications: Right now, Sent, Drafts and Stationery settings are on the Copies and Folders prefs dialog. Trash behavior is on the Server prefs dialog. If the selecting a default folder behavior is the same for the Trash, we might want/need to reorganize these so Trash is with the others? In my opinion, this isn't something we need to do. Phil's call.
Updated•25 years ago
|
Assignee: phil → nobody
Summary: Let me specify a trash folder → [HELP WANTED] Let me specify a trash folder
Whiteboard: [HELP WANTED]
Comment 7•25 years ago
|
||
Adding to the [help wanted] list.
Comment 8•25 years ago
|
||
Here's an example: I use a free IMAP service, mailandnews.com. It insists on having a `Deleted' folder on the server. Whenever I delete messages using the Web interface, they go into the Deleted folder; but whenever I delete messages using IMAP on Communicator 4.5, they go into the Trash folder. Mailandnews.com has a nice feature where messages are permanently removed from the Deleted folder after x days; so it would be nice if I could configure Mozilla to use the Deleted folder, rather than the Trash folder, for deleted messages on this account. Another example is that if I'm using Mozilla in a LOTE (language other than English), I would probably want to use localized names for folders -- Inbox, Drafts, Templates, *and* Trash. As for any potential confusion, that would largely be alleviated by the trashcan icon, which indicates which folder in an account is the trash folder.
Updated•25 years ago
|
Keywords: helpwanted
Comment 9•25 years ago
|
||
Interesting point about mailandnews.com. On the localization point, I believe we did localize the folder names in Communicator 4.x
Summary: [HELP WANTED] Let me specify a trash folder → Let me specify a trash folder
Whiteboard: [HELP WANTED]
Comment 10•24 years ago
|
||
*** Bug 35309 has been marked as a duplicate of this bug. ***
Comment 11•24 years ago
|
||
*** Bug 35309 has been marked as a duplicate of this bug. ***
Comment 12•23 years ago
|
||
How about saving space on your IMAP server? My school server gets whiney if my mailboxes get too big, and it's logical to keep one's trash archives local. How about an option on the trash folder > make local, and the icon changes to correspond.
Comment 13•23 years ago
|
||
*** Bug 72449 has been marked as a duplicate of this bug. ***
Comment 14•23 years ago
|
||
I don't like having 6 Trash-Folders on 6 different folders either, I'd prefer to keept them in a centralized trash folder in my local folders.
Comment 15•23 years ago
|
||
Exchange uses Deleted Items. So when I switch between Outlook and MozMail, I'd like to be able to use "Deleted Items" on both. I really hope this gets added to make it more seemless when switching between programs.
Comment 16•23 years ago
|
||
I use an English localized Mozilla but I connect to a Spanish localized MS Exchange Server. In this server the Trash folder is named "Elementos eliminados". So I think we should let the user select the Trash folder. Like we do with Drafts, Sent and Templates.
Comment 17•22 years ago
|
||
The PocketPC 2002 (in English at least) calls its trash "Deleted Items" so I have both "Deleted Items" and "Trash" in my folder display. I would not mind if it was in prefs.js, or if it were still referred to as Trash in the menus (I guess these are synonymous, as not changing the menus too would be a hacky solution). I would just like some way for deleted messages to go to Deleted Items and for "Empty Trash" to empty *that* folder.
Comment 18•22 years ago
|
||
*** Bug 135125 has been marked as a duplicate of this bug. ***
Comment 19•22 years ago
|
||
See dupe bug #135125 for a possible way of integrating this into a gui. I can't believe I missed this bug while writing the dupe.
Updated•22 years ago
|
QA Contact: lchiang → huang
Comment 20•22 years ago
|
||
As far as I can tell there's only one universal folder name: INBOX (looking at RFC2060). So it's important that Mozilla supports any arbitrary name for any other special folders including trash. Since this bug is owned by nobody, I'm changing the Summary and Keywords to make this bug more general so it can be found easier and so we can dup more bugs against it.
Keywords: interop
Summary: Let me specify a trash folder → Need ability to map/specify Trash folder; IMAP mail servers use other names for Trash
Comment 21•22 years ago
|
||
*** Bug 27785 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 22•22 years ago
|
||
spam: I don't see how your summary widens the scope - it's only longer. shortening.
Summary: Need ability to map/specify Trash folder; IMAP mail servers use other names for Trash → Need ability to specify Trash folder
Comment 23•22 years ago
|
||
Jumping over here from the recently-duped 27785 and having read the comments where it is argued that we dont gain anything more than consistency by allowing a user-specified trash folder name I have to argue against that point of view. Other mail and news clients may not do it but where does it say that mozilla isnt allowed to be better than the others? Personally I'm wishing I was a good enough coder to respond to the helpwanted keyword....
Comment 24•22 years ago
|
||
Outlook Express just marks messages at deleted in the INBOX. If "Store special folders on IMAP server" is checked in the advanced account setup the user can type in the "Sent items path" ("Sent Items" by default) and the "Drafts path" ("Drafts" by default). It will look for these folders on the server and automatically subscribe to them if they currently aren't. The program is hard coded to support "Deleted Items" and can expunge the folder automatically upon disconnect but this is only for local folders (e.g., a POP account). There is no setting as far as I can find for support of this folder on IMAP.
Comment 25•22 years ago
|
||
outlook, full version or express, isnt an issue since outlook is just another mail client - the mail servers quirks, however, are. When using mozilla to connect to mail servers with those quirks we ought to be able to configure mozilla to take account of both the quirks and our own preferences. I wont go into a rant about braindamaged exchange servers and the quirks of their IMAP gateways, lets just say that as a unix admin in a microsoft-dominated corporate environment anything that eases interop concerns is a major benefit for me.
Comment 26•22 years ago
|
||
If I use mozilla with IMAP on an outlook account, mozilla shows the "deleted items" folder. But if I delete messages from mozilla, they go to "Trash". And that trash folder also appears in outlook since it is a IMAP folder. So I end up with to folders for deleted items: one called Trash where mozilla puts its mails, and one called "Deleted Items" where outlook puts its mails.
Updated•22 years ago
|
Summary: Need ability to specify Trash folder → Need ability to specify Trash folder for IMAP
Reporter | ||
Comment 27•22 years ago
|
||
(summary: no, local folders, too.)
Summary: Need ability to specify Trash folder for IMAP → Need ability to specify Trash folder
Comment 28•22 years ago
|
||
A lot of interesting discussion. As this bug/issue is open for more than two years now, what about the suggestion mentioned in bug 135125? --quote-- "When I delete a message: - Move it to specified Trash folder - Mark it as deleted - Remove it immediately und below that: Use the following folder as Trash folder: (allow you to select any folder here) --quote-- (Similar to "Sent" etc.) What do you think?
Comment 29•22 years ago
|
||
no. i see three options, 1, something like: "When I delete a message: ( ) Move it to [Trash folder on this server |v] ( ) Mark it as deleted ( ) Remove it immediately 2, leave the ui in server prefs panel alone (except perhaps changing the text a bit to reflect where trash is). Add the folder selection option to the Copies and Folders pref panel 3, leave this bug unresolved and continue to get bug reports and complaints. <we are here> imo we have enough reasons for wanting this option, which only leaves the issue of where to put it, and who to implement it. I'm going to be arbitrary and put my support behind 2. mpt, ben, stephend, jglick, what do you think? if pixel jockies still exists, should we discuss it there? (i haven't gotten anything other than spam from it in months) ere expressed an interest in imap bugs, so i'm volunteering ere for backend. and my guess is that stephend would do the front end if we made a decission.
Severity: normal → enhancement
Comment 30•22 years ago
|
||
Or 4. first, use user_pref("mail.identity.id1.deleted_folder", "..."); and leave the ui as it is. Later, implement 2.
Reporter | ||
Comment 31•22 years ago
|
||
> I'm going to be arbitrary and put my support behind 2.
> mpt, ben, stephend, jglick, what do you think?
me too (2.)
Assignee | ||
Comment 34•22 years ago
|
||
This patch adds a pref to specify the trash folder name (mail.server.server?.trash_folder). It is server, not identity, specific. Did I use the strings correctly? This raises one problem. When another trash folder is specified, it will be created automatically, but there is no way to delete the old one. It cab be unsubscribed, but should we be able to delete it completely?
Comment 35•22 years ago
|
||
Personally, I would much rather have it be identity-specific and work exactly like the other folder-preferences in Copies&Folders, because that's more discoverable than a hidden pref. Could you do it that way?
Assignee | ||
Comment 36•22 years ago
|
||
I don't see how it's connected to identity. Sent and Drafts are clearly connected to the identity used to compose the message, but there is no such thing with trash. Of course we can use the default identity for a server, but is there a reason to do so? Only reason I can find is that the settings in Copies & Folders are identity specific. The front end will probably be done separately (see comment #29).
Comment 37•22 years ago
|
||
You say:
> I don't see how it's connected to identity. Sent and Drafts are clearly
connected to the identity used to compose the message, but there is no such
thing with trash.
OK, think this.
You are using an email service where you consolidate several different email
accounts (by forwarding them from the others)
You have several identities, but want to save the drafts and sent folder to this
IMAP account where you are consolidating all your accounts.
It would also make sense to make the Trash part of this. A simple reason is that
it would be just an IMAP copy and delete operation rather than uploading the
mail to a different server.
Also, should multiple identities per server ever become possible (I severly hope
so), it would be nice to have a seperate trash folder for each, even if they are
loated on the same server.
Assignee | ||
Comment 38•22 years ago
|
||
Personally, most of the messages I delete are inbound (not located in Sent or Drafts). Then having the Trash on the same server makes perfect sense. Maybe you can convince me that having separate trash folders specified in identities is the way to go, but there are some more problems to solve: 1. Do the (much larger) changes required to support trash folder on another server (if we decide to do that, I can't do the back end). 2. Choose where a deleted message goes 2.1 Delete a message from Inbox - probably the trash of default identity? 2.2 Delete a message from Sent - default identity? - identity where this is the Sent or Drafts folder? - what if there are two identities with same Sent but different Trashes? 3. Other trash folder settings (such as emptying trash on exit) - move them to identities? - migrate old settings from servers? There are probably still other issues I don't remember at the moment.
Comment 39•22 years ago
|
||
I think it should be one per mail account. I have a work server and a personal server. One runs Cyrus and the other runs Exchange. So shouldn't it be customizable per account where the deleted items go as different servers and clients like different folders for their trash. I feel it should work just like the Sent Messages in how configurable it is.
Comment 40•22 years ago
|
||
If you are using an IMAP server, the trash folder named "foobar" will be created on the server. So on this server there is a trash folder called "foobar" and this name is independent of the identity. If you create a second identity with your private nickname and connect to the same IMAP server, you will still see the trash folder "foobar". (Connection identities with servers can be done manually in prefs.js) Also if you delete a mail, you can't delete it with different identities nor is there an identity which deletes. It's simply a user which deletes from a specific server. So in my opinion the trash folder is server bound.
Reporter | ||
Comment 41•22 years ago
|
||
IMO, having the trash folder as pref of the incoming server is just fine and logical. This fact doesn't have to change the (current) frontend - the trash folder could still be specified at the "Copies and Folders" pane, that's where I'd expect it to be.
Comment 42•22 years ago
|
||
*** Bug 153716 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 43•22 years ago
|
||
Just a new diff against the current version.
Attachment #93066 -
Attachment is obsolete: true
Comment 44•22 years ago
|
||
Comment on attachment 105707 [details] [diff] [review] v1.1 Patch to add backend pref for trash folder name Incoming server standards for the remote message storage. Each account includes only one single incoming server. Identity is used for composing and outgoing mail message. Each account can have more than one identities. So it's fine to bundle the trash folder with the incoming server to hold the deleted messages. r=henry
Attachment #105707 -
Flags: review+
Comment 45•22 years ago
|
||
Comment on attachment 105707 [details] [diff] [review] v1.1 Patch to add backend pref for trash folder name +const char *kDefaultImapTrashFolderName = "Trash"; // **** needs to be localized **** can you make this localizable? + *retval = nsCRT::strdup(kDefaultImapTrashFolderName); given your current code, the value of trash folder is known at compile-time. so, when you make kDefaultImapTrashFolderName a const char[], you could use strndup here (in combination with sizeof) + nsString trashName; + GetTrashFolderName(trashName); err. no. In most cases, the trash folder will be less than 64 chars. So use an nsAutoString, that will save an malloc. +void nsImapMailFolder::GetTrashFolderName(nsString &aFolderName) Use nsAString& in function arguments, not nsString&. + if (!m_trashFolderName.Length()) convert the tab to spaces. + if (NS_SUCCEEDED(imapServer->GetTrashFolderName(&temps)) && temps) + m_trashFolderName.AssignWithConversion(temps); um, why can't you make the GetTrashFolderName use nsAString& instead of char**? even if you don't, do _not_ use AssignWithConversion. imho, you should store the pref as UTF-8... (with SetComplexValue and nsISupportsString, iirc). That would allow non-latin1 names. + nsString m_trashFolderName; use nsAutoString. + char *temps; + server->GetTrashFolderName(&temps); um... well... can you at least use nsXPIDLCString here?
Comment 46•22 years ago
|
||
Christian Biesinger: Thx for the string usage. Henry
Assignee | ||
Comment 47•22 years ago
|
||
Comment on attachment 105707 [details] [diff] [review] v1.1 Patch to add backend pref for trash folder name I'll try to do the changes.
Attachment #105707 -
Flags: needs-work+
Assignee | ||
Comment 48•22 years ago
|
||
This patch changes the string handling quite a bit. The pref is now stored in UTF-8 and it's correctly converted to IMAP UTF-7 when used. Default trash name is taken from the string bundle (where it has existed also before).
Attachment #105707 -
Attachment is obsolete: true
Assignee | ||
Comment 49•22 years ago
|
||
Additional fixes after consulting biesi.
Attachment #105756 -
Attachment is obsolete: true
Assignee | ||
Comment 50•22 years ago
|
||
Still improved.
Attachment #105766 -
Attachment is obsolete: true
Comment 51•22 years ago
|
||
Comment on attachment 105770 [details] [diff] [review] v2.2 Patch to add backend pref for trash folder name I think this patch is ok. Just some trivials. + nsresult rv = GetUnicharValue("trash_folder", retval); Since the pref is for the trash folder name instead of the trash folder itself, IMO it's better to use 'trashFolderName' than 'trash_folder'. Also, use '#define' instead of the hardcode. + rv = bundleService->CreateBundle("chrome://messenger/locale/messenger.properties", How about #define MESSENGER_STRBUNDLE_URL "chrome://messenger/locale/messenger.properties" and + rv = bundleService->CreateBundle(MESSENGER_STRBUNDLE_URL, instead of the hardcode? + nsCString m_trashFolderName; => + nsCAutoString m_trashFolderName; biesi, what's your opinion?
Assignee | ||
Comment 52•22 years ago
|
||
>+ nsresult rv = GetUnicharValue("trash_folder", retval);
>Since the pref is for the trash folder name instead of the trash folder itself,
>IMO it's better to use 'trashFolderName' than 'trash_folder'. Also, use
>'#define' instead of the hardcode.
trash_folder is more consistent with existing stuff ("draft_folder",
"fcc_folder" etc.). Where should I #define it? No other prefs are #defined either..
Comment 53•22 years ago
|
||
>+ nsCString m_trashFolderName;
>=> + nsCAutoString m_trashFolderName;
>biesi, what's your opinion?
oh indeed, CAutoString looks indeed better here, thanks for catching this.
Comment 54•22 years ago
|
||
> trash_folder is more consistent with existing stuff ("draft_folder", > "fcc_folder" etc.). Where should I #define it? No other prefs are > #defined either.. There is a little difference. fcc_folder, etc is used for defining the folders, such as "imap://hj@mail1.sun.com/Sent". Here, we just try to make the trash folder name configurable, such as 'Deleted Items'. I mean #define TRASH_FOLDER_NAME "trashFolderName" and + nsresult rv = GetUnicharValue(TRASH_FOLER_NAME, retval); by '#define' above.
Assignee | ||
Comment 55•22 years ago
|
||
Still more fixes. I took the liberty of studying existing code and naming the defines and the pref accordingly. Ok?
Attachment #105770 -
Attachment is obsolete: true
Comment 56•22 years ago
|
||
Comment on attachment 105849 [details] [diff] [review] v2.2 Patch to add backend pref for trash folder name It's meaningful. So I think it's ok. Thx, ere.
Attachment #105849 -
Flags: review+
Comment 57•22 years ago
|
||
As for the comment in http://bugzilla.mozilla.org/show_bug.cgi?id=24064#c54, I don't think that this bug is only for changing the name of the trash folder, but about the ability to select a different folder for trash, just like the FCC folder. At least this is what the bug I reported (http://bugzilla.mozilla.org/show_bug.cgi?id=153716) meant, and it was marked as duplicate of this one. So either the scope of this bug should include the option to select a different folder and not only the name, or the other bug should be re-opened.
Assignee | ||
Comment 58•22 years ago
|
||
Ilan, "Name" in this context is "the folder". This pref is just for what you say. There is just a difference in having a pref for a folder "name" rather than a whole folder uri.
Comment 59•22 years ago
|
||
Ere & Ilan, Since the trash folder is bundled to the incoming server. There is no need to select the folder. Ability to specify the trash folder name is enough. I've not seen any server with the trash folder in some sub-folder. It's always in the first level folder (of course, if there is a namespace, it's under the namespace). Also, this patch is for the ability to specify the trash folder name instead of the folder uri. Right? If I'm wrong, please correct me. Henry
Comment 60•22 years ago
|
||
Hi, When I originaly reported the bug (that was marked duplicated, http://bugzilla.mozilla.org/show_bug.cgi?id=153716), I thought about the ability to specify the trash folder to be, for example one of the local folders. This way, the trash is not accumulated on the server, where I have limited disk space. I didn't really think about changing the name (which I do see why this option is needed), or specifing a different place on the server. bye, Ilan
Comment 61•22 years ago
|
||
Ilan, I know your meaning. Let me take a deep think. Anyone else's opinions? Henry
Comment 62•22 years ago
|
||
It is fine if I can change the imap folder name from "Trash" to "Deleted Items". But if someone wants to store the trash in his local folders, this is something completely different.
Comment 63•22 years ago
|
||
Ere, I was with you until I came to the part of the patch where you store the trash folder name in every imap mail folder. That seems wrong, and wasteful. Why not get it from the server? It's a little slower, but it cuts down on unneeded bloat.
Comment 64•22 years ago
|
||
Comment on attachment 105849 [details] [diff] [review] v2.2 Patch to add backend pref for trash folder name David's right. If the user have 50 folders, the footprint will increase 3K. Another thing is, the trash folder name should be case sensitive. Otherwise, if you have two folders named 'trash' & 'TRASH', they'll both be flaged to the specific trash folder. But indeed, there is only one is in use by mozilla and the user may potentially want to use the other for other usage.
Attachment #105849 -
Flags: review+ → review-
Assignee | ||
Comment 65•22 years ago
|
||
Ok, I'll change it so that the folder does not cache the name. I was just thinking that getting it from the server all the time would be expensive, but if it's not a problem, even better. Trash name comparison has been case insensitive before, should I change that now?
Comment 66•22 years ago
|
||
Thx for the hard work, ere. I think trash name comparison should be case sensitive. Mozilla will only use one. Another issue is that after another trash folder is defined, the old one is still flaged as trash folder and still works as a trash folder. Right? If so, I think this is an issue also needed to be solved.
Assignee | ||
Comment 67•22 years ago
|
||
Henry, Yes, leaving the old trash as a trash folder is actually worse than it seemed to me in the beginning. It will choose the first trash folder it finds as the target for trash actions. Until now I always had trash names that were sorted before "Trash" and of course it worked. I'll dig deeper into it. Do you think it's ok to just remove the trash flag from the old trash folder when a new one is created? I just want to make sure that it isn't a big no-no to demote the trash folder to a normal one.
Comment 68•22 years ago
|
||
I think it's ok to just remove the trash flag from the old trash folder when a new one is created.
Comment 69•22 years ago
|
||
I think you have to remove the trash flag from the old folder. We do that for other folders when the pref changes.
Assignee | ||
Comment 70•22 years ago
|
||
This should do the trick if we just had a front end to change the pref. What do you guys think?
Comment 71•22 years ago
|
||
When do you want nsImapIncomingServer::SetTrashFolderName to be called and the trash flag removed from the old folder besides invoking by the front end? How to handle the two scenarios? 1. the old trash folder that has been set up by the user without the patch 2. the user change the pref setting manually Thx.
Assignee | ||
Comment 72•22 years ago
|
||
Good, this is something I expected. As far as I can see, now I've done it the same way it's done for the other folders. Try changing the drafts folder by hand and see for yourself. It's not handled there either. Now, what needs to be decided is, if this is satisfactory (maybe changing prefs was considered so advanced that the user should know how to remove the folder by hand also) or if we want to handle it better here. The latter sounds better to me, but it will add some overhead somewhere in the code. I could for example enumerate all folders with trash flag and reset wrong ones when reading the folder name from prefs, but this would add a bit to the reading (always need to check if there are multiple folders with the flag set). I don't know how expensive that would be, though. Opinions?
Assignee | ||
Comment 73•22 years ago
|
||
Hmm.. no opinions? I'll just do it then and we'll see..
Assignee | ||
Comment 74•22 years ago
|
||
Added: Check for multiple trash folders in DiscoveryDone() and unflag the wrong one. Removed: Use of localized trash folder name on the server, now back to using "Trash" if the pref is not set. Localized string says what is displayed. Would otherwise break ruin old localized installations. Fixed: An old reference leak at http://lxr.mozilla.org/seamonkey/source/mailnews/imap/src/nsImapMailFolder.cpp#4189. GetFoldersWithFlag() already addrefs the reference, should not do it again, right? Also fixed string leak if the trash_folder_name is set but empty. Comments?
Attachment #105849 -
Attachment is obsolete: true
Attachment #106407 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #107237 -
Flags: superreview?(bienvenu)
Attachment #107237 -
Flags: review?(Henry.Jia)
Comment 75•22 years ago
|
||
I don't think we should refresh the view in that way. We should either just live with the 2 trash folder icons getting displayed (it's just temporary, and doesn't break things, right, because the trash folder flag is set correctly), or we should notify rdf correctly that the special node property has changed when this flag is cleared or set. It's surprising we don't have a general mechanism for this - it must be the cause of a lot of little bugs. For example, if you change the sent folder, from the code, it doesn't seem like we change the icon until the next time you run. that's my major question. The rest of the stuff is minor. this is a strange ownership model: +NS_IMETHODIMP nsImapIncomingServer::GetTrashFolderName(PRUnichar **retval) +{ + nsresult rv = GetUnicharValue(PREF_TRASH_FOLDER_NAME, retval); + if (NS_FAILED(rv)) + return rv; + + if (!*retval || !**retval) + { + if (*retval) + nsMemory::Free(*retval); I think the caller should free this memory, if needed, not this routine. temps is an odd name - how about "trashFolderName" + nsXPIDLString temps; + if (NS_SUCCEEDED(imapServer->GetTrashFolderName(getter_Copies(temps)))) + { + aFolderName = temps; + } we don't use the "_" convention in this code, so it should be something like oldTrashNameUtf7. + char *oldTrashName_utf7 = CreateUtf7ConvertedStringFromUnicode(oldTrashName); + if (oldTrashName_utf7)
Assignee | ||
Comment 76•22 years ago
|
||
> this is a strange ownership model: > > +NS_IMETHODIMP nsImapIncomingServer::GetTrashFolderName(PRUnichar **retval) > +{ > + nsresult rv = GetUnicharValue(PREF_TRASH_FOLDER_NAME, retval); > + if (NS_FAILED(rv)) > + return rv; > + > + if (!*retval || !**retval) > + { > + if (*retval) > + nsMemory::Free(*retval); > > I think the caller should free this memory, if needed, not this routine. No, GetUnicharValue() on the top of the method might have allocated it but it might be empty. If it's empty, it's not acceptable and is replaced by the default right after the quoted part. Anyway, if it is allocated in GetUnicharValue(), we must free it before re-allocating. I'll add a comment to the code. I'll fix the naming issues.
Comment 77•22 years ago
|
||
duh, stupid me, yes, I see.
Assignee | ||
Comment 78•22 years ago
|
||
Fixed string naming and removed the hack to update the UI. It sure would be nice to be able to easily update the UI though..
Attachment #107237 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #107499 -
Flags: superreview?(bienvenu)
Comment 79•22 years ago
|
||
Comment on attachment 107499 [details] [diff] [review] v3.1 Patch to add backend pref for trash folder name this string name temps should be fixed: + { + nsXPIDLString temps; + if (NS_SUCCEEDED(imapServer->GetTrashFolderName(getter_Copies(temps)))) + { + aFolderName = temps; + } other than that, sr=bienvenu. I'll look for a bug about changing the folder flags invalidating the view, and if there's not one, I 'll file one.
Attachment #107499 -
Flags: superreview?(bienvenu) → superreview+
Assignee | ||
Comment 80•22 years ago
|
||
Fixed that one too.
Attachment #107499 -
Attachment is obsolete: true
Comment 81•22 years ago
|
||
see bug 31685 re updating the folder icon.
Assignee | ||
Comment 82•22 years ago
|
||
Comment on attachment 107502 [details] [diff] [review] v3.2 Patch to add backend pref for trash folder name So is there a way to transfer sr+ so that it doesn't look like I sr'd it myself?
Attachment #107502 -
Flags: superreview?(bienvenu)
Attachment #107502 -
Flags: review?(Henry.Jia)
Comment 83•22 years ago
|
||
Comment on attachment 107502 [details] [diff] [review] v3.2 Patch to add backend pref for trash folder name most people just add a comment "transfering sr"...
Attachment #107502 -
Flags: superreview?(bienvenu) → superreview+
Comment 84•22 years ago
|
||
Comment on attachment 107502 [details] [diff] [review] v3.2 Patch to add backend pref for trash folder name ere: Just one question: why put the trash flag clear code in nsImapIncomingServer::OnlineFolderDelete ? See also comment #75 and comment #79. If it is not a good place to put the code, I think we should get rid of the code here and fix the issue in another bug. David, when you file the bug, please add me to the cc list. Henry
Comment 85•22 years ago
|
||
Henry, there's already an existing bug 31685 - And Ere's change is in OnDiscoveryComplete, not OnlineFolderDeleted, if you'll look at the line numbers specified in the diff.
Comment 86•22 years ago
|
||
Comment on attachment 107502 [details] [diff] [review] v3.2 Patch to add backend pref for trash folder name bug 31685 has a little difference with the issue here. Sorry for the stupid question. r=henry
Attachment #107502 -
Flags: review?(Henry.Jia) → review+
Assignee | ||
Comment 87•22 years ago
|
||
Fix checked in to trunk.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 88•22 years ago
|
||
Do we want UI for this now? Should a new bug be filed about it? I'm probably not able to do it.
Comment 89•22 years ago
|
||
ere, feel free to log a bug about pref UI. please cc jglick and me.
Comment 90•22 years ago
|
||
Ere, is this only going in for IMAP mail accounts? If a user also has a POP account and wants all trashed msgs from any account IMAP or POP to go into one designated Trash folder will this fix (UI and backend) allow for POP accounts to change their Trash folder destination?
Assignee | ||
Comment 91•22 years ago
|
||
Yes, this is only for imap and only for specifying the name of the trash folder on the server, mostly for people using Exchange, different Web mails or localized mail systems that want the trash named differently (well summarized in comment #8). Yes, there have been requests for the ability to set the trash folder to be a local folder or anything else, but that would be another thing (with its own problems).
Updated•22 years ago
|
Attachment #107237 -
Flags: superreview?(bienvenu)
Attachment #107237 -
Flags: review?(Henry.Jia)
Comment 92•22 years ago
|
||
Filed bug 185494 for the UI.
Comment 93•21 years ago
|
||
*** Bug 197188 has been marked as a duplicate of this bug. ***
Comment 94•21 years ago
|
||
*** Bug 211303 has been marked as a duplicate of this bug. ***
Comment 95•21 years ago
|
||
*** Bug 159429 has been marked as a duplicate of this bug. ***
Comment 96•20 years ago
|
||
*** Bug 232610 has been marked as a duplicate of this bug. ***
Comment 97•20 years ago
|
||
*** Bug 232610 has been marked as a duplicate of this bug. ***
Comment 98•20 years ago
|
||
To change the "Trash" folder, add/modify the following preference in prefs.js user_pref("mail.server.server5.trash_folder_name", "Deleted Items"); To know what to use in place of 'server5' for your config, look for something like user_pref("mail.server.server5.name", "John Work Mail"); Sorry if this is obvious, but none of the other comments in this defect made it clear to me, and I'm not sure what's happening with Bug 182274 (UI for this pref)
Updated•20 years ago
|
Product: MailNews → Core
Comment 99•19 years ago
|
||
*** Bug 281967 has been marked as a duplicate of this bug. ***
Comment 100•19 years ago
|
||
*** Bug 320573 has been marked as a duplicate of this bug. ***
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•