Last Comment Bug 376235 - Filters won't move mail after copying it
: Filters won't move mail after copying it
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Filters (show other bugs)
: Trunk
: All All
: -- normal with 3 votes (vote)
: mozilla1.9.1a2
Assigned To: David :Bienvenu
:
Mentors:
Depends on:
Blocks: 289683 307540 398498
  Show dependency treegraph
 
Reported: 2007-04-02 05:18 PDT by Thomas
Modified: 2011-12-16 00:06 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (771 bytes, patch)
2008-07-17 02:06 PDT, Boying Lu
mozilla: review-
Details | Diff | Review
proposed fix (4.49 KB, patch)
2008-07-18 14:47 PDT, David :Bienvenu
standard8: review+
neil: superreview+
Details | Diff | Review
patch with missing .h file change (5.54 KB, patch)
2008-07-21 06:58 PDT, David :Bienvenu
no flags Details | Diff | Review

Description Thomas 2007-04-02 05:18:49 PDT
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 2007-04-02 10:17:56 PDT
(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 2007-06-18 11:07:19 PDT
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 2007-06-22 16:09:16 PDT
Clearing dependency - this is really a backend bug
Comment 4 David :Bienvenu 2007-06-22 16:10:35 PDT
taking - though fixing this is going to be tricky.
Comment 5 foxian 2007-06-22 19:30:55 PDT
This looks related to Bug 289683.
Comment 6 WADA 2007-10-02 17:21:27 PDT
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 2007-10-02 18:12:07 PDT
Is this all with message quarantining turned on, or does it not matter?
Comment 8 WADA 2007-10-02 19:26:17 PDT
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 2007-10-03 18:17:30 PDT
I've opened Bug 398498 for problem of Comment #6 and Comment #8.
Comment 10 Boying Lu 2008-07-17 02:06:35 PDT
Created attachment 329974 [details] [diff] [review]
patch
Comment 11 David :Bienvenu 2008-07-18 08:45:00 PDT
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 2008-07-18 08:51:22 PDT
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 2008-07-18 08:58:07 PDT
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 2008-07-18 14:47:55 PDT
Created attachment 330304 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2008-07-18 15:59:19 PDT
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 2008-07-18 16:15:54 PDT
Comment on attachment 330304 [details] [diff] [review]
proposed fix 

switching r to standard8
Comment 17 Boying Lu 2008-07-20 19:58:04 PDT
David, do you forget to add .h file in your patch? m_msgCopiedByFilter is not defined in your patch.
Comment 18 Boying Lu 2008-07-20 20:25:54 PDT
I've verified that the David's patch works 
Comment 19 eddie 2008-07-21 06:36:10 PDT
hello. can someone clarify how the patch of David is applied?...
Comment 20 David :Bienvenu 2008-07-21 06:58:07 PDT
Created attachment 330566 [details] [diff] [review]
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 2008-07-21 19:41:53 PDT
(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 2008-07-28 18:19:51 PDT
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 2008-07-29 02:52:11 PDT
(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 2008-07-29 04:21:33 PDT
>(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 2008-07-29 04:27:03 PDT
I've opened Bug 448337 for remaining issue in Case-2-1/2.
Comment 26 WADA 2008-08-30 00:51:37 PDT
Is there any plan of back port to Tb 2.0.0.x?

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