376235 - Filters won't move mail after copying it

Status: RESOLVED FIXED

(MailNews Core :: Filters, defect)

Description

[Thomas]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
Build Identifier: Version 1.5.0.10 (20070221)

(This might also relate to, or even have been fixed with, bug # 366508 ?)

When a filter is set up to do the two following actions:
1. copy the message to a folder
2. then move this message to another folder,

the move will not happen at all. If there are other actions included, like tagging:
1. copy
2. tag
3. move,

or 1. copy, 2. move, 3. tag,

then the copy and the tag action will succeed, but it's always the move which is left out.
It also doesn't matter if the two destinations folders are local or remote (IMAP). The source folder in the scenario is a local folder (INBOX filled via POP).



 


Reproducible: Always

Steps to Reproduce:
1. Set up filter for a POP INBOX which matches arbitrarily some messages.
2. The action takens should be like mentioned: 1) copy msg to some other folder, 2) move msg to some third folder.
3. Test this filter with the "Test Filter" button on the INBOX.

Actual Results:  
As described above: The copy is performed, but the move isn't. The message remains in the INBOX.


Expected Results:  
The message should have been copied to the one folder, and then be moved to the other folder. The message shouldn't be in the INBOX any more.

Comment 1

[Magnus Melin [:mkmelin]]

(In reply to comment #0)
> (This might also relate to, or even have been fixed with, bug # 366508 ?)

Trying a brach build will tell... Still a problem with this? 
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-mozilla1.8/

xref bug 307540

If you see the problem with a recent build, does the move show in the filter log? 

Comment 2

[WADA:World Anti-bad-Duping Agency]

Root problem seems to be found in Bug 384735(problem while first copy by filter
or just after first copy by filter). Putting Bug 384735 in "Depends on:".
Is there JS Error of Bug 384735 Comment #4 in Error Console?

Comment 3

[David :Bienvenu]

Clearing dependency - this is really a backend bug

Comment 4

[David :Bienvenu]

taking - though fixing this is going to be tricky.

Comment 5

[foxian]

This looks related to Bug 289683.

Comment 6

[WADA:World Anti-bad-Duping Agency]

To David David Bienvenu:
After fix of Bug 384735(problem of hang after copy), phenomenon when "copy&move in a rule"(this bug) became same phenomenon as one when "copy by a rule and move by other later rule"(bug 289683). See Bug 289683 Comment #5, please.

Comment 7

[David :Bienvenu]

Is this all with message quarantining turned on, or does it not matter?

Comment 8

[WADA:World Anti-bad-Duping Agency]

I tested with quarantining=OFF. Sorry but I didn't test with quarantining=on.

Another funny phenomenon occurred during test of copy&move to same mail folder case. (POP3, quarantining=off, Thunderbird trunk 2007/9/30 build)
> Filter Rules
>  Rule-1: If subject contains XXX, Then copy to folder-2
>  Rule-2: If subject contains XXX, Then copy to folder-1 (I forgot to disable) 
>  Rule-3: If subject contains XXX, Then move to folder-2 (same one as Rule-1)
> I sent 3 mails.
Results:
1) folder-1 : 3 mails(normal mail), 3 mail data of X-Mozilla-Status: 0000
   folder-2 : 3 mails(normal mail), 3 mail data of X-Mozilla-Status: 0000
2) Three error dialog of "other process is in progress"
   3 mails remained in Inbox.
   Filter log says 3 hits of Rule-1 and 3 hits of Rule-2. No log for Rule-3

