735940 - folder name changed in several filters after ONE target folder of a filter got moved

Status: VERIFIED FIXED

(MailNews Core :: Filters, defect)

Description

[Wayne Mery (:wsmwk)]

I barely know how to describe this - steps seem the only way

1. I have an imap account with roughly 50 filters - coincidentally the account that handles my bugmail
2. i accidentally  moved one folder (drag and drop) tb-enterprise which has ONE filter pointing to it, such that it became a subfolder of another folder, which itself is a target of ONE filter.
3. after some drag mistakes, I got tb-enterprise back to it's original location as a subfolder of X in local folders
4. 10 hours later I find that many (~25) filters are now pointing to tb-enterprise - precisely, every filter BELOW the th-enterprise filter was affected

what the heck happened?

didn't attempt to reproduce

Comment 2

[:aceman]

This seems to be real, already has a dupe, another user on IMAP.

Comment 3

[Wayne Mery (:wsmwk)]

If this really is a regression, it should block something :)

Comment 4

[:aceman]

I can reproduce this too on TB14, Win XP, POP3.

Comment 5

[:aceman]

Can you check this bug really wasn't in 10?

Comment 6

[:aceman]

Well, my tree is currently broken so I can't test it. I just looked through my patches that went into TB11.
I think this folder updating functionality may be in 
mailnews/base/search/src/nsMsgFilterList.cpp: nsMsgFilterList::MatchOrChangeFilterTarget
which I changed a bit in bug 707306. Can anybody try to revert the changes to that function from that patch? I can't do that myself now.

Comment 7

[:aceman]

Attachment: attempt at a fix

Or alternatively try to apply this patch. I think I found out where I made the logical mistake:
I didn't notice there is a loop and *found will stay at true forever if one match is found. So do not use (*found==true) to decide whether to set new target folder in a filter.

Patch untested so far.

Comment 8

[Wayne Mery (:wsmwk)]

i don't currently have build setup to test patch or backout prior patches

Comment 9

[:aceman]

I can try it in 2 days.

Comment 10

[Wayne Mery (:wsmwk)]

assuming, unless we determine otherwise, that bug  707306 is at fault. (which jives with reports starting in version 11)

Comment 11

[:aceman]

Comment on attachment 606334 [details] [diff] [review]
attempt at a fix

This patch fixes it for me.
(No need to revert anything if this is applied.)

Comment 12

[Wayne Mery (:wsmwk)]

does this already have, or need a test?

Comment 13

[WADA:World Anti-bad-Duping Agency]

