Closed Bug 448337 Opened 16 years ago Closed 15 years ago

Move step of "Copy then Move" action in single filter rule is not executed(even after fix of 376235)

Categories

(MailNews Core :: Filters, defect)

x86
Windows XP
defect
Not set
minor

Tracking

(Not tracked)

VERIFIED FIXED
Thunderbird 3.0b4

People

(Reporter: World, Assigned: rkent)

References

Details

Attachments

(1 file, 1 obsolete file)

Move step in "Copy then Move" action in single rule is not executed(even after fix of Bug 376235).
This bug is successor of Bug 376235 and Bug 398498.

Tested with Tb Trunk(Gecko/2008072800 Shredder/3.0a2pre) on MS Win-XP SP3.
Tools/"Run Filters on Folder" on a local mail folder is executed.
F-X means local mail folder named "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)

Phantom mail problem in Case-1-1/Case-2-1(Bug 398498) disappered.
When Case-1, both of COPY & MOVE were executed.
When Case-2, only COPY was executed (No filter log for MOVE).
>(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)"
Summary: Move step in "Copy then Move" action in single rule is not executed(even after fix of 376235) → Move step of "Copy then Move" action in single filter rule is not executed(even after fix of 376235)
Product: Core → MailNews Core
Workaround is available:
  Don't request both of "Copy" & "Move" in single filter.
  Request "Copy" and "Move" in separated filters.
Severity: normal → minor
I don't see this problem most of the time.  I'm using an IMAP inbox, and the Copy goes to a Local Folder while the Move goes to another IMAP folder (this lets me have a temporary cache of recent messages viewable from anywhere, not just from Thunderbird where the real filing goes on).

However, when I first start up Thunderbird, I often get this error:

Alert

The operation failed because an other operation is using the folder.  Please wait for that operation to finish and then try again.

When this happens, one of two things occurs:  Generally either the Copy or Move succeeds, but not both.  So some messages get filed into my local folders but are still in the Inbox instead of being moved to the other IMAP folder, and some messages get filed into the IMAP folder but are not also copied to my local folders.

I'm not sure if Thunderbird is encountering a problem due to trying to compact folders at the same time, or if it's some other kind of problem.  But in any case, it mostly works with IMAP (which is why I upgraded to Thunderbird 3.0b2 and swapped from POP3 to IMAP).