Difference from previous test(Bug 289683 Comment #5) is added Rule-1 only.
It interfered "move by Rule-3", and phantom mail was not produced by "copy mail by Rule-1" nor "copy by Rule-2".

Comment 9

[WADA:World Anti-bad-Duping Agency]

I've opened Bug 398498 for problem of Comment #6 and Comment #8.

Comment 10

[Boying Lu]

Attachment: patch

Comment 11

[David :Bienvenu]

Comment on attachment 329974 [details] [diff] [review]
patch

if you do this, we'll never remove the original msg hdr row when not downloading to a temp file. So it may fix this problem, but will introduce others...the problem is that the move is synchronous, but the copy is async, so even if we do the copy first, the move actually fires first, I believe.

Comment 12

[David :Bienvenu]

One way to fix this would be to use the copy service for the move, in the case where there was also a copy action. But this might break the setting of has new messages, and make biff fire incorrectly, and might also cause folder lock issues if we're download a message into a folder at the same time as trying to delete an other message (which is what move does). Like I said, this is tricky to fix.

Comment 13

[David :Bienvenu]

Or we could use the imap move coalescer in this case, which runs after the whole download process is done (and despite the name, works on non imap folders as well, I believe). I think I'll try that approach out.

Comment 14

[David :Bienvenu]

Attachment: proposed fix

the imap move coalescer change is just cleanup - we're not using the imap service anymore, so don't get it.

The fix in nsParseMailbox.cpp is to use the move coalescer for a move if a previous action was a copy. I had to make m_msgCopiedByFilter a member variable in case some previous filter on the same message had done a move.

This fixes the bug as long as quarantining is turned off (that's a separate issue).

Comment 15

[[e-mail]]

Comment on attachment 330304 [details] [diff] [review]
proposed fix 

>+          if (StringBeginsWith(actionTargetFolderUri, NS_LITERAL_CSTRING("imap:"))
>+            || m_msgCopiedByFilter)
I don't know what mailnews style is but I prefer
if (m_msgCopiedByFilter ||
    StringBeginsWith(actionTargetFolderUri, NS_LITERAL_CSTRING("imap:")))

Comment 16

[David :Bienvenu]

Comment on attachment 330304 [details] [diff] [review]
proposed fix 

switching r to standard8

Comment 17

[Boying Lu]

David, do you forget to add .h file in your patch? m_msgCopiedByFilter is not defined in your patch.

Comment 18

[Boying Lu]

I've verified that the David's patch works 

Comment 19

[eddie]

hello. can someone clarify how the patch of David is applied?...

Comment 20

[David :Bienvenu]

Attachment: patch with missing .h file change

oops, Boying is right - I forgot the .h file change - I've included it in this patch. Thx for catching that.

Comment 21

[Boying Lu]

(In reply to comment #19)
> hello. can someone clarify how the patch of David is applied?...
> 
Yes, the patch fixed this bug.

Comment 22

[WADA:World Anti-bad-Duping Agency]

Tested with Tb Trunk(Gecko/2008072800 Shredder/3.0a2pre) on MS Win-XP SP3.
(F-X means "Mail Folder X" in case description)
Case-1 : Copy and Move in different rule
 OK   Case-1-1 : Rule-1=Copy to F-A, Rule-2=Move to F-A (same folder)
 OK   Case-1-2 : Rule-1=Copy to F-A, Rule-2=Move to F-B (different folder)
Case-2 : Copy then Move in single rule
 BAD  Case-2-1 : Rule-1=Copy to F-A, then Move to F-A (same folder)
 BAD  Case-2-2 : Rule-1=Copy to F-A, then Move to F-B (different folder)

When Case-1, problem was resolved.
When Case-2, only COPY was executed (No filter log for MOVE).

Phantom mail problem in Case-1-1/Case-2-1(Bug 398498) was also resolved.
I close Bug 398498 as FIXED, and I'll open separate bug for remaining Case-2-1/Case-2-2 (single rule case).

Comment 23

[Boying Lu]

(In reply to comment #22)
> Tested with Tb Trunk(Gecko/2008072800 Shredder/3.0a2pre) on MS Win-XP SP3.
> (F-X means "Mail Folder X" in case description)
> Case-1 : Copy and Move in different rule
>  OK   Case-1-1 : Rule-1=Copy to F-A, Rule-2=Move to F-A (same folder)
>  OK   Case-1-2 : Rule-1=Copy to F-A, Rule-2=Move to F-B (different folder)
> Case-2 : Copy then Move in single rule
>  BAD  Case-2-1 : Rule-1=Copy to F-A, then Move to F-A (same folder)
>  BAD  Case-2-2 : Rule-1=Copy to F-A, then Move to F-B (different folder)
> 
What do "Rule-1" and "Rule-2" mean? different filters? and what does "then" mean?
I want to re-produce the "BAD" cases. Can you tell me how?

> When Case-1, problem was resolved.
> When Case-2, only COPY was executed (No filter log for MOVE).
> 
> Phantom mail problem in Case-1-1/Case-2-1(Bug 398498) was also resolved.
> I close Bug 398498 as FIXED, and I'll open separate bug for remaining
> Case-2-1/Case-2-2 (single rule case).
> 
Can you add my e-mail address in the CC list when you file the bug?
I want to work on this issue.  Thanks

Comment 24

[WADA:World Anti-bad-Duping Agency]

>(Case-1-1)
> name="Copy_to_X1"
> enabled="yes"
> type="1"
> action="Copy to folder"
> actionValue="mailbox://ccc@c.c.c/Test-01/X1"
> condition="AND (subject,contains,test)"
> name="Move_to_X1"
> enabled="no"
> type="1"
> action="Move to folder"
> actionValue="mailbox://ccc@c.c.c/Test-01/X1"
> condition="AND (subject,contains,test)"

>(Case-1-2)
> name="Copy_to_X1"
> enabled="yes"
> type="1"
> action="Copy to folder"
> actionValue="mailbox://ccc@c.c.c/Test-01/X1"
> condition="AND (subject,contains,test)"
> name="Move_to_X2"
> enabled="no"
> type="1"
> action="Move to folder"
> actionValue="mailbox://ccc@c.c.c/Test-01/X2"
> condition="AND (subject,contains,test)"

>(Case-2-1)
> name="Copy_to_X1_then_Move_to_X1"
> enabled="no"
> type="1"
> action="Copy to folder"
> actionValue="mailbox://ccc@c.c.c/Test-01/X1"
> action="Move to folder"
> actionValue="mailbox://ccc@c.c.c/Test-01/X1"
> condition="AND (subject,contains,test)"

>(Case-2-2)
> name="Copy_to_X1_then_Move_to_X2"
> enabled="no"
> type="1"
> action="Copy to folder"
> actionValue="mailbox://ccc@c.c.c/Test-01/X1"
> action="Move to folder"
> actionValue="mailbox://ccc@c.c.c/Test-01/X2"
> condition="AND (subject,contains,test)"

Comment 25

[WADA:World Anti-bad-Duping Agency]

I've opened Bug 448337 for remaining issue in Case-2-1/2.

Comment 26

[WADA:World Anti-bad-Duping Agency]

Is there any plan of back port to Tb 2.0.0.x?