Last Comment Bug 187768 - allow filter of "To or CC" to use "is in Address Book..." and "is not in Address Book..."
: allow filter of "To or CC" to use "is in Address Book..." and "is not in Addr...
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Filters (show other bugs)
: Trunk
: All All
: -- enhancement with 11 votes (vote)
: Thunderbird 3.0a3
Assigned To: Kent James (:rkent)
:
:
Mentors:
: 227365 242527 246084 248983 284345 302503 356666 363869 475071 (view as bug list)
Depends on: 432812
Blocks: 161338
  Show dependency treegraph
 
Reported: 2003-01-05 03:23 PST by xolaware.llc
Modified: 2009-01-23 14:51 PST (History)
21 users (show)
standard8: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Add To, ToOrCC, and CC to Ab filter checks (19.63 KB, patch)
2008-04-02 23:28 PDT, Kent James (:rkent)
standard8: review-
Details | Diff | Splinter Review
Added unit tests, fixed widget issue (55.54 KB, patch)
2008-05-23 22:29 PDT, Kent James (:rkent)
no flags Details | Diff | Splinter Review
Removed some leftovers (54.30 KB, patch)
2008-05-24 00:26 PDT, Kent James (:rkent)
standard8: review+
Details | Diff | Splinter Review
Fixed Standard8's nits (50.34 KB, patch)
2008-06-18 15:12 PDT, Kent James (:rkent)
rkent: review+
mozilla: superreview+
Details | Diff | Splinter Review

Description xolaware.llc 2003-01-05 03:23:56 PST
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.3b) Gecko/20030103
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.3b) Gecko/20030103

when creating a filter, allow a filter rule based on "To or CC" to look in an
address book.  if i get mail that has a To: or CC: to someone i know that is
from someone that i don't keep in my address book, but has been BCC:ed to me,
i'd like to be able to recognize it.  similarly, if i get spam that is addressed
to a bunch of people on the to: or cc: list, i'd like to be able to add those
addresses to a "xref spam addresses" address book, and then have future
filtering check that address book; i find that spam often contains a similar
group of addresses which could help block a good amount of spam i see.


Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Comment 1 Jesse Ruderman 2003-01-05 17:21:12 PST

*** This bug has been marked as a duplicate of 162789 ***
Comment 2 xolaware.llc 2003-01-05 18:41:46 PST
jruderman, are you certain this is a duplicate.  i looked at 162789, and it
appears to be for a bug for looking for the sender in a chosen address book.

that already works for me, though i understand that there are other minor
issues regarding the rules file that have that bug left marked as open.

the issue of this bug about the ability to allow items in the "To or CC" field
to also be searched in the Address book of choice.  i.e. a separate field.

this is not an option when "To or CC" is chosen.

i'm going to re-open this for now, but if you are absolutely certain that the
work being done on 162789 is going to address the ability for having other
items searched in the address books, then re-resolve it.
Comment 3 Jesse Ruderman 2003-01-05 18:51:04 PST
You're right, this isn't a dup.  Sorry.
Comment 4 (not reading, please use seth@sspitzer.org instead) 2003-05-08 11:01:50 PDT
mass re-assign.
Comment 5 David Hagood 2003-07-02 13:34:53 PDT
When this is done, I'd like to ask that "Address book" be generalized to all
using LDAP address books as well as locally stored address books.

In a corporate environment, being able to filter based upon presence in the
corporate LDAP server would be very useful.
Comment 6 Eric Liga 2003-08-28 09:30:39 PDT
I think this would be more useful to users than one might initially think--
here's a common case (an actual one for me, as well) illustrating how useful
this feature would be:

