262184 - Filters should support copy

Status: RESOLVED FIXED

(MailNews Core :: Filters, enhancement)

Description

[Judson Valeski]

One of the actions that should be supported for Mail Filters should be "copy." I
should be able to setup a rule that causes a message to be "copied" to another
folder.

Comment 1

[Scott MacGregor]

*** Bug 264564 has been marked as a duplicate of this bug. ***

Comment 2

[Mike Shaver (:shaver emeritus)]

I'm mortified that I didn't see this on my original search; I think I was only
checking in "mailnews" or something.

Anyway, I'll take this.

Comment 3

[Yuriy Zakalyuzhnyy]

I believe "Copy to Folder" Filter action is very important for TB because it is
standard feature for the majority of available Mac OS X mail clients, e.g.
Entourage, Apple Mail. This is a feature I was looking for all the time since TB
became publicly available. It is very annoying (and not user-friendly :)) to
copy manually tons of messages daily to local folders from IMAP account!

Comment 4

[Mike Shaver (:shaver emeritus)]

Attachment: untested: add copy support (tbird-trunk)

I need to set up some test accounts for this stuff, but I wanted to checkpoint
my work up here, and start to get some review -- I'm not really 100% on how the
folder and listeners and copy service all work together, but this stuff seems
to be mostly right.  I'll try to get a branch patch too, in case I can talk
Scott into it for 1.0.

Comment 5

[Mike Shaver (:shaver emeritus)]

Attachment: now tested, with fixed UI and filter-rule ordering (tbird-trunk)

This works pretty well for me.	I'd like to figure out how to apply certain
changes only to the copy, instead of to the original, but that's definitely a
post-1.0 thing.  (There are also some cleanups in the filter logic that I
resisting in the interest of a 1.0-eligible patch.)

Comment 6

[Mike Shaver (:shaver emeritus)]

Comment on attachment 166236 [details] [diff] [review]
now tested, with fixed UI and filter-rule ordering (tbird-trunk)

Requesting review on this bad boy.

Comment 7

[David :Bienvenu]

Comment on attachment 166236 [details] [diff] [review]
now tested, with fixed UI and filter-rule ordering (tbird-trunk)

Thx for doing this.

I think I'm the de facto module owner of this code at this point.

+      else if (filterAction.type == nsMsgFilterAction.CopyToFolder) {

style nit - prevailing braces style is not K&R...

-      if (loggingEnabled && actionType != nsMsgFilterAction::MoveToFolder &&
actionType != nsMsgFilterAction::Delete)  
+      if (loggingEnabled && actionType != nsMsgFilterAction::MoveToFolder &&
actionType != nsMsgFilterAction::Delete)

is this a whitespace change? do you need to add a check for the copy filter
action here?

sr=bienvenu

Comment 8

[Mike Shaver (:shaver emeritus)]

Comment on attachment 166236 [details] [diff] [review]
now tested, with fixed UI and filter-rule ordering (tbird-trunk)

I thought I caught all those brace-positions.  Sorry, hard reflex to suppress.

Yeah, that was just a whitespace change.  We don't want to follow that path for
the copy case, because we can run an action (move/delete, specifically) after
copy completes.

I'll cook up a branch version of the patch as well (I think just the DTD file
location is materially different).

Comment 9

[Mike Shaver (:shaver emeritus)]

Attachment: tbird-branch: with fixed folder-selection UI

I had to resort to c&p for the actionTargetFolder overlay template, because
it's ID-triggered and duplicate IDs are a no-no.  It's kinda gross, and I'd
like to revisit it on the trunk, but this'll get us through 1.0 and the aviary
landing.

I'll ask David to re-review, because the changes aren't totally trivial (also
fixed some dmose-nits).

Comment 10

[David :Bienvenu]

Comment on attachment 166731 [details] [diff] [review]
tbird-branch: with fixed folder-selection UI

that duplication is really ugly, like you say - can you add a comment about
removing that in the xul?

Comment 11

[Mike Shaver (:shaver emeritus)]

Fixed.  I'll land this on the trunk when it opens, if I don't forget. =)

Comment 12

[David :Bienvenu]

re-opening until it's checked in on the trunk...thx for doing this, Mike.

Comment 13

[Frederic Bezies]

Sorry to spam this bug, but is this bugfix could be related to bug 271696 I
reported a few minutes ago ?

Comment 14

[David :Bienvenu]

*** Bug 61635 has been marked as a duplicate of this bug. ***

Comment 15

[Brian 'netdragon' Bober]

There appears to be core mailnews code in this patch. If so, and it's fixed in
the core code, mark bug 61635 fixed. It's not a dupe of a Thunderbird bug.

Comment 16

[David :Bienvenu]

*** Bug 61635 has been marked as a duplicate of this bug. ***

Comment 17

[Judson Valeski]

I get an unexpected error when I try to run a filter using "copy".

"The current command did not succeed. The mail server responded: Command
'SEARCH' not supported."

I'm using aol's IMAP interface, but what I don't understand is why we'd be
talking to the server at all for this filter operation.

Comment 18

[Mike Shaver (:shaver emeritus)]

What are you copying to/from?  Do you get the same error with a "move" filter?

Comment 19

[David :Bienvenu]

filter after the fact uses online search - it's unfortunate that aol imap
servers don't support search - maybe we could have an rfe to do offline search
for those imap servers that don't support online search.

Comment 20

[Judson Valeski]

copying from: "Inbox" (AOL IMAP), to: local folder.

I didn't realize we were using online search. I thought it was all offline.

Comment 21

[Judson Valeski]

I get the same error w/ Move BTW. This is unfortunate 'cause I can't move or
copy... a big reason to use filters.

Comment 22

[Noel St. John]

I can't get the filter to both move and copy.  It appears the move supercedes
the copy checkmark.  Is this correct?  If so, how does one do a copy and move?

If I just copy (no move) then it works.

Comment 23

[Noel St. John]

Adding self to CC.

Comment 24

[Patrick]

(In reply to comment #11)
> Fixed.  I'll land this on the trunk when it opens, if I don't forget. =)

what's the status of this now? 

Keyword says "fixed-aviary1.0" and this bug was briefly reopened (comment 12) to
indicate it wasn't checked in on the trunk yet before it was marked FIXED again. 

However, as far as i can see in my 1.0+ (20050328) OS X nightly there still is
no COPY option on the trunk?

Comment 25

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 166731 [details] [diff] [review]
tbird-branch: with fixed folder-selection UI

this never landed on trunk.  a=caillon to do so, please.

Comment 26

[Christopher Aillon (sabbatical, not receiving bugmail)]

Reopen until someone lands on trunk.  RESOLVED FIXED is for trunk resolution. 
Use keywords for branches.

Comment 27

[Mike Shaver (:shaver emeritus)]

FIXED on trunk, at long last.

Comment 28

[Patrick]

This fix as checked in doesn't seem to work for me. Adding a new filter, with
one requirement for the incoming message (sender contains) and one action
checked (copy to local folder) i get a warning "you must select at least one
filter action".

Comment 29

[John David Galt]

In TB 1.0.2 this works for mail only; it is grayed out for news.  I would like
this reopened for news.

Comment 30

[Mike Shaver (:shaver emeritus)]

Please file a new bug for news support.  (And for RSS, if we have a filtering
story there and we want to copy, or whatever.)

Comment 31

[John David Galt]

Done, bug 292786.

Comment 32

[Andrew Schultz]

*** Bug 298158 has been marked as a duplicate of this bug. ***