In mockups related to bug 449691 ( improved message reader ) we have been using an Archive button for "processing" or moving messages from the Inbox to an archive folder. To make this action a reality we need the backend bits implemented. I've included the wiki page with the Archive designs. https://wiki.mozilla.org/Thunderbird:Message_Archive Initial implementation should * Have a default Archive folder selected on the remote system * create the Archive folder upon the first use of the Archive button. * Provide at least a status bar message notification of the message move I think our first use, where the folder is created, might require an extra explanation to the user of what is happening vs. routine usage of the archive button. I'm not sure what the best method for notification of the first usage should be, however it is probably the optimum time to do something.

xref bug 93094.

In my own usage, a more typical scenario to archive mail from an arbitrary folder which is the target of a filter, i.e. not from inbox folder, and I wouldn't want that mail to archive where inbox gets archived.

I think initial versions of the Archive will likely just be simple moving mail from the Inbox to an Archive folder. Once we've settled the user experience issues that come along with the simple end we can add in filtering / scripting to the archive button such that it becomes more of a "process mail" button than just archive. This should work well for people who want to archive from any folder or archive mail into mail specific folders. And it looks like we should have someone lined up to work on this very soon, hopefully he'll make a comment or two about his approach.

I am interested in working on this project. Initially I plan on doing what Bryan suggested with the simple moving of the mail and add more complexity in future iterations.

Consider the people that are using a strong folder structure under inbox or such. Out of view doesn't mean out of structure .. Archive should remember that in order to be useful to them or they'll not use it. Either by reproducing the folder structure under it or by having some metadata/tag, which may lead to same thing.. I do not use that much so I cannot give clear scenario and wish, but you know many people use that. [Or this idea may just be the opposite of the archive concept you propose ..]

An idea I had discussed a week ago or so was that by using Gloda we can know where the rest of a conversation exists even though it's buried in folders. So if a person files one message in a certain folder we can implement the archive in such a way that it places the next message in that conversation in the same folder. Could possibly solve this problem. We'll have to figure out a way to inform the user of where the message will go when they archive it. Some suggestions were something like this: ( Archive ) /to Inbox->Biz->Mozilla/ Essentially providing some subtle text about where the Archive action will send your message. Of course the initial version of Archive will have to stick to simply moving any message to a single folder. Once the simple implementation is done we can move toward advanced setups.

- updating platform flag - Bartosz: any progress so far? Feel free to email me with questions

Here's a screenshot of my initial WIP patch that should be coming

Here's the patch. This adds initial support for the Archives folder Account Settings option. Things that are left to do: * Rename the msg flag from Unused3 to Archives (easy) * Continue changing the msgDBFolder to follow the code paths for "drafts" (difficult)

Shouldn't that be "Archive" without the s?

> Shouldn't that be "Archive" without the s? Ah, yes that should drop the trailing 's'. good catch!

To clarify the last comment after OOB chat w/ bryan: the flag name should be Archive, but the parent folder is supposed to be Archives.

This WIP adds an Archive menuitem to the Message menu, and an "archive" button to the message reader. They both work. This patch ignores bryan's patch, and just puts stuff in a toplevel Archives folder based on the first message. The two patches should be merged at some point. A few things to note before I forget: 0) I don't currently do anything on "first archive", but it should be relatively easy to handle that, as long as we know what it is we want to do. I'd suggest we defer that discussion until the Activity Manager & super status bar has landed. 1) this patch should be tweaked to chunk up patches based on server as well as month, in case the set of selected messages spans servers. 2) this patch doesn't deal well with IMAP servers for which every folder needs to be a child of INBOX unless the IMAP personal namespace is set (bienvenu seemed to think this was a bug that should be handled at a lower level. 3) I deliberately didn't use the "traditional" SetNextMessageAfterDelete() mechanism the "standard" way because that mechanism doesn't deal well with single UI operations that result in several IMAP moves. Specifically, if multiple messages spanning multiple destination archive folders were selected, then the DBview was telling the front-end to select messages that the next batch was already moving away, resulting in error dialogs. Also, it was slowing things down a lot. The approach I used here is to call SetNextMessageAfterDelete() to figure out what message we want, then reset that global (ugh) so that the DBview doesn't result in any selection updates, and then do the selection setting myself at the end of the last copy operation. That last bit needs to be changed to be done at the end of _all_ copy operations, as I wouldn't be surprised if the last copy operation started isn't necessary the last one to finish. This makes archiving quite responsive compared to the alternative -- I'd like to know what problems aren't handled, given that I'm deliberately bypassing an existing mechanism which appears horribly tricky. 4) I don't set the Archive bit on newly created Archive folder. TBD. 5) the structure of the archives folder (figured out with various people in mountain view) is: Archives/2008/2008-04 (for april 2008, for example). The reasons for this were: - to not end up with neither too small nor too huge folders, for IMAP health as well as for MSF performance. Monthly seems a reasonable best guess. - to sort well on disk To note about this directory structure in particular: - the plan is to localize the presentation of the archives folder, not its structure on disk. So it will say April 2008 in en-US, Avril 2008 in fr-FR, etc. - the plan is to leverage the JS folder pane to provide archive-specific UI for those folders, particularly suited to the tasks that people do when managing their archives. These last two points should likely be the topic of another bug having to do with the UI of the archive folder. - we need to figure out a way to properly treat Archive-flagged folders so that they get synced appropriately for IMAP -- we're archiving messages in folders corresponding to the From header of the message, and so it is possible to archive a message that's been sitting in an inbox for months in an "old" folder. At the same time, we want other Thunderbird installations pointing at the same IMAP server to detect the new messages, but we don't want Thunderbird to auto-sync every archive folder too frequently.

Oh, and I should add that something that I expect to do at some point is to open my 100,000 Archive folder, select all and hit A. It'd be good to test that before I (or anyone else) does it with real mail. =)

Does BatchMessageMover want to be its own service or JS module, rather than living directly in the chrome?

(In reply to comment #15) > Does BatchMessageMover want to be its own service or JS module, rather than > living directly in the chrome? Good idea. One thing that could make sense is to split the stuff that interacts with the view (figuring out selected messages, managing selection when done) from the creation of batch moves. I'll need to do that anyway when implementing archive on conversations. In general, I think we need to do a higher level review of the APIs having to do with the view & message handling. There's way too much tying together of logic with view stuff.

I'll drive this into the tree, as it were. One thing Davida pointed out over IRC - we probably want to treat archive in GMail accounts specially - they shouldn't create the per-month archive folders, and should just do whatever gmail does when you archive a message, or the imap equivalent thereof.

Right, for GMail actually want to delete the message, without moving it to the trash. This will remove it from the Inbox but the message remains in [GMail]/All Mail. See the GMail IMAP actions table for other actions: http://mail.google.com/support/bin/answer.py?answer=77657&ctx=sibling

I've un-bitrotted this patch, and started to make the initial imap folder creation work. I'll be fixing some core issues this patch has brought up, including bug 470011, and the fact that creating an imap folder at the root level doesn't pay attention to the personal namespace (e.g., make the folder a child of the INBOX for servers where the personal namespace is "INBOX.")

we seem to be creating a different folder for every hour of the day, which seems a bit excessive :-) I've gotten the two patches to work together, such that we use the identity's idea of the archive folder. But I believe we really want to archive imap messages in the imap account instead of using the default local folders account, by default. I'm having a lot of fun with the folders not showing up in the UI until a restart. This feature is quite the stress test on a bunch of different code :-)

this is what I have so far - it contains some diffs that are in patches in separate spin-off bugs, which I'll land first.

this should get into b2

(In reply to comment #20) > I've gotten the two patches to work together, such that we use the identity's > idea of the archive folder. But I believe we really want to archive imap > messages in the imap account instead of using the default local folders > account, by default. Yes, this is just a fault of my not understanding all the code for the Drafts folder implementation. We really do want to default to the IMAP account instead of the Local Folders.

Also, just to note that I think we can handle the GMail case in a follow up patch. This is a large enough change that I think it's important that we get some good testing started ASAP. For GMail users that are testing they will get some labels created that they don't want but no harm will really be done.

it's a bit tricky to default to a non-existent folder on a server (we don't have any defaults that behave that way now), though I'm sure I can figure out a way. I also need to make it so the Archives folder name is displayed localized, though I think I'll make the folder name on the server be "Archives".

this is some plumbing work to support an Archives folder. I've changed getFolderPref to fall back on the default folder on the server, if possible, and then the default folder on the local folders account. This allows us to have a default on the current server, which wasn't easy before. Currently, we default to the local folders account for things like drafts, but we don't want to do that for Archives.

none of this code should currently get exercised, until more of this work lands.

(In reply to comment #26) > I've changed getFolderPref to fall back on the default folder on the server, if > possible, and then the default folder on the local folders account. This allows > us to have a default on the current server, which wasn't easy before. > Currently, we default to the local folders account for things like drafts, but > we don't want to do that for Archives. I thought Drafts, Templates, Sent, Trash and Junk always defaulted to the server, not local folders.

I guess someone must set them up during account creation?

yes, the account wizard and copies dialog both seem to do that: http://mxr.mozilla.org/comm-central/source/mailnews/base/prefs/resources/content/am-copies.js#356 http://mxr.mozilla.org/comm-central/source/mailnews/base/prefs/resources/content/AccountWizard.js#650 http://mxr.mozilla.org/comm-central/source/mailnews/base/prefs/resources/content/AccountWizard.js#678 So that's an alternative to what I've done - I didn't know about/remember the allows_specialfolders_usage server attribute

Comment on attachment 354888 [details] [diff] [review] add archives folder flag, archives folder property to identity, and localized Archives folder name oh, yes, the reason I did it this way is so that existing profiles will automatically get their archives folder on the imap server. Also, at least for TB, the account wizard is getting replaced, so that would be one less piece of logic I'd need to redo for the new account wizard. So I think I'll just tweak the nsMsgIdentity::getFolderPref code to check that allows_specialfolders_usage server attribute.

Comment on attachment 355157 [details] [diff] [review] use GetDefaultCopiesAndFoldersPrefsToServer >+archiveFolderName=Archives This was just the most obvious example of the archive/archives mix but you've been horribly inconsistent throughout this patch; pick one and stick to it! > PRUnichar *nsMsgDBFolder::kLocalizedUnsentName; > PRUnichar *nsMsgDBFolder::kLocalizedJunkName; > PRUnichar *nsMsgDBFolder::kLocalizedBrandShortName; >+PRUnichar *nsMsgDBFolder::kLocalizedArchivesName; Nit: Brand should come last, or better still, separated with a blank line. (Same goes for the NS_Free section; the init code is already separated.) >+ nsCOMPtr<nsISupportsArray> servers; >+ rv = accountManager->GetServersForIdentity(this, getter_AddRefs(servers)); >+ NS_ENSURE_SUCCESS(rv,rv); >+ PRUint32 cnt = 0; >+ servers->Count(&cnt); >+ if (cnt > 0) >+ { >+ nsCOMPtr<nsIMsgIncomingServer> server(do_QueryElementAt(servers, 0, &rv)); >+ if (NS_SUCCEEDED(rv)) You probably don't have to count them first, as this will fail anyway. [And you didn't check the rv for Count ;-) ] >+ retval.Append("/"); >+ retval.Append(folderName); Is it worth changing the macro to prepend the "/" automatically i.e. rv = getFolderPref(_prefname, folderPref, "/" _foldername, _flag); \ [Otherwise use retval.Append('/'); instead]

re Archive vs. Archives - that was a decision that I inherited, not one that I originated. The decision was to call the folder flag "Archive", but the folder "Archives". Perhaps I should go back and edit all the patch to call all the vars "Archive" and leave the folder name "Archives" so that the only inconsistency would be the display name.

(In reply to comment #13) > 5) the structure of the archives folder (figured out with various people in > mountain view) is: > > Archives/2008/2008-04 This hierarchy seems to be unncessarily deep. Why do we need the extra year subdir? Actually I'd prefer there not to be a month at all as I don't think the amount of archived mail per month folder will be large on average. Then we also wouldn't have to bother with month names which will cause the folders to be sorted non-chronologically (and folder name loclizations).

Month basis can be need for the most users, not deletes mails with multimedia. I see a lot of "folder is larger 2 gb size" problem for less than a year. I am using mail archiving, and not use month level - but I remove all large attachements. Maybe, there can be an option, and archiver can propose more optimal way depending on inbox size and message count?

I dare say if you need to archive over 2 or 4 GB of mail per month, you're mailing habits are highly unusual. Of all the bugs re the max 4GB / folder I don't recall anyone ever complaining about the limit for folders - except for the Inbox. (And Trash, when people don't know they must compact folder once in a while.)

If it's about the "never throw away anything" policy Gmail tried, even Gmail had to add the delete button. Big surprise deletion *is* on of the most powerful labeling mechanisms, especially with the amount of junk (non-spam) you get...

I didn't say about "2 GIG per month", I said about "per year". But have 2-3 gig in one file os not comfortable. This issue created for help users, that not toss mail by boxes. This issue created as a decision for people who not keep in mind that inbox can have any limits, who don't know about email really have a volume, and inbox and trash need to be purged time to time. I often fix broken mail systems of that people, and please, don't tell to **me**, how to **right** use email. Tell it to all email users of all ages - from 5 to 85 years. Computer must work without any harrasements to users, without any user-made "housekeeping". This issue is one more step to computers that "just works", and should perform this task.

(In reply to comment #36) > This hierarchy seems to be unncessarily deep. Why do we need the extra year > subdir? > > Actually I'd prefer there not to be a month at all as I don't think the amount > of archived mail per month folder will be large on average. Then we also > wouldn't have to bother with month names which will cause the folders to be > sorted non-chronologically (and folder name loclizations). The primary criterion for picking a monthly folder structure was that we'll need to periodically open the corresponding MSF files for autosync, and it really hurts to open huge MSF files. Also, Bryan and I felt that years and months are concepts that people understand really well, and which correspond to common searches, meaning that the file structure will likely map normal organizational patterns. Archive folders are intended to be displayed with custom logic, which can take into account l10n issues. It makes sense for me to use numbers on the server, as long as the UI uses appropriate l10n names.

the time it takes to open a .msf file is miniscule compared to the time it takes to do an imap sync of a large folder - I don't think the .msf opening time is really an issue.

(In reply to comment #42) > the time it takes to open a .msf file is miniscule compared to the time it > takes to do an imap sync of a large folder - I don't think the .msf opening > time is really an issue. I knew i had my argument for keeping file sizes down wrong. thanks for correcting the record.

this adds the archive folder to the copies sheet - I'm assuming that SeaMonkey is going to want some sort of Archive feature, though perhaps not exactly the same as TB, so I've left this in SM. Let me know if I should #ifdef MOZ_THUNDERBIRD this...

this is the TB front end work to make the archive command work, and to add an archive button to the message header area. I think all the work is now in patches attached to this bug.

(In reply to comment #10) > Shouldn't that be "Archive" without the s? I disagree. Drafts and Templates have an s, and so should Archives. As a result of the mix up, attachment 355672 [details] [diff] [review] has entity name mismatches!

Comment on attachment 355672 [details] [diff] [review] add prefs ui to pick an archive folder. >+ SaveFolderSettings( gArchiveRadioElemChoice, >+ "messageArchive", >+ gArchiveFolderWithDelim, >+ "msgArchiveAccountPicker", This was the worst spot for copied trailing spaces, but git-apply claims there are 18 whitespace errors in all. >+ <label value="&keepArchive.label;" control="messageArchive"/> Archives. >+ value="0" label="&archiveFolderOn.label;" >+ accesskey="&archiveFolderOn.accesskey;"/> Archives. Archives! </rant>

A single message in a drafts folder is a draft. Is a single message in an Archive folder an archive? I'm happy to make it all Archives other than the folder flag and if someone doesn't like that, they can change it themselves :-)

Comment on attachment 355157 [details] [diff] [review] use GetDefaultCopiesAndFoldersPrefsToServer >@@ -102,6 +102,7 @@ > attribute ACString fccFolderPickerMode; > attribute boolean fccReplyFollowsParent; > attribute ACString draftsFolderPickerMode; >+ attribute ACString archivesFolderPickerMode; > attribute ACString tmplFolderPickerMode; I think it would be worth adding some documentation (just one-liners probably) as to what these folder picker modes relate to. It certainly took me a bit of digging to realise. > attribute ACString draftFolder; >+ attribute ACString archiveFolder; > attribute ACString stationeryFolder; Again, this needs a little documentation I think just to say it will set/return the name of the folder to use for the identity. >diff --git a/mailnews/base/public/nsMsgFolderFlags.idl b/mailnews/base/public/nsMsgFolderFlags.idl >--- a/mailnews/base/public/nsMsgFolderFlags.idl >+++ b/mailnews/base/public/nsMsgFolderFlags.idl >@@ -98,8 +98,8 @@ > const nsMsgFolderFlagType Inbox = 0x00001000; > /// Whether this folder on online IMAP > const nsMsgFolderFlagType ImapBox = 0x00002000; >- /// Used to be for category container >- const nsMsgFolderFlagType Unused3 = 0x00004000; >+ /// Whether this is an archive folder >+ const nsMsgFolderFlagType Archive = 0x00004000; > /// This is a virtual newsgroup > const nsMsgFolderFlagType ProfileGroup = 0x00008000; This file should probably have a uuid bump r=me with those fixed.

Comment on attachment 355672 [details] [diff] [review] add prefs ui to pick an archive folder. Something is causing the achiveFolderPickerMode not to be save or restored - it always comes up on "Other" whenever I open the account settings dialog. The archive folder pref does seem to be set correctly though. >+ SaveFolderSettings( gArchiveRadioElemChoice, >+ "messageArchive", >+ gArchiveFolderWithDelim, >+ "msgArchiveAccountPicker", >+ "msgArchiveFolderPicker", >+ "identity.archiveFolder", >+ "identity.archiveFolderPickerMode" ); >+ nit: some of these lines of blank space on the end, and the last line has whitespace on it when it doesn't need to (ditto with other lines you've added). >+ <hbox align="center"> >+ <label value="&keepArchive.label;" control="messageArchive"/> >+ </hbox> This doesn't match the strings. >+ <radio id="archive_selectAccount" >+ oncommand="setPickersState('msgArchiveAccountPicker', 'msgArchiveFolderPicker', event)" >+ value="0" label="&archiveFolderOn.label;" >+ accesskey="&archiveFolderOn.accesskey;"/> nor do these two. >+ <menulist id="msgArchiveAccountPicker" >+ <menulist id="msgArchiveFolderPicker" There's a tab on these lines.

Comment on attachment 355157 [details] [diff] [review] use GetDefaultCopiesAndFoldersPrefsToServer >+++ b/mail/locales/en-US/chrome/messenger/messenger.properties >+archiveFolderName=Archives >+ bundle->GetStringFromName(NS_LITERAL_STRING("archivesFolderName").get(), >+ &kLocalizedArchivesName); >+++ b/suite/locales/en-US/chrome/mailnews/messenger.properties >+archiveFolderName=Archives I've just tested all these patches together. These don't match, and so we then crash (because kLocalizedArchivesName is null and nsDependentString doesn't like that).

Comment on attachment 355674 [details] [diff] [review] TB front end archive work messenger.properties didn't seem to apply, but I expect that's because its diff is already in one of the other patches. Should Bryan be doing ui-review on this patch and the account manager? Probably follow-up bugs, but I'll mention them here, please make sure they get filed if you aren't addressing them: - IMAP folders don't get tagged for offline use when they are created. Is that expected, should that happen? - No archive option in context menu (especially useful/expected for multiple messages) - Should the archive button/menu option really be displayed/enabled when you are viewing a message in an archive folder? - Archive button is displayed for RSS messages, but does nothing. >diff --git a/mail/base/content/mailWidgets.xml b/mail/base/content/mailWidgets.xml >--- a/mail/base/content/mailWidgets.xml >+++ b/mail/base/content/mailWidgets.xml >@@ -2098,7 +2098,10 @@ > <xul:button anonid="hdrForwardButton" label="&forwardButton.label;" > oncommand="MsgForwardMessage(event);" > observes="button_forward" class="msgHeaderView-button hdrForwardButton"/> >- <xul:button anonid="hdrJunkButton" label="&junkButton.label;" >+ <xul:button anonid="archiveButton" label="&archiveButton.label;" >+ oncommand="MsgArchiveSelectedMessages(event);" observes="button_archive" >+ class="msgHeaderView-button hdrArchiveButton"/> Seeing as you're observing/setting up button_archive, why not goDoCommand('button_archive') as well? >+ <xul:button anonid="hdrJunkButton" label="&junkButton.label;" nit: no need to change this line (indent is now wrong). >+ // subtle: >+ SetNextMessageAfterDelete(); // we're just pretending >+ this.messageToSelectAfterWereDone = gNextMessageViewIndexAfterDelete; >+ gNextMessageViewIndexAfterDelete = -2; >+ nit: whitespace on blank lines again. >+ let msgHdr = messenger.msgHdrFromURI(selectedMsgUris[i]); >+ >+ let rootFolder = msgHdr.folder.server.rootFolder; >+ //if (rootFolder instanceof Components.interfaces.nsIMsgFolder) >+ //dump("Found root folder for archiving: " + rootFolder); >+ >+ let msgDate = new Date(msgHdr.date / 1000); // convert date to JS date object >+ //dump("msgDate = " + msgDate.toString() + '\n'); >+ let msgYear = msgDate.getFullYear().toString(); >+ //dump("msgYear = " + msgYear + '\n'); >+ let monthFolderName = msgDate.toLocaleFormat("%Y-%m") >+ //dump("monthFolderName= " + monthFolderName + '\n'); >+ let dstFolderName = monthFolderName; >+ //dump("dstFolderName= " + dstFolderName + '\n'); I think we can probably remove these commented out lines (in multiple places) >+ if (!subfolder.containsChildNamed(dstFolderName)) >+ { >+ //dump("Creating month folder\n"); >+ subfolder = subfolder.addSubfolder(dstFolderName); >+ dump("create folder " + subfolder.URI + '\n'); we shouldn't really need this now should we? >+ OnStopCopy: function(aStatus) >+ { >+ try { ... >+ } catch (e) { >+ dump("Exc: " + e + '\n'); >+ } Why do we need this try/catch? I'd prefer if it wasn't there or at least not over the whole function - it is very easy to mask errors otherwise.

(In reply to comment #52) > - No archive option in context menu (especially useful/expected for multiple > messages) And in the context menus of folders? (e.g., for archiving a completed project)

this accumulates the various patches into one, for review. I've addressed most of your comments, except for a couple: I don't believe we need to change uuids for new constants Using go_doCommand('button_archive') does not work - there's a downstream error that I don't quite understand, but the code that's there does work :-) Re the bugs you noticed, I'll file those, but I'd like to get the basic stuff in sooner rather than later. The one that worries me a bit more is not configuring the archive for offline - I think I know why that is, and it should be easy to fix.

this makes new archive folders get configured for offline use, by default.

Comment on attachment 356010 [details] [diff] [review] cumulative fix nit: there are still a few whitespace issues (http://beaufour.dk/jst-review/?patch=356010&file=) >+ // URI for the fcc (Sent) folder > attribute ACString fccFolder; nit: doxygen requires /// for single-line comments. >+ // these attributes control whether the special folder pickers for >+ // fcc, drafts,archives, and templates are set to pick between servers >+ // (e.g., Sent on accountName) or to pick any folder on any account. >+ // "0" means choose between servers; "1" means use the full folder picker. > attribute ACString fccFolderPickerMode; >- attribute boolean fccReplyFollowsParent; > attribute ACString draftsFolderPickerMode; >+ attribute ACString archivesFolderPickerMode; > attribute ACString tmplFolderPickerMode; nit: if you make this: /** * @{ * <the comments> */ attribute attribute ... /** @} */ Doxygen can then pick this up as a group comment and will apply it to all of the attributes This also applies to the special folder attributes. r=me with those fixed.

fix checked in - I'll file followup bugs on these: - No archive option in context menu (especially useful/expected for multiple messages) (though I don't know why anyone would use a context menu when there's a single key shortcut available) - Hide or disable the archive button when viewing a message in an archive folder. - Either fix or hide the Archive button for RSS messages

Bug 473212 is the tracking bug for remaining issues.