Last Comment Bug 735940 - folder name changed in several filters after ONE target folder of a filter got moved
: folder name changed in several filters after ONE target folder of a filter go...
Status: VERIFIED FIXED
: dataloss, regression
Product: MailNews Core
Classification: Components
Component: Filters (show other bugs)
: Trunk
: All All
: -- critical with 1 vote (vote)
: Thunderbird 14.0
Assigned To: :aceman
:
Mentors:
: 736034 737824 738096 739075 740786 745092 770549 (view as bug list)
Depends on:
Blocks: 755658 707306
  Show dependency treegraph
 
Reported: 2012-03-14 17:20 PDT by Wayne Mery (:wsmwk, NI for questions)
Modified: 2014-12-12 03:50 PST (History)
20 users (show)
vseerror: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
+
fixed
+
fixed
unaffected
wontfix
fixed


Attachments
attempt at a fix (2.94 KB, patch)
2012-03-15 13:05 PDT, :aceman
standard8: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
standard8: approval‑comm‑release+
Details | Diff | Review

Description Wayne Mery (:wsmwk, NI for questions) 2012-03-14 17:20:04 PDT
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 1 :aceman 2012-03-15 08:52:39 PDT
*** Bug 736034 has been marked as a duplicate of this bug. ***
Comment 2 :aceman 2012-03-15 08:53:18 PDT
This seems to be real, already has a dupe, another user on IMAP.
Comment 3 Wayne Mery (:wsmwk, NI for questions) 2012-03-15 09:06:08 PDT
If this really is a regression, it should block something :)
Comment 4 :aceman 2012-03-15 09:19:25 PDT
I can reproduce this too on TB14, Win XP, POP3.
Comment 5 :aceman 2012-03-15 12:17:16 PDT
Can you check this bug really wasn't in 10?
Comment 6 :aceman 2012-03-15 12:34:17 PDT
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 2012-03-15 13:05:35 PDT
Created attachment 606334 [details] [diff] [review]
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, NI for questions) 2012-03-15 13:10:28 PDT
i don't currently have build setup to test patch or backout prior patches
Comment 9 :aceman 2012-03-15 13:28:37 PDT
I can try it in 2 days.
Comment 10 Wayne Mery (:wsmwk, NI for questions) 2012-03-16 02:37:10 PDT
assuming, unless we determine otherwise, that bug  707306 is at fault. (which jives with reports starting in version 11)
Comment 11 :aceman 2012-03-17 04:50:59 PDT
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, NI for questions) 2012-03-17 04:55:49 PDT
does this already have, or need a test?
Comment 13 WADA 2012-03-18 18:23:05 PDT
(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 2012-03-19 01:06:21 PDT
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 2012-03-19 16:33:04 PDT
"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 2012-03-20 00:56:22 PDT
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 2012-03-20 02:16:26 PDT
(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 2012-03-20 03:00:56 PDT
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 2012-03-20 12:12:02 PDT
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 2012-03-20 12:25:52 PDT
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 2012-03-20 22:08:30 PDT
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 2012-03-20 22:10:14 PDT
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 2012-03-21 01:29:03 PDT
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 2012-03-21 01:29:26 PDT
... what is the event ...
Comment 25 :aceman 2012-03-21 01:34:51 PDT
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 26 torsten.zachert 2012-03-21 09:02:10 PDT
*** Bug 737824 has been marked as a duplicate of this bug. ***
Comment 27 Ian Neal 2012-03-21 12:54:38 PDT
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 2012-03-22 01:27:36 PDT
(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 2012-03-22 09:11:48 PDT
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 2012-03-25 03:06:53 PDT
(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) 2012-03-26 02:22:50 PDT
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) 2012-03-26 02:48:59 PDT
Checked in: http://hg.mozilla.org/comm-central/rev/99a185a0fd6e
Comment 33 Mark Banner (:standard8) 2012-03-27 00:53:27 PDT
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 2012-03-27 01:11:42 PDT
Has anybody confirmed the fix is working (except me)?
Comment 36 Mark Banner (:standard8) 2012-03-27 01:14:39 PDT
(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 2012-03-27 01:19:28 PDT
Ok, thanks.
What about TB11?
Comment 38 WADA 2012-03-27 02:24:39 PDT
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) 2012-03-27 10:37:16 PDT
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 40 :aceman 2012-03-29 11:52:23 PDT
*** Bug 738096 has been marked as a duplicate of this bug. ***
Comment 41 Onno Ekker [:nONoNonO UTC+1] 2012-03-30 06:34:24 PDT
*** Bug 740786 has been marked as a duplicate of this bug. ***
Comment 42 Onno Ekker [:nONoNonO UTC+1] 2012-03-30 06:34:51 PDT
*** Bug 737946 has been marked as a duplicate of this bug. ***
Comment 43 Carol 2012-03-31 13:43:18 PDT
Updated to 11.01 and still experiencing bug.
Comment 44 :aceman 2012-03-31 13:54:28 PDT
Can you describe how you tested it?
Comment 45 Carol 2012-03-31 15:14:07 PDT
I asked my husband to send me an email and it still went to the wrong folder.
Comment 46 :aceman 2012-03-31 15:27:13 PDT
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 2012-03-31 15:42:59 PDT
thank you.
Comment 48 WADA 2012-04-04 03:22:01 PDT
(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 49 WADA 2012-04-17 18:47:55 PDT
*** Bug 745092 has been marked as a duplicate of this bug. ***
Comment 50 WADA 2012-04-17 18:53:53 PDT
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) 2012-04-24 13:22:18 PDT
(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 2012-04-24 13:35:13 PDT
But there were plans for 2.8.1 to merge this bug, as Thunderbird did.
Comment 53 Rod Carty 2012-05-15 19:39:19 PDT
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 2012-05-15 20:01:04 PDT
(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 2012-05-15 20:33:52 PDT
Thanks for the quick reply. Somehow I missed that post.
Comment 56 :aceman 2012-05-15 23:54:03 PDT
Of course, you can do it in the filter editor inside Thunderbird normally.
Comment 57 Rod Carty 2012-05-16 01:40:07 PDT
I will have to, as different filters send messages to different folders. (I have a lot of folders too.)
Comment 58 :aceman 2012-07-03 23:11:10 PDT
*** Bug 770549 has been marked as a duplicate of this bug. ***
Comment 59 Wayne Mery (:wsmwk, NI for questions) 2014-12-12 03:50:41 PST
*** Bug 739075 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.