To reproduce, I suggest setting up rules as I have, closing Thunderbird, sending a half-dozen messages to yourself, and then restarting Thunderbird.
(In reply to comment #3)
> I don't see this problem most of the time.
> I'm using an IMAP inbox, and the Copy goes to a Local Folder while the Move goes to another IMAP folder

Does it mean next?
 - Filter action of "copy to local folder" & "move to IMAP folder (to same IMAP
   account's folder as Inbox)" in a sigle message filter rule
   usually works well when automatic mail check, but fails sometimes.
 - "Run filter on folder" successfully copies and moves the mail.
   (Please note that comment #0 with local mail folder occurred always.)
   (And I tested with local mail folder only.                          )

> The operation failed because an other operation is using the folder.  Please
> wait for that operation to finish and then try again.
> 
> When this happens, one of two things occurs:  Generally either the Copy or Move
> succeeds, but not both.  So some messages get filed into my local folders but
> are still in the Inbox instead of being moved to the other IMAP folder, and
> some messages get filed into the IMAP folder but are not also copied to my
> local folders.

> But in any case, it mostly works with IMAP (which is why I upgraded to Thunderbird 3.0b2 and swapped from POP3 to IMAP).

Your case sounds to be interfere by auto-compact.

See following.
a.Bug 274330 Comment #13: Run filter while "compact of filer move source folder"
b.Bug 274330 Comment #14: Run filter while "compact of filer move target folder"
These are test with "local mail folder" + "move by filter" + "compact".
In these cases, "another operation is using folder..." was observed.
I think b. is applicable to your case, because you have "copy to local mail folder" in message filter rule.

If culprit is "silent auto-compact of local mail folder", "upgraded to Thunderbird 3.0b2" can be a solution, because Tb 3 changed behaviour of auto-compact : Don't invoke it while downloading or message filterring.

If "changing to IMAP" resolved problem in your case, phenomenon of Comment #0 may be local mail folder only issue. 
  
> However, when I first start up Thunderbird, I often get this error:

"Internal automatic rebuild index due to outdated .msf" may be a culprit in your case.

To toddgleason@gmail.com:
  
Anyway, check whether "silent auto-compact" is relevant or not first.
Re-enable "Confirmation dialog before auto-compact of local mail folder" (see Bug 482836 Comment #9), and check difference between "OK to confirmation" and "Cancel to confirmation" (see Bug 482836 Comment #16).
I had auto-compact enabled (no prompting).  I have turned off compacting entirely for now and will see if things improve.  I have also commented on https://bugzilla.mozilla.org/show_bug.cgi?id=274330#c19 .
I still saw the problems with auto-compact disabled.  Is it enough to disable that in the UI or do I really need to change anything in prefs.js?  I re-enabled the compacting because it didn't seem to help or hurt.
We need to clarify if this bug applies to AfterTheFact message filters ("Run Filters on Folder" in comment 0) or during normal checking mail, which I assume is the context used in comment 3. The code and solutions is different.

I recommend that this bug be limited to AfterTheFact filters, as then we could attempt a specific solution. In nsMsgFilterAfterTheFact::ApplyFilter, currently both MoveToFolder and CopyToFolder return after application, which means that no further filter actions are applied for the current filter. I think for the copy case, we could try not returning, which would then allow the additional actions for the filter to work. Not sure if that would cause other problems. If the reporter (WADA) can agree to this limitation, then I could try a patch to address that specific case.

There is a larger issue with most filters (including moves associated with junk processing), that there is no reliable method of preventing interference between multiple async requests on the same folder. Then you get some variation of the alert "another operation is in progress" as mentioned in comment 3. We really need to define a bug as the mother-of-all "another operation is in progress" bugs that could be used as a dependency or macro bug for these issues. The solution is likely to be some sore of queue for these requests, rather than the current behaviour which is just to fail or alert.
(In reply to comment #7)
> We need to clarify if this bug applies to AfterTheFact message filters ("Run
> Filters on Folder" in comment 0) or during normal checking mail, which I assume
> is the context used in comment 3.

My comment #0 is for (case-1-B) in below only. I didn't check (case-1-A).
I can say nothing about (case-2-A)/(case-2/B)/(case-3)/(case-4).
I agree with you on limiting to "Local mail folder & AfterTheFact filters", i.e. limit to (case-1-B) only.

(case-1-A) POP3, move/copy to local mail folder, Filter during download
(case-1-B) POP3, move/copy to local mail folder, AfterTheFact filters
(case-2-A) IMAP, move/copy to IMAP folder, Filter during download
           (same or different IMAP account)
(case-2-B) IMAP, move/copy by filter to IMAP folder, AfterTheFact filters
           (same or different IMAP account)
(case-3)   IMAP, move/copy to local mail folder,
           Filter during download and/or AfterTheFact filters
(case-4)   POP3, move/copy to IMAP folder,
           Filter during download and/or AfterTheFact filters
Your case 1-B includes both move and copy. I think there is a possibly easy fix for copy, but not for move (and the bug description only specifies "Copy"). In a move, the message no longer exists in the current folder, so an operation post-move is not meaningful for our folder-based filter definitions. It would be theoretically possible I guess to follow a message to a new folder, then apply the filter there, but that is not implemented currently. Probably what we shoud really be doing is to disable the ability to add an additional filter action following a move to make it clear that a filter action after a move does not work by design.

I'll try to do a patch to allow the Copy case with after-the-fact filters, but there is some risk that there may be some issues with that (for example, what happens if the message or its metadata is modified by a subsequent filter action? Should that change be included in the copy, or not?)
Assignee: nobody → kent
Status: NEW → ASSIGNED
Target Milestone: --- → Thunderbird 3.0rc1
(In reply to comment #9)
> (and the bug description only specifies "Copy").

What do you mean? I wrote 'Move step of "Copy then Move" action in single filter rule' in bug summary...

> In a move, the message no longer exists in the current folder, so an operation
post-move is not meaningful for our folder-based filter definitions.

I wrote '"Copy then Move" action in single filter rule'...
  rule-1: Both action of "copy to a folder" & "move to a folder" is checked.
  In this case, Tb executes "copy" then executes "move", and stops at this rule,
  because of "move".
Next case is already resolved by Bug 376235.
  rule-1: If(some conditions) {copy to a folder;}
  rule-2: If(some conditions) {move to a folder;} (same or different folder)

I never refer to next case.
  rule-1: If(some conditions) {move to a folder;}
          => stops at this step because of "move"
  rule-2: If(some conditions) {copy to a folder;}
Please look "enabled=no" in filter rules in Comment #1.
Only one filter rule is enabled upon test of each case.
From my understanding of the code, the second action is unimportant, it is the first that matters. So I confess I have been paying attention to the first action "Copy" and not your second action "Move" from comment 0. I was also confused by your "POP3, move/copy to local mail folder" to mean that either move or copy is possible as the first action. If you always meant "Copy then Move" then I apologize for the misunderstanding.

What I am saying is that I can attempt a fix if copy is the first action, but not if move is the first action. If you understand my intentions, and that is what you want fixed, fine. If not, then I can either abandon the effort, or spin off a new bug.
Sorry for my confusing "move/copy". It should have been "copy&move".

> What I am saying is that I can attempt a fix if copy is the first action,
> but not if move is the first action.

AFAIK, order of "action="Copy to folder" and "action="Copy to folder" in a filter rule definition(and in filter definition UI) doesn't have meaning. Tb has internal rule for "order of actions".
  Other actions such as "add tag", ..., "Copy"(=last-1), "Move"(=last).
  (I saw this in Message Filter Log.)
  Note-1: Message Filter UI currently permits single "Copy to folder" only
   (and needless to say single "Move to folder" only) in a single filter rule.
  Note-2: Phenomenon of "other action such as 'set status=read' is not reflected
   to copied mail" is different issue. AFAIR, some cases are already reported.

So, I believe your fix will be correct fix of problem of Comment #0.
Depends on: 497622
Whiteboard: [blocked by 497622]
Whiteboard: [blocked by 497622]
Fix plus unit test.
Attachment #391632 - Flags: superreview?(neil)
Attachment #391632 - Flags: review?(neil)
Whiteboard: [needs r/sr Neil]
Target Milestone: Thunderbird 3.0rc1 → Thunderbird 3.0b4
Comment on attachment 391632 [details] [diff] [review]
enable return to action list after copy

>+  PRUint32                    m_nextAction; // next filter action to perform
Is it worth zeroing this out in the constructor?
Attachment #391632 - Flags: superreview?(neil)
Attachment #391632 - Flags: superreview+
Attachment #391632 - Flags: review?(neil)
Attachment #391632 - Flags: review+
(In reply to comment #15)
> (From update of attachment 391632 [details] [diff] [review])
> >+  PRUint32                    m_nextAction; // next filter action to perform
> Is it worth zeroing this out in the constructor?

I did that in an earlier version, then removed it in my cleanup because I did not think it was really needed. I could go either way - do you care?
Neil, perhaps you could respond to my comment 16, but if not I'll just add the check you asked about then ask for checkin.
Carrying over reviews, fixed Neil's request to zero m_nextAction in the constructor, plus moved one m_nextAction call to RunNextFilter for consistency as discussed on IRC. I'll request checkin in about a day if there are no further comments.
Attachment #391632 - Attachment is obsolete: true
Attachment #392430 - Flags: superreview+
Attachment #392430 - Flags: review+
Kent, please don't count this comment as a reason not to checkin.  I just wanted to say thank you very much to the Thunderbird team.  This was my #1 annoyance in Thunderbird 3 Beta 2, and since I upgraded to Beta 3 it has not occurred.  The new beta has in general been running great for me.
(In reply to comment #19)
> This was my #1
> annoyance in Thunderbird 3 Beta 2, and since I upgraded to Beta 3 it has not
> occurred.

Thanks for your kind words. If you saw some improvements in beta 3, it was possibly due to the fixing of bug 497622. I got into that bug when it became clear, in trying to fix this bug, that there were larger issues in copy and move. The particular issue in this bug, as I have tried to narrow it in comment 7, applies to manually applied filters only, and that was not fixed in beta 3.
Keywords: checkin-needed
Whiteboard: [needs r/sr Neil] → [needs checkin]
Checked in: http://hg.mozilla.org/comm-central/rev/6572691c1252
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs checkin]
Checked with Tb 11.0.1 on Win-XP again.
- No qurantine option,
- Filter : If subject contains a string,
           Add tag Importtant($label1), Copy to FolderX, Move to FolderY
- Copy step was executed as expected on multiple mails.
- Move step was executed as expected on multiple mails.
- Tag($label1) was normally written in X-Mozilla-Keys: header of mail data in
  any of Inbox(marked as deleted), FolderX(copy target), FolderY(move target)
VERIFIED.
Status: RESOLVED → VERIFIED
See Also: → 1781792
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: