Filters won't move mail after copying it

RESOLVED FIXED in mozilla1.9.1a2

Status

MailNews Core
Filters
RESOLVED FIXED
10 years ago
5 years ago

People

(Reporter: Thomas, Assigned: Bienvenu)

Tracking

Trunk
mozilla1.9.1a2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
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

10 years ago
(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? 
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?
Depends on: 384735
(Assignee)

Comment 3

10 years ago
Clearing dependency - this is really a backend bug
Assignee: mscott → nobody
Component: Preferences → MailNews: Filters
No longer depends on: 384735
Product: Thunderbird → Core
QA Contact: preferences → filters
Version: unspecified → Trunk
(Assignee)

Comment 4

10 years ago
taking - though fixing this is going to be tricky.
Assignee: nobody → bienvenu
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 5

10 years ago
This looks related to Bug 289683.
Blocks: 307540
Blocks: 289683
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.
(Assignee)

Comment 7

10 years ago
Is this all with message quarantining turned on, or does it not matter?
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".
I've opened Bug 398498 for problem of Comment #6 and Comment #8.
Depends on: 398498

Comment 10

9 years ago
Created attachment 329974 [details] [diff] [review]
patch
Attachment #329974 - Flags: review?(bienvenu)
(Assignee)

Comment 11

9 years ago
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.
Attachment #329974 - Flags: review?(bienvenu) → review-
(Assignee)

Comment 12

9 years ago
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.
(Assignee)

Comment 13

9 years ago
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.
(Assignee)

Comment 14

9 years ago
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).
Attachment #329974 - Attachment is obsolete: true
Attachment #330304 - Flags: superreview?(neil)
Attachment #330304 - Flags: review?(neil)
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:")))
(Assignee)

Comment 16

9 years ago
Comment on attachment 330304 [details] [diff] [review]
proposed fix 

switching r to standard8
Attachment #330304 - Flags: review?(neil) → review?(bugzilla)

Updated

9 years ago
Attachment #330304 - Flags: superreview?(neil) → superreview+

Comment 17

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

Comment 18

9 years ago
I've verified that the David's patch works 

Comment 19

9 years ago
hello. can someone clarify how the patch of David is applied?...
(Assignee)

Comment 20

9 years ago
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

9 years ago
(In reply to comment #19)
> hello. can someone clarify how the patch of David is applied?...
> 
Yes, the patch fixed this bug.
Attachment #330304 - Flags: review?(bugzilla) → review+
(Assignee)

Updated

9 years ago
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Blocks: 398498
No longer depends on: 398498

Updated

9 years ago
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → mozilla1.9.1a2
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

9 years ago
(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

>(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)"
I've opened Bug 448337 for remaining issue in Case-2-1/2.
Product: Core → MailNews Core
Is there any plan of back port to Tb 2.0.0.x?
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x?
You need to log in before you can comment on or make changes to this bug.