(In reply to :aceman from comment #11)
> attempt at a fix
> This patch fixes it for me.

Is this bug actually regression by some patches?
Does your patch work for "rename of IMAP folder" case too?

IIRC, bug report for it exists(move/copy target folder of filter is lost/cleared if IMAP folder is renamed), because rename of IMAP folder == unsubscribe of original(==similar to delete for Tb's folder pane UI) + rename + subscribe of  new name). If rename of parent IMAP folder, it's worse.

Comment 14

[:aceman]

I think it is a regression from bug 707306 as noted previously.
The fix works for POP3. I can't test IMAP. Can you?
I am not sure it helps with the problem you describe. It may be unrelated, i.e. your problem existed even before this regression was introduced. But I don't know.

Comment 15

[WADA:World Anti-bad-Duping Agency]

"Problem with IMAP only" I reffered was not message filter issue.
It was problem on search target folder of Virtual Folder(Search Folder) upon rename/move of IMAP folder.
Sorry for my confusion.

(In reply to :aceman from comment #14)
> I can't test IMAP. Can you?

Yes. You also can test IMAP with free Gmail, Yahoo!, vmail.me account.
(vmail.me : https://www.vmail.me/en/ IMAP of vmail.me uses namespace="INBOX.", so convenient for test of namespace related issues)

For message filter and IMAP folder move, I could see this bug by next test with Tb 10.0.2 and Tb 14.0a1(trunk 2012/03/18 build) on MS Win-XP.
(1) Filter's copy/move target folder = A1/B/C
(2) Move parent IMAP folder of B under A1 to A2 : A1/B/C => A2/B/C, 
    => msgFilterRules.dat is not updated
(3) Edit message filter
    => copy/move target folder of filter is changed to blank

Comment 16

[:aceman]

I mean if you can apply the patch and test it on an IMAP server.

But the problem you describe in comment 15 is something else.
Can you reproduce the problem from the original description and test with the patch?

Comment 17

[WADA:World Anti-bad-Duping Agency]

(In reply to :aceman from comment #16)
> I mean if you can apply the patch and test it on an IMAP server.

I can't build. If you provide tryserver build for Win, I can download it and test with IMAP server. 

> But the problem you describe in comment 15 is something else.

If phenomenon of "IMAP folder move/message filter" in comment #15 is something else even though very simple "IMAP folder move/message filter" case, should I open separate bug for it?
Please note that "simple move of IMAP folder(Move B under A1 to under A2)" is correctly tracked when message filter is views after the IMAP folder move.

What is exact "steps to reproduce" of problem of comment #0? What happens on what by steps to reproduce? (on msgFilterRules.dat, on message filter rule panel, ...)

Comment 18

[:aceman]

Your description:
(1) Filter's copy/move target folder = A1/B/C
(2) Move parent IMAP folder of B under A1 to A2 : A1/B/C => A2/B/C, 
    => msgFilterRules.dat is not updated
(3) Edit message filter
    => copy/move target folder of filter is changed to blank (BAD!)

This bug:
(0) There are filters X, Y, Z in the filter list dialog.
(1) Filter Y's copy/move target folder = A1/B/C
(2) Move parent IMAP folder of B under A1 to A2 : A1/B/C => A2/B/C, 
(3) Edit message filter Y
    => copy/move target folder of filter Y is changed to A2/B/C (CORRECT!)
(4) If any filter BELOW this filter (i.e. Z) in the filter list also has a move/copy action then its target is also changed to A2/B/C, regardless of what it was before. Completely unrelated target folders are changed to A2/B/C. (BAD!)

Comment 19

[Mitra Ardron]

I can confirm this one as well, I don't remember moving a folder, but I had a whole batch of my filters switch to new addresses (including the one that filters bugzilla mail - so I've been missing a lot of requests for attention).

Comment 20

[:aceman]

Ok, I have tested the patch on IMAP and it seems to work there too. It should actually be protocol neutral as the logic just decides whether to change a target or not, just then calls the protocol specific functions.
But just to be sure.

The problem happens also when folders are just renamed. I assume any time the string 'user@account/folder' (used as target folder in filter) changes.

Standard8 can you look at this soon?

Comment 21

[Mitra Ardron]

Just note that as Wada says, it happens even when the target has NOT changed, e.g. when you move a folder earlier in the hierarchy.   Definately repeatable, and really painful to undo if you have a lot of filters !

Comment 22

[Mitra Ardron]

Thinking ..... I think the problem could be (without looking at the code).

Folder A is changed to B 

A test is made on each filter to see if it needs changing, filter points to "C" but test says its moved, so changes it to point to B, i.e. its worth looking to see if its the test thats broken.

Comment 23

[:aceman]

So again (folders A, B, C have nothing in common, are in independent branches of the folder tree):
Filter X: move to A
Filter Y: move to B
Filter Z: move to C

Now B changes to D:
Filter X: move to A
Filter Y: move to D (GOOD!)
Filter Z: move to D (BAD!)
... all filters beneath change to D.

B is a path specifier in the format "user@host/folder1/folder2"
If any folderX in the path to the target changes then it is a change in B.
folderX can change via rename or via move in the hierarchy.

So what do you mean "the problem happens even when target has NOT changed"? So what is the even when this problem kicks in?

Comment 24

[:aceman]

... what is the event ...

Comment 25

[:aceman]

Comment on attachment 606334 [details] [diff] [review]
attempt at a fix

As this is in Mailnews core, let's make also Seamonkey guys aware of the regression. It may affect a released stable version (as it does for TB11).

Comment 27

[Ian Neal]

Comment on attachment 606334 [details] [diff] [review]
attempt at a fix

Sorry, I don't really know enough about this code to review.

Comment 28

[WADA:World Anti-bad-Duping Agency]

(In reply to :aceman from comment #18)
> Your description:
> (1) Filter's copy/move target folder = A1/B/C
> (2) Move parent IMAP folder of B under A1 to A2 : A1/B/C => A2/B/C, 
>     => msgFilterRules.dat is not updated
> (3) Edit message filter
>     => copy/move target folder of filter is changed to blank (BAD!)
> 
> This bug:
> (0) There are filters X, Y, Z in the filter list dialog.
> (1) Filter Y's copy/move target folder = A1/B/C
> (2) Move parent IMAP folder of B under A1 to A2 : A1/B/C => A2/B/C, 
> (3) Edit message filter Y
>     => copy/move target folder of filter Y is changed to A2/B/C (CORRECT!)
> (4) If any filter BELOW this filter (i.e. Z) in the filter list also has a
> move/copy action then its target is also changed to A2/B/C, regardless of
> what it was before. Completely unrelated target folders are changed to
> A2/B/C. (BAD!)

:aceman, thanks for detailed STR and phenomenon of this bug.

I could observe this bug in Tb 11.0 with my description, with clean IMAP folder(no suffix in .msf/.sbd name, no special character in Mbox name).
(1) Filter x, y, z : move to A1/B/Cx, A1/B/Cy, A1/B/Cz, respectively
(2) Move B under A1 to B ander A2
    A1/B/Cx => A2/B/Cx , A1/B/Cy => A2/B/Cy , A1/B/Cz => A2/B/Cz
(3) msgFilterRules.dat content
    Filter x : move to A2/B/Cx (Good)
    Filter y : move to A2/B/Cx (BAD! this bug)
    Filter z : move to A2/B/Cx (BAD! this bug)
(4) Move B under A2 back to B ander A1
    A2/B/Cx => A1/B/Cx , A2/B/Cy => A1/B/Cy , A2/B/Cz => A1/B/Cz
(5) msgFilterRules.dat content
    Filter x : move to A1/B/Cx (Good)
    Filter y : move to A1/B/Cx (normal, but BAD! due to bug at step 2/3)
    Filter z : move to A1/B/Cx (normal, but BAD! due to bug at step 2/3)

Actual folder name I previously tested was;
  Move folder rename from INBOX/[A]/B/Cn to INBOX/[A/B/Cn where n is x,y,z
    with msf file for Inbox == Inbox-23.msf, sbd for Inbox == Inbox-23.sbd
  (suffix is phenomenon of bug 520437 due to internal unsubscribe/re-subscribe,)
  (or due to case sensitive Mbox name at IMAP server in bug 520437 comment #7)
In this case, folder of INBOX/[A]/B/Cn remained at folder pane even after folder move is successuful and new INBOX/[A/B/Cn is listed in subscription list and old INBOX/[A]/B/Cn is removed from subscriptin list. And, if still shown INBOX/[A]/B/Cn is clicked, SELECT command fails due to "nonexistent Mbox".
Because old INBOX/[A]/B/Cn is still existent for Tb, move to new location can't be tracked by message filter code.

This is problem when suffixed .msf/.sbd is used and special char such as "[", "]" is used in folder name(bug 727324 is an example), and is absolutely different problem from this bug although it produces filter rule corrupion.
I'll open separate bug for it after additional testing.

Comment 29

[Mitra Ardron]

To aceman's question in Comment #23 abouy my statement in Comment #21

Set filter's X,Y,Z to A/Bx A/By A/Bz

Move By to A/Bq/By  
X -> A/Bx GOOD
Y -> A/Bq/By GOOD
Z -> A/Bq/By BAD

In this example Z should not have been touched at all. 

In Wada's example in Comment #23, step 2, the filter for Z *SHOULD* change, but is changed to the wrong place,

Comment 30

[WADA:World Anti-bad-Duping Agency]

(In reply to Mitra Ardron from comment #29)
> In this example Z should not have been touched at all. 

By simple folder rename in Tb 11, phenomenon of "target folder of all subsequent filter rule is changed" was observed, regardless of "sould be touched" or "shouldn't be touched", regardless of same parent folder or different parent folder, regardless of folder level.

           Initial            After rename N yo NNN
  Rule-A : Move to A          not changed
       |           |           |
  Rule-M : Move to M          not changed
  Rule-N : Move to N          NNN
  Rule-O : Move to O          NNN
       |           |           |
  Rule-Z : Move to Z          NNN

Comment 31

[Mark Banner (:standard8)]

In future, when flagging significant regressions such as this, please use the tracking-thunderbird flags (set to ?) and include a statement of which release it regressed in and the issue.

Using the tracking flag rather than status will mean I'll pay more attention to it.

Comment 32

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/99a185a0fd6e

Comment 33

[Mark Banner (:standard8)]

Comment on attachment 606334 [details] [diff] [review]
attempt at a fix

[Triage Comment]
Taking onto the branches to fix this issue there.

Comment 34

[:aceman]

Has anybody confirmed the fix is working (except me)?

Comment 35

[Mark Banner (:standard8)]

Checked in:

http://hg.mozilla.org/releases/comm-aurora/rev/8fe6f678459e
http://hg.mozilla.org/releases/comm-beta/rev/816333b3a460

Comment 36

[Mark Banner (:standard8)]

(In reply to :aceman from comment #34)
> Has anybody confirmed the fix is working (except me)?

I did a small test when I reviewed it.

Comment 37

[:aceman]

Ok, thanks.
What about TB11?

Comment 38

[WADA:World Anti-bad-Duping Agency]

Verified with Tb trunk 2012-03-26 build on Win-XP.
> Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120326 Thunderbird/14.0a1
move target folder path in msgfilterRules.dat is correctly updated accordin to;
  move/rename IMAP folder, move/rename parent IMAP folder,
without changing irrelevant filter rules.

Comment 39

[Mark Banner (:standard8)]

Comment on attachment 606334 [details] [diff] [review]
attempt at a fix

[Triage Comment]
a=Standard8, as per drivers meeting we've decided to spin a 11.0.1 for this and another issue.

I've landed it on comm-release already:

http://hg.mozilla.org/releases/comm-release/rev/fed9432136dc

Comment 43

[Carol]

Updated to 11.01 and still experiencing bug.

Comment 44

[:aceman]

Can you describe how you tested it?

Comment 45

[Carol]

I asked my husband to send me an email and it still went to the wrong folder.

Comment 46

[:aceman]

The fix by itself does not fix your target folders once they are incorrect. TB doesn't know what they were previously so it can't restore them.
It just prevents them changing in the future when you move any unrelated folder.
Unfortunately you must go into the filter editor and fix the target in each filter manually.

Comment 47

[Carol]

thank you.

Comment 48

[WADA:World Anti-bad-Duping Agency]

(In reply to WADA from comment #13)
> IIRC, bug report for it exists(move/copy target folder of filter is
> lost/cleared if IMAP folder is renamed), (snip)

It was bug 454589 comment #4 for Tb 7.0.1, and was posted by me :-)

Comment 50

[WADA:World Anti-bad-Duping Agency]

This bug occurs in Sm 2.8 release build.
Is there any plan of Sm 2.8.1? Or Sm 2.9(2.9b3-candidates build2) will be released soon?

Comment 51

[Serge Gautherie (:sgautherie)]

(In reply to WADA from comment #50)

> Is there any plan of Sm 2.8.1?

There was no SM 2.8.x.

> Or Sm 2.9(2.9b3-candidates build2) will be released soon?

2.9 is being released "Today"!?

Comment 52

[:aceman]

But there were plans for 2.8.1 to merge this bug, as Thunderbird did.

Comment 53

[Rod Carty]

I just upgraded to TB 12.0.1 because it listed this version as fixing this bug. Perhaps it makes it so this no longer occurs if one is moving folders around, but it doesn't fix filters that were messed up previously. They still point to one folder for me. Do I have to manually fix them? I have a lot more than 50 filter rules.

Comment 54

[WADA:World Anti-bad-Duping Agency]

(In reply to Rod Carty from comment #53)
> Do I have to manually fix them? I have a lot more than 50 filter rules.

As already written in Comment #46 by :aceman, answer is YES.
If you can know correct internal folder path by unaffected other filter rule's definition, you can change back "broken move/copy target folder path" to required one by text editor.
(1) Terminate Tb, Keep backup of current msgFilterRules.dat,
    Edit msgFilterRules.dat by text editor(capable to edit utf-8 character).
(2) Change copy/move target folder path in rules appropriately, and Save.
(3) Restart Tb, confirm that any message filter rule is expected one.

Comment 55

[Rod Carty]

Thanks for the quick reply. Somehow I missed that post.

Comment 56

[:aceman]

Of course, you can do it in the filter editor inside Thunderbird normally.

Comment 57

[Rod Carty]

I will have to, as different filters send messages to different folders. (I have a lot of folders too.)