475511 - Rename "Unsent Messages" to "Outbox" in the backend

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Backend, defect)

Description

[[e-mail]]

"Unsent Messages" is a bad special folder name because it contains a space. It only worked by accident (see bug 466261). We should rename it to Outbox.

Comment 1

[[e-mail]]

Attachment: Possible patch

I think I may have gone overboard with the s/Unsent Messages/Outbox/g since I now have two Local Folders subfolders named Outbox...

Comment 2

[Ed]

Note that one 'Outbox' points to
 /<Profile>/Mail/Local Folders/Outbox

and the other 'Outbox' points to
 /<Profile>/Mail/Local Folders/Unsent Messages

Using Win XP-PRO SE-3 and SM 2.0.6pre 20100604

Comment 3

[Karsten Düsterloh]

Comment on attachment 448000 [details] [diff] [review]
Possible patch

All these string compares are rather worrisome - most of the time we could (and should, especially in JS) just check the nsMsgFolderFlags. Due to these string compares, this patch will not work correctly with old profiles, because the string match for "Outbox" will fail there (sometimes!). Hence you get two Outboxes, one newly created with the file name "Outbox", one old one named "Unsent Messages". Both are shown as specialFolder-Outbox in the UI, though, because you deliberately (I hope *g*) didn't change the or-clause in <http://mxr.mozilla.org/comm-central/source/mailnews/base/util/nsMsgDBFolder.cpp#3627>.

So the code needs to take care of an already existing "Unsent Messages" folder as well in several places.

(Sidenote: Could add 
  [diff]
  git=true
  showfunc=true
  unified=8
to your hgrc? Function names are really handy, especially if your context is just three lines short.)

>diff -r 1b14adf0e98a mailnews/import/public/nsIImportMail.idl
> #define kDestTrashFolderName "Trash"
>-#define kDestUnsentMessagesFolderName "Unsent Messages"
>+#define kDestUnsentMessagesFolderName "Outbox"
> #define kDestSentFolderName "Sent"
> #define kDestInboxFolderName "Inbox"

It'd really help if we had defined all these hard-coded names only once. :-/

>diff -r 1b14adf0e98a suite/mailnews/mail3PaneWindowCommands.js
>@@ -101,7 +101,7 @@
> 					//dump("specialFolder failure: " + ex + "\n");
> 				} 
>         if (specialFolder == "Inbox" || specialFolder == "Trash" || specialFolder == "Drafts" ||
>-            specialFolder == "Sent" || specialFolder == "Templates" || specialFolder == "Unsent Messages" ||
>+            specialFolder == "Sent" || specialFolder == "Templates" || specialFolder == "Outbox" ||
>             (specialFolder == "Junk" && !CanRenameDeleteJunkMail(GetSelectedFolderURI())) || isServer)
>           canDeleteThisFolder = false;

This is especially bad (more in other JS below, which I don't comment on). First we use getSpecialFolderString to turn the flags into strings and then compare the strings. *yuck*

>diff -r 1b14adf0e98a suite/mailnews/msgMail3PaneWindow.js
>--- a/suite/mailnews/msgMail3PaneWindow.js	Fri May 28 13:49:09 2010 +0200
>+++ b/suite/mailnews/msgMail3PaneWindow.js	Fri May 28 15:07:08 2010 +0100
>@@ -1570,7 +1576,7 @@
>   if (aFolder.flags & 0x0100) // MSG_FOLDER_FLAG_TRASH
>     return "Trash";
>   if (aFolder.flags & 0x0800) // MSG_FOLDER_FLAG_QUEUE
>-    return "Unsent Messages";
>+    return "Outbox";
>   if (aFolder.flags & 0x0200) // MSG_FOLDER_FLAG_SENTMAIL
>     return "Sent";
>   if (aFolder.flags & 0x0400) // MSG_FOLDER_FLAG_DRAFTS

What about mailnews/base/util/folderUtils.jsm::getSpecialFolderString?
But actually, both getSpecialFolderString are true atrocities worth dying.

Comment 4

[[e-mail]]

(In reply to comment #3)
> (From update of attachment 448000 [details] [diff] [review])
> All these string compares are rather worrisome - most of the time we could (and
> should, especially in JS) just check the nsMsgFolderFlags. Due to these string
> compares, this patch will not work correctly with old profiles, because the
> string match for "Outbox" will fail there (sometimes!).
Hmm, I wonder whether there's any way around that, or will the Outbox's URI forever internally be Unsent Messages?

> (Sidenote: Could add 
>   [diff]
>   git=true
>   showfunc=true
>   unified=8
> to your hgrc? Function names are really handy, especially if your context is
> just three lines short.)
Different .hgrc, sorry ;-) I'll try to add showfunc=true to all my .hgrc files, but git=true doesn't show the source changeset, which is why I only use it when git features are necessary, and unified=8 increases the chances of merge conflicts, but of course I'll do it on request.

> This is especially bad (more in other JS below, which I don't comment on).
> First we use getSpecialFolderString to turn the flags into strings and then
> compare the strings. *yuck*
Yeah but rewriting callers of getSpecialFolderString is a separate bug ;-)

> What about mailnews/base/util/folderUtils.jsm::getSpecialFolderString?
Ooh, shiny thing! The next revision needs to use that version :-)

> But actually, both getSpecialFolderString are true atrocities worth dying.
Sure, but we need to create atoms for the folder pane tree view CSS somehow...

Comment 5

[David :Bienvenu]

Comment on attachment 448000 [details] [diff] [review]
Possible patch

clearing review request based on Karsten's comments. We might need to migrate the pref *and* rename the file on disk to truly fix this.

Comment 6

[[e-mail]]

Attachment: SeaMonkey UI only

So, it turns out that we already import folderUtils.jsm and we therefore never use the version in mail3PaneWindowCommands.js causing two regressions :-(

Comment 7

[[e-mail]]

Attachment: Branch regression fix

This is a branch patch that fixes the following regressions:
* No "Send Unsent Messages" in the context menu for Outbox.
* Outbox is deletable, but shouldn't be.

Comment 8

[[e-mail]]

Pushed changeset 1e4c588cac1c to comm-central.

Comment 9

[[e-mail]]

Pushed changeset bd51b52dd746 to releases/comm-1.9.1