> Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120403 Thunderbird/14.0a1
If both "Copy to folder" and "Move to folder" is requested in single filter rule, folder path of secondly specified action is not updated when folder location is changed by move/rename of mail folder.
[Steps to reproduce]
(0) Folder structure, filter rules
F1, F2, F3
Rule-1 : Copy to ABC/F1, Move to ABC/F2
Rule-2 : Copy to ABC/F2, Move to ABC/F3
Rule-3 : Copy to ABC/F3, Move to ABC/F1
Rule-4 : Move to ABC/F1, Copy to ABC/F3
(1) Rename ABC to XYZ
Rule-1 : Copy to XYZ/F1 (Good), Move to ABC/F2 (BAD!, not updated)
Rule-2 : Copy to XYZ/F2 (Good), Move to ABC/F3 (BAD!, not updated)
Rule-3 : Copy to XYZ/F3 (Good), Move to ABC/F1 (BAD!, not updated)
Rule-4 : Move to XYZ/F1 (Good), Copy to ABC/F3 (BAD!, not updated)
Note: Bug 735940 is already fixed, so first action is corrrectly updated.
Fortunately, only one Copy action is currently permitted in message filter UI, so "loss of folder path setting in a single filter rule of Copy&Move" occurs on one action only. However, if multiple Copy actions will be permitted, user will loose many folder path settings in a filter rule at same time.
Nice flag and dependencies games WADA!:)
Seriously, when you write:
> Rule-1 : Copy to XYZ/F1 (Good), Move to ABC/F2 (BAD!, not updated)
what is the resulting value of "Move to" ?
When I had Copy to ABC/F1 and Move to ABC/F1/F2, then moving ABC to other place caused Move to become empty. What value did you get?
Anyway, confirming there is a problem and taking.
standard8, it is not yet sure if this is in TB12. WADA revoked all the dependencies and flags.
Both of us confirmed it only on TB14 so far.
(In reply to :aceman from comment #1)
> Seriously, when you write:
> > Rule-1 : Copy to XYZ/F1 (Good), Move to ABC/F2 (BAD!, not updated)
> what is the resulting value of "Move to" ?
> When I had Copy to ABC/F1 and Move to ABC/F1/F2, then moving ABC to other
> place caused Move to become empty. What value did you get?
What's wong in following filter rules?
Rule-1 : If From=from1, Copy to ABC/F1, Move to ABC/F2
Rule-2 : If From=from2, Copy to ABC/F2, Move to ABC/F3
Rule-3 : If From=from3, Copy to ABC/F3, Move to ABC/F1
Rule-4 : If From=from4, Move to ABC/F1, Copy to ABC/F3
If mail1, mail2, mail3, mail4 has from1, from2, from3, from4 respectively, mail1 is copied to F1 and moved to F2, mail2 is copied to F2 and moved to F3, mail3 is copied to F3 and moved to F1, mail4 is copied to F3 and moved to F1.
Even when move action appears first and copy action appears second in msgFilterRules.dat(user sets in this order at UI, and Tb shows in this order at UI), Copy is executed first and Move is second.
And, "Move to" implies "Stop Filter Execution" on a mail as "Delete" does, because mail doesn't exist in Inbox any more by "Move" or "Delete".
"Move", "Delete", "Stop Filter Execution" is final action in application of filter rules on a mail, so only one of three actions should be permitted at UI to avoid confusion. And, bug 541416 exists for it.
Please note that who changed
from Rule-1 : If From=from1, Copy to ABC/F1, Move to ABC/F2
to Rule-1 : If From=from1, Copy to XYZ/F1, Move to ABC/F2(BAD!,not updated)
when ABC/F1 is renamed to XYZ/F1 and ABC/F2 is renamed to XYZ/F2 at once,
is Tb, not me.
I merely renamed folder ABC/F1, ABC/F2, ABC/F3 to XYZ/F1, XYZ/F2, XYZ/F3 respectively, at same time, by single "rename ABC XYZ" operation via Tb's UI.
(In reply to :aceman from comment #1)
> Nice flag and dependencies games WADA!:)
(In reply to :aceman from comment #2)
> WADA revoked all the dependencies and flags.
It was never "revoke by me", although "games by me" may be right.
I created this bug by "Clone This Bug" of bug 735940, then Tb automatically copied them, unexpectedly. Because I confirmed this bug in Tb trunk only and I didn't check with Tb 11, 12, 13, I don't know this bug occurs in Tb 11, 12, 13 or not. So I had to remove them.
I should have created this bug as purely new bug.
> to Rule-1 : If From=from1, Copy to XYZ/F1, Move to ABC/F2(BAD!,not updated)
I see what you mean now. "Move to" value of ABC/F2 is only seen in the msgfilterrules.dat file. The UI filter editor shows the target folder as blank. Because ABC/F2 no longer exists, ABC was renamed to XYZ.
Could you try if it appear in TB 11,12,13?
This bug was observed with Tb 11.0.1 on Win-XP too.
Created attachment 613755 [details] [diff] [review]
The code seems to wrongly assume there can only be one of "copy to" or "move to" actions but not both and it shortcuts out of the loop when the first one is found. So it does not update the other action even if it exists.
So just removing the shortcut (break) solves the problem for me.
WADA, are you able to test the patch?
The shortcut code (break) seems to be very old, from before HG repository. So maybe it hasn't worked for ages. Maybe in the past it wasn't possible to use both copy and move in one filter so the assumption was correct.
Dbienvenu, should I just remove the 'break' as in the patch, or is it worth to keep it but count to two (copy+move) before break? But that may essentially amount to no break at all because most filters do not have both so the code will loop through all actions before finding out there is no second copy/move. Is there any other optimization possible (but a safe one)?
standard8, see comment 7. If I understand hg blame correctly (on mailnews/base/search/src/nsMsgFilterList.cpp), this is not a recent regression. It surely would be nice to get into branches due to dataloss, but not because of a recent regression status. (Mentioning this due to your setting of tracking-thunderbird12: ?)
Let's see what David Bienvenu thinks.
I guess you can copy several times, but what would several moves even mean?
Currently the filter editor UI does not allow multiple Copy actions. Only one of each action (with exception of Tag and Forward). I am currently mucking around this in bug 541416, you can comment there and propose allowing multiple Copies. But I am not the one to see if the backend would cope with that.
I meant you can't remove the break at least for moves, since that can't really do anything reasonable - if the message is moved it's not conceptually possible to move it (from the same place) again.
Why? I understand the break is just an optimization, assuming there is only one move/copy (as it treats them the same). And that is not true. You can have both. Now I do not know if it is OK to have both, but the UI allows it so far.
The patch just removes the break so that the loop iterates over all the actions and updates the path in all of them (all copy/move rules) regardless of how many it finds and whether it is allowed to have so many. What is wrong with that?
I'm just saying a message can't be moved twice from the same destination. It's obviously not there for the second pass. (Not a problem for copy.) I don't know much about the actual code there :)
It is true that there can only be one Move. But the code wants to find all Move AND Copy rules and therefore there can be 0, 1 or 2 of them.
Exactly, you want to count copy-rules separately and moves separately, and for moves there can be only one.
It looks like there can also only be one copy too.
But why would I want to count the Moves? I still need to find the Copies, so no break shortcut is possible.
I could only break if I found 1 Copy AND 1 Move, which is exactly what I ask in comment 7. And this break would almost never be hit as most filters do not have both actions. So just looping through all actions without counting anything is almost the same, just easier.
So if the patch in bug 541416 passes and allows unlimited Copy actions then there is nothing we can count here. We must loop through all actions in the filter and fix any Copy/Move found. They can be in any order and mixed with other actions.
(In reply to :aceman from comment #8)
> standard8, see comment 7. If I understand hg blame correctly (on
> mailnews/base/search/src/nsMsgFilterList.cpp), this is not a recent
> regression. It surely would be nice to get into branches due to dataloss,
> but not because of a recent regression status. (Mentioning this due to your
> setting of tracking-thunderbird12: ?)
Thanks for the info, that's useful to know.
Comment on attachment 613755 [details] [diff] [review]
But can we get it at least into TB14? It is not a regression but still corrupts users' filters. And will do more so after bug 541416 has landed.
WADA, the patch was checked in. Can you please verify the fix in tomorrows build of TB14/15? Thanks.