+++ This bug was initially created as a clone of Bug #486827 +++ Spun off per bug 486827 comment #19 to provide an easier access to the archive_granularity attribute for a server. A possible location would be the Copies & Folders page of the Account Manager, e.g., as a drop-down menu right underneath the selector for the Archives folder location. Possible values of archive_granularity: 0 = Flat archive (no subfolders) 1 = subfolders in Archives for year (1-level structure, default) 2 = subfolders for year, then for month (2-level structure)

Currently the archiving granuality is per server, which is really suboptimal as archive folder is per identity. This patch makes granuality per identity, and adds UI to choose how to archive. Past string freeze, so i don't know if this has a chance to make tb3. Anyway, at least the patch is more testable with UI. With this we should be able to close bug 476217 too. Screenshot coming up. SeaMonkey: - run out of accesskeys on that pane it seems :( - I just copy-pasted over the archiving function from tb. Was missing a few fixes there anyway...

Screenshot.

The word "flat" feels wrong to me -- it implies a very widely spread tree, whereas I think what it means is "just one folder", right?

I'm not sure about the term "archive hierarchy" either. Maybe something along the lines of Archive messages "by month" "by year" "in one folder" - ugly, I know...

Not sure it's the best word or not, but googling "flat hierarchy" seems to support that it is an hierarchy with only same-level children. I'll look in to bienvenu's suggestion.

I was going to suggestion a new dialog window for this. For reasons of having more room to explain the choices given as well as giving space for extensions to offer alternate archiving patterns. Basic proposal would be this: * replace the Archive Hierarchy radios with an ( Archive Options... ) button * create new dialog window with the radio buttons and examples near by. +-------------------------------+ | | (o) Flat Folder | /Archives/ | | (o) Yearly Archived Folders | /[-] Archives/ | |--- 2009 | +--- 2008 | | (o) Yearly Archived Folders | /[-] Archives/ | |--[-] 2009 | | |--- 2 | | +--- 1 | +--[-] 2008 | |--- 12 | +--- 11 | +-------------------------------+ I think we might actually want the examples beside the radio buttons but that was harder to do in ascii art. For these types the examples could all be done in JS such that the year and folder are close to current. Future Archive schemes provided via extensions could just append to that dialog for their options. I could try a real mockup some time later when I can breath a little

This makes the archive granuality per identity. I'd really like to get this done for tb3 as otherwise we'll have to have migration later. Since archive folder is per identity the mismatch makes for impossible ui. This part is the backend only. I'll attach the (unfinished) front end patch for testing in a bit.

Unfinished frontend for testing only.

Comment on attachment 407330 [details] [diff] [review] proposed fix, v2 (backend only) this patch has bit-rotted - can you attach a non-bittrotted one? I tried to make it as little bit-rotted as possible when I fixed the archive batching code... One reason archive granularity is per-server is that different servers tend to have different tolerance for giant folder. Making it per-identity blurs that a little but the benefit of having a consistent UI is probably greater... Is this an unrelated change? : - - // We're just going to select the message now. - let treeView = gDBView.QueryInterface(Components.interfaces.nsITreeView); - treeView.selection.select(this.messageToSelectAfterWereDone); - treeView.selectionChanged();

Backend part - attachment 407331 [details] [diff] [review] can still be used to test. For multi-msg archive there is still a minor issue, as the messages will all archive according to the first selected msg. We can fix that later though. I can't really say i understand why the archiving is implemented like it is, the batching seems odd to me (especially that it's in two functions.)

Comment on attachment 408437 [details] [diff] [review] proposed fix, v3 (backend) this still fails to apply on trunk or 1.9.1 branch for me: patching file mail/base/content/mailWindowOverlay.js Hunk #1 FAILED at 1283. Hunk #2 succeeded at 1352 (offset 19 lines). 1 out of 3 hunks FAILED -- saving rejects to file mail/base/content/mailWindowOv erlay.js.rej