Our business works with several clients.  I've created an address book for each
client listing the e-mail addresses of all of our contacts there.  I've got a
filter that sorts incoming mail into folders for each client if it was sent by
someone in that client's address book.  Fine and dandy-- but it's MUCH harder
for me to filter the outbound e-mails (on which I am CC'd) from co-workers TO
those companies into the company's folder.  Being able to say "filter into this
folder if any of the recipients is in a given address book" would make life
much, much easier.  As it stands, all of our outbound mail ends up in my
"general office" folder and has to be hand sorted into the company folders or
the thread-view in the folder is missing items.  

I also BCC myself on all outbound mails so both back-and-forth will show up in
the threads.  I currently have to hand-move these messages as well.

Given that the filter based on the FROM being in an address book is implemented,
adding the feature for TO/CC shouldn't be too hard, should it?  Is it simple
enough for an outside party like myself to implement and submit as a patch? 
Comment 7 Jo Hermans 2003-12-03 12:17:32 PST
*** Bug 227365 has been marked as a duplicate of this bug. ***
Comment 8 R.K.Aa. 2004-06-28 22:45:01 PDT
*** Bug 248983 has been marked as a duplicate of this bug. ***
Comment 9 Mike Cowperthwaite 2004-08-18 14:16:28 PDT
*** Bug 242527 has been marked as a duplicate of this bug. ***
Comment 10 Matthias Versen [:Matti] 2005-03-01 16:11:03 PST
*** Bug 246084 has been marked as a duplicate of this bug. ***
Comment 11 Matthias Versen [:Matti] 2005-03-01 16:11:41 PST
*** Bug 284345 has been marked as a duplicate of this bug. ***
Comment 12 David Hagood 2005-04-20 07:39:28 PDT
Is this ever going to be fixed, or is this really a WONTFIX (and run Thunderbird)?

It's been over 2 years since this bug was filed, and there seems to be no
progress being made on it.
Comment 13 evildave 2005-04-20 09:53:17 PDT
Well... technically it's not a *bug*, you see.  It's more of a feature request.
 A patently obvious, supremely useful, and probably not very hard to implement
feature (though I would have to delve into the source code to make that call). 
Just allow filters to be placed on the 'Sent' folder as well as the 'Inbox', so
I can filter mail by who it's TO when I send it as well as who it's FROM when I
receive it.

It doesn't work this way in Thunderbird, either.  In Thunderbird, they do have
seperate filter settings for different points in the mail tree hierarchy (i.e.
'Local Folders' can have different settings from 'mymailaccount'), but it
doesn't allow you to assign filters to specific folders, such as 'Sent'.  Only
the inboxes of the respective seperate mail accounts are allowed to be filtered.
 So, I still track down and drag messages from 'Sent' to 'BBE/Sent' manually
about once a day.  Obviously the logic is there to highlight and activate menus,
and behave differently according to what part of the tree is highlighted, but
perhaps the filter stuff is all one big, fat, ugly kludge, and hardwired to 'Sent'.
Comment 14 David Hagood 2005-04-20 10:02:20 PDT
 
> Just allow filters to be placed on the 'Sent' folder as well as the 'Inbox', so
Which has NOTHING to do with this bug whatsoever.

This is about using the TO field to filter INCOMING mail against an address
book, - so that mail sent to a valid address can be received and not treated as
spam (or alternatively, mail to known bad addresses can be marked as spam).
Comment 15 evildave 2005-04-20 11:14:25 PDT
Hmm.  I thought I had mentioned this in 248983, and never wrote up this seperate
suggestion.  So much for relying my memory.  Can't imagine it hasn't been
suggested about a million times by now.

Comment 16 Mike Cowperthwaite 2005-07-28 09:00:21 PDT
*** Bug 302503 has been marked as a duplicate of this bug. ***
Comment 17 Magnus Melin 2006-10-14 09:53:49 PDT
*** Bug 356666 has been marked as a duplicate of this bug. ***
Comment 18 Mike Cowperthwaite 2006-12-14 14:12:14 PST
*** Bug 363869 has been marked as a duplicate of this bug. ***
Comment 19 Abhi 2008-03-10 21:58:37 PDT
Hoorrrraaaayyyyyyyyyy

Completes 5 years. Even President's tennure expires in this time.

I propose to close this bug/issue/enhancement/new feature
Comment 20 xolaware.llc 2008-03-11 11:38:26 PDT
(In reply to comment #19)
> Hoorrrraaaayyyyyyyyyy
> 
> Completes 5 years. Even President's tennure expires in this time.
> 
> I propose to close this bug/issue/enhancement/new feature

please no.  as the original submitter, i would still like to see it "fixed" (i.e. implemnted).  if closing it and forcing me to open a duplicate bug will get it done sooner, then close it, and i'll open a new one.

but it seems popular enough.  there have been 7 bugs marked duplicate; one would think that is a cry for the feature from the masses.
Comment 21 djohnson53 2008-03-11 15:40:34 PDT
I'd love to see this in the product too. It would make my life much simpler. My case is that I have owned a domain name for a long time and have many usernames that have either leaked onto spam lists or that have been created by spammers and have been picked up by other spammers.

Whether its a bug or a feature, 5 years doesn't say much for the engineering process.

Comment 22 Martin Petricek 2008-03-18 12:44:15 PDT
I'd like to have this fixed too. I mass-imported about 14000 mails from my old mail program and now I want to filter them in various folders. Unfortunately, I can't filter according to presence of address from To: field in an addressbook.

I guess all three "To:", "Cc:" and "To or Cc:" should benefit of added email filtering using list from addressbook
Comment 23 Kent James (:rkent) 2008-04-02 23:28:33 PDT
Created attachment 313285 [details] [diff] [review]
Add To, ToOrCC, and CC to Ab filter checks

Since I learned enough about the mail filters to accomplish my goals in bug 414179, I looked around for some other similar bugs I could fix and found this one.

The main issue, beyond simply turning on the feature, was figuring out the purpose of code that only executed if filter was Sender and one of the AB checks. Was that Sender check critical for some reason? I finally convinced myself that the Sender check was simply a literal conversion of the older SenderInAB filter type rather than some critical issue.

This patch works, but I want to test it a little more before asking for a review.
Comment 24 Kent James (:rkent) 2008-04-03 21:31:26 PDT
Comment on attachment 313285 [details] [diff] [review]
Add To, ToOrCC, and CC to Ab filter checks

OK I'm happy with this patch, requesting review.
Comment 25 djohnson53 2008-04-04 20:16:32 PDT
You're a gentleman and a scholar, my friend.
Comment 26 Abhi 2008-04-09 03:39:00 PDT
Bravo Bravo.  You don't know how much of my pain has been reduced.
I never knew my earlier comment was motivating.

But really thanks a ton. Waiting when it gets to production.
Comment 27 djohnson53 2008-05-05 14:16:00 PDT
Well, it missed 2.0.0.14 .....
Comment 28 Kent James (:rkent) 2008-05-05 14:28:22 PDT
(In reply to comment #27)
> Well, it missed 2.0.0.14 .....
> 
Virtually no feature changes are being added to the 2.0.0.x series. The patch if for trunk.
Comment 29 djohnson53 2008-05-07 21:37:09 PDT
After 5 years, you think someone might be embarrassed enough to make an exception - As simple enough a change that this is... What is this Microsoft?

Can someone make this happen?

Is this something that someone who's too old to code can cobble into an existing client?
Comment 30 Mark Banner (:standard8, limited time in Dec) 2008-05-08 00:14:34 PDT
(In reply to comment #29)
> After 5 years, you think someone might be embarrassed enough to make an
> exception - As simple enough a change that this is... What is this Microsoft?

No. The 2.0.0.x series is for security and stability fixes only. We don't allow other changes to ensure compatibility with extensions, so that we don't get regressions and break what users do, and for a variety of other reasons.

Comment 31 Mark Banner (:standard8, limited time in Dec) 2008-05-13 05:45:22 PDT
Comment on attachment 313285 [details] [diff] [review]
Add To, ToOrCC, and CC to Ab filter checks

So in general this looks reasonable. However there's a problem in mailWidgets.xml I think:

1) Open up Filter dialog, select the first column as "To".
2) Then select the second column as either is or isn't in address book.
3) Third column is displayed correctly.
4) Now select CC or "To or CC" in the first column
5) Third column reverts to a text box.
Comment 32 Kent James (:rkent) 2008-05-13 11:02:17 PDT
OK I see that problem, so I'll look into it. Also I want to submit a unit test for at least the backend features in my next patch. I'm also going to block this bug on bug 432812 which we need for unit test.
Comment 33 Kent James (:rkent) 2008-05-23 22:29:16 PDT
Created attachment 322338 [details] [diff] [review]
Added unit tests, fixed widget issue

