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

RESOLVED FIXED in seamonkey2.1a2

Status

SeaMonkey
MailNews: Backend
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

({fixed-seamonkey2.0.6})

Trunk
seamonkey2.1a2
fixed-seamonkey2.0.6

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
"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.
(Assignee)

Comment 1

7 years ago
Created attachment 448000 [details] [diff] [review]
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...
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #448000 - Flags: superreview?(bienvenu)
Attachment #448000 - Flags: review?(mnyromyr)

Comment 2

7 years ago
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

7 years ago
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.
Attachment #448000 - Flags: review?(mnyromyr) → review-
(Assignee)

Comment 4

7 years ago
(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

7 years ago
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.
Attachment #448000 - Flags: superreview?(bienvenu)
(Assignee)

Comment 6

7 years ago
Created attachment 450130 [details] [diff] [review]
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 :-(
Attachment #448000 - Attachment is obsolete: true
Attachment #450130 - Flags: review?(mnyromyr)
(Assignee)

Comment 7

7 years ago
Created attachment 450132 [details] [diff] [review]
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.
Attachment #450132 - Flags: review?(mnyromyr)

Updated

7 years ago
Attachment #450130 - Flags: review?(mnyromyr) → review+

Updated

7 years ago
Attachment #450132 - Flags: review?(mnyromyr) → review+
(Assignee)

Comment 8

7 years ago
Pushed changeset 1e4c588cac1c to comm-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
Attachment #450132 - Flags: approval-seamonkey2.0.6?

Updated

7 years ago
Attachment #450132 - Flags: approval-seamonkey2.0.6? → approval-seamonkey2.0.6+
(Assignee)

Comment 9

7 years ago
Pushed changeset bd51b52dd746 to releases/comm-1.9.1
Keywords: helpwanted → fixed-seamonkey2.0.6

Updated

7 years ago
Target Milestone: --- → seamonkey2.1a2
You need to log in before you can comment on or make changes to this bug.