Unbitrotted again.

+1 for this feature, on a per-account basis!

just to note, bug 533815 is not the same but similar

Comment on attachment 408885 [details] [diff] [review] proposed fix, v4 (backend) There isn't much UI here really. My only concern is that a per identity setting would complicate our options UI. We should still have the Archive options available outside of the, very hidden, identity settings. Not sure how that will work out right now.

(In reply to comment #14) > just to note, bug 533815 is not the same but similar And by that I meant bug 522761

The obvious place I'd thought would be in the Account Settings, Disk Settings. Its too obscure hidden away in a preferences file. As a data point -- I've noticed that on Mac's its a particularly BAD idea to use the yearly archive, as it means the Archive file, which gets HUGE gets backed up every time TimeMachine runs, because it always changes. With monthly archives only the most recent month gets backed up.

(In reply to comment #1) > SeaMonkey: > - run out of accesskeys on that pane it seems :( > - I just copy-pasted over the archiving function from tb. Was missing a few > fixes there anyway... I don't think we should go that way. The SM MailNews reviewers (Karsten et. al.) probably won't like that (correct me if I'm wrong!), and there are some deliberate differences between the two implementations, e.g. this: (In reply to comment #9) > Is this an unrelated change? : > > - > - // We're just going to select the message now. > - let treeView = gDBView.QueryInterface(Components.interfaces.nsITreeView); > - treeView.selection.select(this.messageToSelectAfterWereDone); > - treeView.selectionChanged(); AFAICS that's not part of the latest patch, but it just shows. I'm already in the process of porting the missing Archive feature changes, e.g. in bug 573392, one at a time. I think it's very generous of you to do that for us/me, but personally I'd prefer to do it in separate, SM-only bugs. Of course this shouldn't cause stop-energy on your side, so I'd be fine if a change here temporarily broke (part of) SM's Archive functionality. I'd fix it in a follow-up, then. Of course Karsten's view on that may be different and is authoritative there.

Comment on attachment 408885 [details] [diff] [review] proposed fix, v4 (backend) this patch is severely bit-rotted. I'll try to revive it.

I've de-bitrotted this patch. I've also fixed the mozmill test. It was non-trivial because of the folder structure archiving that landed subsequently. I've also incorporated the SeaMonkey fixes to the folder structure archiving (thx to Jens for those fixes). I'm mainly asking for r from standard8 for the SeaMonkey changes and the mozmill fixes, but it would be good to have a quick look at the whole thing since I had a bear of a time getting the mozmill tests to pass.

I noticed that the original patch didn't remove the now unused default server prefs. Which reminds me, I'm not migrating the old prefs, since they were hidden, and I think we're going to have UI for the new prefs.

Comment on attachment 483007 [details] [diff] [review] remove unused default prefs Drive-by comment regarding the SM part: >+ let archiveGranularity; >+ archiveKeepFolderStructure; The second line doesn't look right. >+ if (srcFolder.server.type == "rss") { >+ // RSS servers don't have an identity so we special case the archives URI. >+ archiveFolderUri = srcFolder.server.serverURI + "/Archives"; Nit: Double space.

This fixes up the SM parts so that they work again. This has r=Standard8,bienvenu.

(In reply to comment #24) > Created attachment 484289 [details] [diff] [review] > Final fix for SM parts Nit: All four occurrences of "msgHdr.folder.server" in the patch can be replaced by "server" which is an in-scope local variable in the existing code (both TB and SM), defined just outside the area visible in the patch.

I've renamed this bug to reference that its just supporting the backend. If we want UI think we should do that in a different bug. Checked in: http://hg.mozilla.org/comm-central/rev/081b2f960f1b

> (comment #26) If we want UI think we should do that in a different bug. I've expected you to close this after the backend patch checked in, thus I'm already working on the follow-up bug and will clone this in a moment. ;-)

Continued in bug 607295, please comment there for the UI implementation.