This patch includes the fix for the issue in bug 434493 comment 6, as well as unit tests. There's also the issue left hanging of whether to implement a set of unit test unload functions as proposed in bug 434810 - though this patch should work regardless. The issue in comment 32 is addressed by forcing the widget to use the default settings when the attribute is changed. This could also affect other uses of the widget - but I don't think anyone really intended to preserve settings when the attribute was changed.
Comment 34 Kent James (:rkent) 2008-05-24 00:26:52 PDT
Created attachment 322344 [details] [diff] [review]
Removed some leftovers

Oops, left in some junk from initial testing and previous bug in the unit tests. Removed the junk.
Comment 35 Mark Banner (:standard8, limited time in Dec) 2008-05-30 06:54:43 PDT
Comment on attachment 322344 [details] [diff] [review]
Removed some leftovers

>Index: mailnews/base/search/src/nsMsgSearchSession.cpp

This bit is no longer needed.

>Index: mailnews/base/search/src/nsMsgSearchTerm.cpp
>-    if (m_attribute == nsMsgSearchAttrib::Sender &&
>-        (m_operator == nsMsgSearchOp::IsInAB ||
>+    if ((m_operator == nsMsgSearchOp::IsInAB ||
>          m_operator == nsMsgSearchOp::IsntInAB))

You don't need the extra pair of ( ) around this.


>-       if (m_attribute == nsMsgSearchAttrib::Sender &&
>-         (m_operator == nsMsgSearchOp::IsInAB ||
>+       if ((m_operator == nsMsgSearchOp::IsInAB ||
>          m_operator == nsMsgSearchOp::IsntInAB))

ditto.

Index: mailnews/base/test/unit/test_bug187768.js

Maybe test_searchToCc.js or something?

>+const Cc = Components.classes;
>+const Ci = Components.interfaces;

These are now global.

>+var have_434810 = false;

Obviously we need to remove these bits now and just get 434810 committed before these.

>+  OnStopCopy: function(aStatus) 
>+  {
>+    var fileName = Files.shift();
>+    if (fileName)
>+    { var file = do_get_file(fileName);
>+      copyService.CopyFileMessage(file, inboxFolder, null, false, 0,
>+                              copyListener, null);

The var needs to be on a new line.

>+// Runs at completion of copy
>+function afterOnStopCopy()
>+{
>+  testAbSearch();
>+  return true;
>+}

Any particular reason not to just call testAbSearch()?

You also don't need the return true here.

>+// Test that validity table for scope aScope has Available and Enabled set 
>+//    to aValue for operator aOp and attribute aAttrib
>+function testValidity(aScope, aOp, aAttrib, aValue)

Didn't I see this in a patch I reviewed the other day? Is it worth sharing some of these functions in a common script?

r=me with the comments fixed.
Comment 36 Kent James (:rkent) 2008-06-18 15:12:49 PDT
Created attachment 325639 [details] [diff] [review]
Fixed Standard8's nits

Fixed Standard8's issues, generally updated unit test to the most recent standards. Carrying over r, asking for sr.
Comment 37 Kent James (:rkent) 2008-06-18 15:16:19 PDT
I forgot to add that attachment 325639 [details] [diff] [review] was tested with attachment 324903 [details] [diff] [review] from bug 436630 backed out, as that patch broke the ability to define message searches.
Comment 38 David :Bienvenu 2008-06-18 15:23:48 PDT
Comment on attachment 325639 [details] [diff] [review]
Fixed Standard8's nits

this looks ok, but I'm curious if the tests work on the mac...
Comment 39 Kent James (:rkent) 2008-06-19 10:10:10 PDT
Standard8 tried this on his Mac, and it passed unit tests. So I'm requesting checkin.
Comment 40 Mark Banner (:standard8, limited time in Dec) 2008-06-19 11:04:19 PDT
(In reply to comment #38)
> (From update of attachment 325639 [details] [diff] [review])
> this looks ok, but I'm curious if the tests work on the mac...
> 
Yep these work fine in both debug and opt builds on mac.
Comment 41 Mark Banner (:standard8, limited time in Dec) 2008-06-19 11:12:09 PDT
Checking in mailnews/base/resources/content/mailWidgets.xml;
/cvsroot/mozilla/mailnews/base/resources/content/mailWidgets.xml,v  <--  mailWidgets.xml
new revision: 1.128; previous revision: 1.127
done
Checking in mailnews/base/search/src/nsMsgImapSearch.cpp;
/cvsroot/mozilla/mailnews/base/search/src/nsMsgImapSearch.cpp,v  <--  nsMsgImapSearch.cpp
new revision: 1.54; previous revision: 1.53
done
Checking in mailnews/base/search/src/nsMsgSearchTerm.cpp;
/cvsroot/mozilla/mailnews/base/search/src/nsMsgSearchTerm.cpp,v  <--  nsMsgSearchTerm.cpp
new revision: 1.161; previous revision: 1.160
done
RCS file: /cvsroot/mozilla/mailnews/base/test/unit/test_searchAddressInAb.js,v
done
Checking in mailnews/base/test/unit/test_searchAddressInAb.js;
/cvsroot/mozilla/mailnews/base/test/unit/test_searchAddressInAb.js,v  <--  test_searchAddressInAb.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/mailnews/test/data/bugmail2,v
done
Checking in mailnews/test/data/bugmail2;
/cvsroot/mozilla/mailnews/test/data/bugmail2,v  <--  bugmail2
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/mailnews/test/data/bugmail3,v
done
Checking in mailnews/test/data/bugmail3;
/cvsroot/mozilla/mailnews/test/data/bugmail3,v  <--  bugmail3
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/mailnews/test/data/bugmail4,v
done
Checking in mailnews/test/data/bugmail4;
/cvsroot/mozilla/mailnews/test/data/bugmail4,v  <--  bugmail4
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/mailnews/test/data/bugmail5,v
done
Checking in mailnews/test/data/bugmail5;
/cvsroot/mozilla/mailnews/test/data/bugmail5,v  <--  bugmail5
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/mailnews/test/data/bugmail6,v
done
Checking in mailnews/test/data/bugmail6;
/cvsroot/mozilla/mailnews/test/data/bugmail6,v  <--  bugmail6
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/mailnews/test/data/bugmail7,v
done
Checking in mailnews/test/data/bugmail7;
/cvsroot/mozilla/mailnews/test/data/bugmail7,v  <--  bugmail7
initial revision: 1.1
done
Comment 42 Daniel B. 2009-01-23 13:01:51 PST
Which version did those changes go into?
Comment 43 Kent James (:rkent) 2009-01-23 13:38:07 PST
The change is in the trunk. So any of the TB3 preliminary releases should have it, including the current beta1.
Comment 44 Bruno 'Aqualon' Escherl 2009-01-23 14:51:39 PST
*** Bug 475071 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.