Closed Bug 262184 Opened 20 years ago Closed 20 years ago

Filters should support copy

Categories

(MailNews Core :: Filters, enhancement)

Product:
MailNews Core
Component:
Filters
Platform:
PowerPC
macOS
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: jud, Assigned: shaver)

References

Details

(Keywords: fixed-aviary1.0)

Attachments

(1 file, 2 obsolete files)

tbird-branch: with fixed folder-selection UI
26.63 KB, patch
caillon
: approval1.8b2+
Details | Diff | Splinter Review 
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.
*** Bug 264564 has been marked as a duplicate of this bug. ***
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.
Assignee: mscott → shaver
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!
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.
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.)
Attachment #166110 - Attachment is obsolete: true
Comment on attachment 166236 [details] [diff] [review]
now tested, with fixed UI and filter-rule ordering (tbird-trunk)

Requesting review on this bad boy.
Attachment #166236 - Flags: superreview?(mscott)
Attachment #166236 - Flags: review?(dmose)
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
Attachment #166236 - Flags: review?(dmose) → review+
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).
Attachment #166236 - Flags: superreview?(mscott)
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).
Attachment #166236 - Attachment is obsolete: true
Attachment #166731 - Flags: review?(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?
Attachment #166731 - Flags: review?(bienvenu) → review+
Fixed.  I'll land this on the trunk when it opens, if I don't forget. =)
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
re-opening until it's checked in on the trunk...thx for doing this, Mike.
Status: RESOLVED → REOPENED
Keywords: fixed-aviary1.0
Resolution: FIXED → ---
Sorry to spam this bug, but is this bugfix could be related to bug 271696 I
reported a few minutes ago ?
*** Bug 61635 has been marked as a duplicate of this bug. ***
Blocks: 61635
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.
No longer blocks: 61635
*** Bug 61635 has been marked as a duplicate of this bug. ***
Component: General → MailNews: Filters
Product: Thunderbird → Core
Version: unspecified → Trunk
Blocks: 66425
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.
What are you copying to/from?  Do you get the same error with a "move" filter?
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.
Status: REOPENED → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
copying from: "Inbox" (AOL IMAP), to: local folder.

I didn't realize we were using online search. I thought it was all offline.
Flags: review+
I get the same error w/ Move BTW. This is unfortunate 'cause I can't move or
copy... a big reason to use filters.
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.
Adding self to CC.
(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 on attachment 166731 [details] [diff] [review]
tbird-branch: with fixed folder-selection UI

this never landed on trunk.  a=caillon to do so, please.
Attachment #166731 - Flags: approval1.8b2+
Reopen until someone lands on trunk.  RESOLVED FIXED is for trunk resolution. 
Use keywords for branches.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
FIXED on trunk, at long last.
Status: REOPENED → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED
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".


In TB 1.0.2 this works for mail only; it is grayed out for news.  I would like
this reopened for news.
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.)
Done, bug 292786.
*** Bug 298158 has been marked as a duplicate of this bug. ***
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: