Closed Bug 735940 Opened 8 years ago Closed 8 years ago

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

Categories

(MailNews Core :: Filters, defect)

defect
Not set
critical

Tracking

(thunderbird11 fixed, thunderbird12+ fixed, thunderbird13+ fixed, thunderbird-esr10 unaffected, seamonkey2.8 wontfix, seamonkey2.9 fixed)

VERIFIED FIXED
Thunderbird 14.0
Tracking Status
thunderbird11 --- fixed
thunderbird12 + fixed
thunderbird13 + fixed
thunderbird-esr10 --- unaffected
seamonkey2.8 --- wontfix
seamonkey2.9 --- fixed

People

(Reporter: wsmwk, Assigned: aceman)

References

(Blocks 1 open bug)

Details

(Keywords: dataloss, regression)

Attachments

(1 file)

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
Duplicate of this bug: 736034
This seems to be real, already has a dupe, another user on IMAP.
If this really is a regression, it should block something :)
I can reproduce this too on TB14, Win XP, POP3.
Can you check this bug really wasn't in 10?
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.
Attached patch attempt at a fixSplinter Review
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.
Assignee: nobody → acelists
Status: NEW → ASSIGNED
i don't currently have build setup to test patch or backout prior patches
I can try it in 2 days.
assuming, unless we determine otherwise, that bug  707306 is at fault. (which jives with reports starting in version 11)
Blocks: 707306
Severity: major → critical
Flags: in-testsuite?
Whiteboard: [datalossy]
OS: Windows Vista → All
Hardware: x86 → All
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.)
Attachment #606334 - Flags: review?(mbanner)
does this already have, or need a test?
(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.
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.
"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
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?
(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, ...)
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!)
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).
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?
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 !
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.
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?
... what is the event ...
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).
Attachment #606334 - Flags: review?(iann_bugzilla)
Duplicate of this bug: 737824
Comment on attachment 606334 [details] [diff] [review]
attempt at a fix

Sorry, I don't really know enough about this code to review.
Attachment #606334 - Flags: review?(iann_bugzilla)
(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.
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,
Attachment #606334 - Flags: review?(dbienvenu)
(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
Blocks: 738096
Attachment #606334 - Flags: review?(mbanner)
Attachment #606334 - Flags: review?(dbienvenu)
Attachment #606334 - Flags: review+
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.
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/99a185a0fd6e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0
Comment on attachment 606334 [details] [diff] [review]
attempt at a fix

[Triage Comment]
Taking onto the branches to fix this issue there.
Attachment #606334 - Flags: approval-comm-beta+
Attachment #606334 - Flags: approval-comm-aurora+
Has anybody confirmed the fix is working (except me)?
(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.
Ok, thanks.
What about TB11?
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.
Status: RESOLVED → VERIFIED
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
Attachment #606334 - Flags: approval-comm-release+
No longer blocks: 738096
Duplicate of this bug: 738096
Duplicate of this bug: 740786
Duplicate of this bug: 737946
Updated to 11.01 and still experiencing bug.
Can you describe how you tested it?
I asked my husband to send me an email and it still went to the wrong folder.
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.
thank you.
No longer blocks: 742155
(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 :-)
Duplicate of this bug: 745092
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?
(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"!?
But there were plans for 2.8.1 to merge this bug, as Thunderbird did.
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.
(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.
Thanks for the quick reply. Somehow I missed that post.
Of course, you can do it in the filter editor inside Thunderbird normally.
I will have to, as different filters send messages to different folders. (I have a lot of folders too.)
Blocks: 755658
Duplicate of this bug: 770549
Duplicate of this bug: 739075
You need to log in before you can comment on or make changes to this bug.