522761 - Need option 'keep folders scheme' under the archive feature to don't forget the messages organization

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, enhancement)

Description

[IagoSRL]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.14) Gecko/2009090216 Ubuntu/9.04 (jaunty) Firefox/3.0.14
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.4pre) Gecko/20091007 Shredder/3.0pre

A new boolean option to the archive feature that allows re-create the folders scheme when a message is archived from the original location.

And example: 
   have a message under the folder 'Projects' and inside under the folder 'Project X' (a filter move message to here),
   click hover 'archive message' and this go to the defined archive folder (for example, inside local folders -> Archived);
  
 this is the current behavior, but with the new option,

   thunderbird must move the message into the 'Archived' folder -> Projects -> Project X (if the folders don't exists, are created auto).

Reproducible: Always

Comment 1

[Ludovic Hirlimann [:Usul]]

confirming RFE. 

Bryan is this something that would make sense ?

Comment 2

[Mugros]

I want to second this.

I archive my emails manually into an archive folder with an identical folder structure since TB 2.

The question is not if this makes sense, which it does. To me it's obvious that dumping all messaged into one folder through the archive feature, like it is done now, doesn't make sense.

The archive feature right now is only useful for people with low email volume or who don't organize their email at all.

Comment 3

[Philip Gladstone]

As previously stated, the current scheme does not make any sense (why does it put messages into a folder named for the year of the first message moved?). As stated above, it also loses the folder structure.

Comment 4

[David :Bienvenu]

(In reply to comment #3)
> As previously stated, the current scheme does not make any sense (why does it
> put messages into a folder named for the year of the first message moved?).
That's a bug that has been fixed in nightly builds and should be fixed in 3.01.

Comment 5

[Marko Macek]

I'd love to see several options:

[ ] don't archive from this folder (for unsorted mails - mostly inbox)

[ ] archive to a custom archive folder (+ year/month) [Folder...]

[ ] keep folder hierarchy (year/month + hiearchy)

Archive granularity is probably enough to be global, as it is now.


However, it seems to me that:
 - archives
 - folders
 - tags
are somewhat orthogonal issues. there's no perfect way to organize things in the current folder based only system (without perf issues and many views anyway).

Comment 6

[Bryan Clark (DevTools PM) [:clarkbw]]

I feel like I just commented on this, bug 533815

Comment 7

[Bryan Clark (DevTools PM) [:clarkbw]]

Also magnus made some progress on a granularity UI earlier in bug 517514

Comment 9

[Christophe Humbert]

Hello,

As said in 533815, I have extended an existing extension and it works under Exchange Imap and Cyrus Imap server.

If you want it ask for it

I still have an issue with the XUL part.
Kind Regards

Comment 10

[Christophe Humbert]

Attachment: First proposal

I have an extension somehow working.

This is a proposal to inculde the function directly in thunderbird. Need UI to set preferences and creation of one preference.

Kind Regards

Comment 11

[Christophe Humbert]

Attachment: Extensions to preserver folder structure

Comment 12

[IagoSRL]

Hi,
 Thank you Christophe for your work.

 I am testing it and works well with elements (mails and folders) under INBOX folder (I have defined 2 levels of granularity), but with elements in another 'main' folder ('Send Items', or another custom folder sibling of Inbox) Thunderbird freezes it, use more and more memory and I need close it with the task manager -windows xp-. When restart Thunderbird, I can see that under the archive folder there are three new folders: /2010/02/02 (duplicate the folder of the day), all empty.
 Testing without levels (single folder option), keep folders structure don't work.

Thank you again

Comment 13

[Christophe Humbert]

Hello Iago,

What type of servers are you using Imap or pop and if imap servers could you precise which one 
Do you have any message in error console or anything

I have not tested with the sent folders.

I have tested only with many subfolders under the inox one up to 5 levels and yes sometimes it was somehow frozen but i have no clue on what is the reason, maybe in the imap folder creation process.

I have some ideas to improve the code but it will help if you have any error message and if someone more acquainted with TB code could have a quick look because it is my first extension and back to coding after 5 years...

Kind REgards

Comment 14

[Ludovic Hirlimann [:Usul]]

Christophe do you intend to release it on AMO ?

Comment 15

[Christophe Humbert]

Hello Ludovic,

In fact it is existing on AMO as I am not the initial developper, but not in this version. I have only extended it to take into account to try to keep the folder structure and the initial dev told me it had not time ...

Plus there is a small XUL issue, I was not able to solve

And as you see it appears that it does not work for everybody

Kind Regards

Comment 16

[IagoSRL]

Hi Christophe,

 My server is IMAP Zarafa (http://www.zarafa.com/) version 6.30.9 Community (last stable).
 I tested again, disabling all extensions except yours:
 - When mail is in INBOX folder or any subfolder of INBOX: no problem, no alert in Error Console.
 - When mail is in another folder sibling of INBOX or subfolder of folder sibling of INBOX (like Send Items, or another you can create manually): Thunderbird freeze, memory consumption increases in more than double and then crash, the Mozilla Crash Report Agent appear and Crash ID: bp-febbcc90-0179-4d33-bbed-d75a12100211. Thunderbird restart and I cannot see any previous messages in Error Console.

 I think the problem can be related to the feature: when you 'Archive' a message in the INBOX, this is moved to the archive folder directly (not in a 'inbox folder' under the 'archive folder', example: ArchiveFolder/2010/02/Inbox/). Can be?

 I am learning how create a Thunderbird extension too and working in create my first.

 Thanks

Comment 17

[Christophe Humbert]

Hello Iago,

I think I know where the problem is. I have no time before end of next week to work on it...so please be patient 

Kind Regards

Comment 18

[timeless]

I filed comment 16's crash as bug 545657

Comment 19

[Christophe Humbert]

Attachment: Diif for patch

Hello

Here is the diff. I have added a preference defaulted to false.
This take care of preserving folder structure in case pref is set to true.
It will need UI as the rest of Archive settings/preferences

Hope This helps

PS: Don't be too harsh, I have not done js for years

Comment 20

[Christophe Humbert]

Attachment: Previous was wrong

Same comment as previous one

Comment 21

[Christophe Humbert]

Hello,

There is no test as I Don't know how to write one. If I can have help or someone want to write one, I would be grateful

Thanks

Comment 22

[Ludovic Hirlimann [:Usul]]

(In reply to comment #21)
> Hello,
> 
> There is no test as I Don't know how to write one. If I can have help or
> someone want to write one, I would be grateful

Look into mail/test/mozmill where we have plenty of examples and documentation is at : https://developer.mozilla.org/en/Thunderbird/Thunderbird_MozMill_Testing

Comment 23

[Christophe Humbert]

Attachment: Mozmill test case

Hello,

Included in this attachment a simplified mozmill test case with 3 levels of folder
No taking into account special folders like trash/junk/sent etc...

Kind Regards

Comment 24

[Ludovic Hirlimann [:Usul]]

(In reply to comment #23)
> Created an attachment (id=434548) [details]
> Mozmill test case

Bienvenu ping for reviews.

Comment 25

[David :Bienvenu]

Comment on attachment 433902 [details] [diff] [review]
Previous was wrong

thx for the patch, sorry for the delay - can I get a patch without tabs? We use two space indents, no tabs...

this comment isn't adding any info so we should just remove it:
+// New pref to keep folder structure
+pref("mail.server.default.archive_keep_folder_structure", false);

space after the // here:

+			//Detect the root folder of message

this var name is ambiguous. Something like startFolderLevel might be clearer. Initial might be better than start, also.

 This detects the most common case of INBOX. being the personal namespace, but you might want a more generic test here. It's a bit tricky because personal namespace uses the server's hierarchy delimiter (usually either '.' or '/') but the URI uses the canonical delimiter of '/'. I guess since this pref defaults to false, we don't have to worry so much about other personal namespaces at the moment...
+			if (folderURI[1] == "INBOX") 
+				startI = 2;

+	  let startI = 1;

Comment 26

[Christophe Humbert]

Attachment: Updated patch with space replacing tab

Hello David,

I have adressed your first comments.
As far as personal namespace, I don't know how to get around this for the moment

Kind Regards

Comment 27

[David :Bienvenu]

Comment on attachment 434548 [details]
Mozmill test case

is this supposed to be a replacement for test-message-commands.js? I don't see it changing the default pref for whether the folder hierarchy is preserved, which I think it would need to work...

Comment 28

[David :Bienvenu]

Comment on attachment 435648 [details] [diff] [review]
Updated patch with space replacing tab

this patch doesn't apply - patch complains about the lines starting here:

diff --git a/mail/base/content/mailWindowOverlay.js b/mail/base/content/mailWindowOverlay.js
--- a/mail/base/content/mailWindowOverlay.js
+++ b/mail/base/content/mailWindowOverlay.js
@@ -1328,22 +1328,30 @@ BatchMessageMover.prototype = {

I'll try to see if I can tweak it locally.

I'm sorry but I messed up my comments previously. I was hoping we could have a better variable name than startI, something like initialFolderLevel.

Comment 29

[David :Bienvenu]

Attachment: fix several problems with previous patch

Hi Christophe, I've attached a patch that applies cleanly (it looks like you edited the diff directly previously, which is very tempting, but can easily break the diff).

More importantly, your previous patch broke the non-folder-level archiving, such that we weren't setting the dstFolder if you had the default yearly archiving set. I've tweaked your patch a little to fix that. My understanding is that this is an either/or setting - if you pick the folder-level archiving, we ignore the granularity setting, so I re-ordered the code so that we check that first, and if not set, fall back to the old code (you had stuck the folder-level archiving in the middle of the other checks, which didn't seem right). I also cleaned out a remaining tab, and some spaces at the end of lines, and the variable name I mentioned before.

It would be great if you could test this version of the patch and make sure it works the way you want it to, and let me know. Thx!

Comment 30

[David :Bienvenu]

Comment on attachment 434548 [details]
Mozmill test case

clearing review request until I'm clear on how this is supposed to work.

Comment 31

[Christophe Humbert]

Hello David,

Thanks for the work you have done and will do better next time I hope.

I will test your patch and give you my comments.

Kind regards

Comment 32

[Christophe Humbert]

Hello David,

I have setup my linux machine and made a build with your patch (newbie on linux eheh but i have suceeded.)

The patch works well but it seems that the fact to keep folder structure overrides the archive year/month feature.

What I had in mind first was more something like that:

Folder A
   --> Folder A.1
      ---> Folder A.2
          Message 1 Year x Month 2
          Message 2 Year x Month 5
          Message 3 Year y Month 6

To have as archives if granularity set to 2
Archive
   ---> Year X
     --> Month 2
      --> Folder A (if A different from Inbox)
       --> Folder A.1
        --> Folder A.2
            Message 1
     --> Month 5
      --> Folder A (if A different from Inbox)
       --> Folder A.1
        --> Folder A.2
            Message 2

That is why my first patch put the folder structure after the year/month test.
Do you want me to try a new patch now that I have A linux box?

Kind REgards

Comment 33

[Sven Ulland]

I would suggest to introduce a formatting string so that the
archive structure can be easily customized by users. Having
a fixed structure is asking for trouble.

It could be an idea to have a pref like
'mail.server.default.archive_folder_structure_format' where
you can specify strftime-like formatting to suit your needs.
Define '/' to be the folder separator, and you can have things
like:

  "%Y/%m/"     -> 2010/04/<message>
  "%Y-%m/"     -> 2010-04/<message>
  "%Y/%B/"     -> 2010-April/<message>
  "%Y/%V/"     -> 2010/14/<message>    # ISO 8601 week number
  "Arch %Y/"   -> Arch 2010/<message>

Sorry for only supplying opinion and not code.

Comment 34

[Christophe Humbert]

Hello Sven,

Good point, I think we should follow your proposal in a follow-up bug.

Comment 35

[David :Bienvenu]

Christophe, ah, ok; I did not know what you intended the patch to do. I can try to rearrange the code so that it does that...

Comment 36

[Christophe Humbert]

Attachment: PAtch to work as explained in comment 32. No yet taking care of namespace

Comment 37

[Christophe Humbert]

David,

I have made a build with this patch and it works as I intented. 

Kind REgards

Comment 38

[David :Bienvenu]

Hi Christophe, 
  Did you make sure it works if the "keep folder scheme" pref is turned off?

thx, - David

Comment 39

[Christophe Humbert]

Hello David,

Yes I tested with pref turned off.

Do we make a follow-up bug for personal namespace and another one for sven suggestion?

Kind Regards

Comment 40

[Ludovic Hirlimann [:Usul]]

(In reply to comment #39)
 
> Do we make a follow-up bug for personal namespace and another one for sven
> suggestion?

yes !

Comment 41

[Christophe Humbert]

Hello,

I have added a follow up bug i.e bug Bug 558332 for Sven proposal.
I am waiting for David comment for namespace follow up bug.

Comment 43

[Nathan Tuggy (:tuggyne)]

Strictly speaking the dependency I just added isn't quite standard, but whatever....

At least this is moving along well!

Comment 44

[David :Bienvenu]

Attachment: patch with mozmill test

this tweaks the prev patch a bit to implement the per server pref for whether we keep folder structure on archiving, and extends the existing mozmill test to test the new pref.

Comment 45

[David :Bienvenu]

Christophe, one concern I have is if you try to archive messages that are already in the archive hierarchy, which I think we allow so people can re-archive after changing archiving prefs. Can you try that? I suspect we may end up duplicating the archive hierarchy when we do that with the keep folder structure pref turned on. Thx!

Comment 46

[Christophe Humbert]

Hello David,

I am off this week and away with limited connectivity. Could it wait for Monday 19th to be included in 3.1 or do you need it before.
BTW without testing I think you guess it right. 
But Generally speaking could we disable the archive button in archive folders?

Kind Regards

Comment 47

[Christophe Humbert]

I have tested and once the message archived the archive button is not present so it is not possible to rearchive the mail.

Kind REgards

Comment 48

[David :Bienvenu]

Christophe, thx for checking. However, the archive command ('A') is still enabled and does what we suspected. We don't disable archive so that people can re-archive after changing the prefs. We're likely to have a code freeze on Tuesday the 20th, though it might slip a day or two, but the 19th might be cutting it close. But if it doesn't make beta 2, we might be able to slip it in after beta 2, since the pref is not on by default and has no UI, yet.

Comment 49

[Christophe Humbert]

Hello David,

What is your proposal? It is when under archives to disabled the command ('A') or to treat that in the code i.e detect when we are under the archives folder and tell the user that his message(s) are already archived?


Kind Regards

Comment 50

[David :Bienvenu]

Hi Christophe,

My thinking was that in the case of a message in a folder under the Archives folder, we strip off the Archives folder hierarchy, though that obviously begs the question of which folder to use as the sub-parent in the Archives hierarchy, since we don't know where the message came from. Given that, I think we need to disable the archive command when keep folder structure is turned on and the selected message(s) are already under the Archives folder.

Comment 51

[Christophe Humbert]

David,

That was also my idea. How to implement that?

Kind REgards

Comment 52

[David :Bienvenu]

(In reply to comment #51)
> David,
> 
> That was also my idea. How to implement that?
> 
> Kind REgards

Look for cmd_archive in mail3PaneWindowCommands.js

right now, it's enabled when

        (gFolderDisplay.selectedCount > 0 );

if we're keeping folder structure, we want it enabled like button_archive is
        
        return gFolderDisplay.selectedCount > 0 && gFolderDisplay.displayedFolder &&
          !gFolderDisplay.displayedFolder.isSpecialFolder(
             Components.interfaces.nsMsgFolderFlags.Archive, true);

to tell if we're keeping folder structure, you could look at the first selected message,

        let selectedMessages = gFolderDisplay.selectedMessages;
        selectedMessages.length > 0 && selectedMessages[0].folder &&
               selectedMessages[0].folder.server.archiveKeepFolderStructure

Comment 53

[Christophe Humbert]

Hello David,

Why not disabled it generally speaking when a message is already archived and not only when the keep folderstructure is enabled?

Kind Regards

Comment 54

[Christophe Humbert]

Attachment: Hello David, This is a first proposal aligning the function of the button archive and the command archive. Kind Regards

Comment 55

[David :Bienvenu]

(In reply to comment #53)

> Why not disabled it generally speaking when a message is already archived and
> not only when the keep folderstructure is enabled?

Because, when people change the granularity from monthly to yearly, for example, and they want to re-archive, they can do that now.

Comment 56

[Peter Lairo]

(In reply to comment #55)
> Because, when people change the granularity from monthly to yearly, for
> example, and they want to re-archive, they can do that now.

Will the button/menu-item say "re-archive" when that scenario/context is the likely/relevant one? Will the tool-tip explain it? (e.g., "re-archive your archive with the new settings")

Comment 57

[Christophe Humbert]

Hello David,

I will Change my patch in your sense... but maybe we should do it for the button archive because for now the possibility to re-archive is limited to people knowing the command. What is your thinking?

Kind Regards

Comment 58

[Christophe Humbert]

Attachment: Disable the A command when keepFolderStructure is on

Hello David,

Here is the patch as requested.

The only thing worrying me is that even before the keepfolderstructure patches the A command and Archive button enabling/disabling were not aligned.

What are you thinking about an alignment?

Kind Regards

Comment 59

[David :Bienvenu]

Comment on attachment 440171 [details] [diff] [review]
Disable the A command when keepFolderStructure is on

you're missing the check that the displayed folder is the archive folder...I'll try to come up with a quick fix.

Comment 60

[Christophe Humbert]

Hello David,

I have tested with a build and it was working...

Nice if you could improve

Comment 61

[David :Bienvenu]

Attachment: fix enabling of archive

cumulative patch - Christophe, does this work for you?

Re inconsistency between button and archive command, it already exists and this doesn't make it any worse.

Comment 62

[David :Bienvenu]

Comment on attachment 440241 [details] [diff] [review]
fix enabling of archive

sr requested for the mailnews/base part of this patch, which I wrote...

Comment 63

[Christophe Humbert]

Ok David,

I will test tomorrow the patch you wrote, sorry not to be as good as you expected.

I understant this does not make it worse but the A command and the button arhive way of being enabled shouldn't they be aligned?

Thanks for supporting me and bearing my poor coding during this patch

Comment 64

[David :Bienvenu]

Christophe, thx very much for all your work on this.  It's much appreciated.

Re aligning the two commands, one reason not to do so is that if we disable the archive button cmd, we remove the button from the message header area, which gives more room for other things in that area. Re-archiving an existing archived message using the archive button in the message header area is pretty unlikely. But we'd like the archive shortcut to still work. But that also disables the archive toolbar button, unfortunately.

Comment 65

[Christophe Humbert]

Ok I understand and thanks for the explanation.

I will provide results of tests tomorrow morning for me i.e Central European Time.

Comment 66

[Mark Banner (:standard8)]

Comment on attachment 440241 [details] [diff] [review]
fix enabling of archive

sr=Standard8 for the mailnews parts.

Comment 67

[Christophe Humbert]

Hello David,

It is working like a charm. Sorry for late testing. I had a hectic day.
For information, here are the tests done
Keepfolderstructure activated: A command disabled
Keepfolderstructure deactivated: A command enabled and when changing archive granularity from 2 to 1 or 1 to 2 the re-archiving process is working well.

Kind Regards

Comment 68

[Mark Banner (:standard8)]

bienvenu said he forgot to land this before the freeze. As this isn't available by default and it should be low risk, a=Standard8 for landing in the tree today.

Comment 69

[David :Bienvenu]

fixed for beta2. Thx, Christophe.

Comment 70

[Christophe Humbert]

Thank to you David

Comment 71

[IagoSRL]

Hi!

Because was not ready before the freeze, there is no strings nor UI for this feature, ok, but my question is: We must open a new bug for UI implementation or reopen this? What is better?

Thanks very much for the work!

Comment 72

[Jim Porter (:squib)]

A new bug (with a comment here pointing to it) is generally the recommended strategy.

Comment 73

[IagoSRL]

I opened the Bug 573336: 'Option "keep folders structure when archive" needs UI' like proposed Jim.

Comment 74

[Jens Hatlak (:InvisibleSmiley)]

Note that this new type of archiving is completely broken with respect to folders with international characters, spaces etc. in the name, because the actual folder name cannot be extracted from folder.URI in those cases. See bug 573392 for how we will fix it for SeaMonkey. You might want to back-port/adapt that.

Comment 75

[Branislav Klocok]

Hello Guys,

The names of the automatically created archive folders are still broken if they contain international characters. (Mozilla/5.0 (X11; U; Linux i686; sk; rv:1.9.2.8) Gecko/20100802 SUSE/3.1.2 Thunderbird/3.1.2). Is there a possibility for fix this soon. It really crates a lot of problems for users that have folders contain such characters.

Comment 76

[Klavs Klavsen]

Has this i18n-bug been fixed? before I turn archiving loose on my 100.000+ emails - I'd like to know Thunderbird won't mess it up - due to this bug with i18n characters in folder names.