Closed Bug 16902 Opened 25 years ago Closed 23 years ago

Filter/Search mail based on any header

Categories

(MailNews Core :: Backend, defect, P2)

Product:
MailNews Core
Component:
Backend
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
Future

People

(Reporter: phil, Assigned: naving)

References

(Depends on 1 open bug)

Details

(Keywords: helpwanted)

Attachments

(8 files)

proposed fix for back-end part.
966 bytes, patch
Details | Diff | Splinter Review
proposed fix for backend, v2
1.04 KB, patch
Details | Diff | Splinter Review
proposed fix
62.61 KB, patch
Details | Diff | Splinter Review
screen-shot of ui
57.61 KB, image/jpeg
Details
screen-shot of headers in search-attributes.
61.39 KB, image/jpeg
Details
fix for aol imap servers to download arbitrary headers
677 bytes, patch
Details | Diff | Splinter Review
proposed fix, v2
63.91 KB, patch
Details | Diff | Splinter Review
proposed fix, v3
75.92 KB, patch
Bienvenu
: superreview+
Details | Diff | Splinter Review 
In 4.5 we added the ability to filter mail messages based on "arbitrary
headers". So you could filter on X-Mailer or Resent-From, etc.

We wanted this to work for news too, but I don't think it did. We should fix
that up in mozilla. We can file a separate bug on the news part if that seems
better.

I can't quite find a feature bug on customizable headers (for filtering,
compose, display) but I'll file one and make this bug dependent on it.
Blocks: 10791
OS: Windows NT → All
QA Contact: lchiang → laurel
Hardware: PC → All
Earlier in the year, it was decided not to have arbitrary header filtering or
searching for news in 5.0. Are you changing that decision?
Blocks: 16904
Uh oh. I don't remember that decision. Scott, Sol or David?
I remember arbitrary headers for news being one of the first cut items we had
back in january when we had those meetings =). I'm pretty sure Phil was there
too.
Summary: [FEATURE] Filter mail & news based on any header → [FEATURE] Filter mail based on any header
Ok, I guess I blocked it out. So let's narrow this bug to just mail. I filed
16913 for news.
No longer blocks: 16904
Depends on: 16904
Fix dependencies
I can't tell whether this works in 4.7 or not, because the window for entering
new headers on which to filter (Edit|Message Filters|New|Advanced) is not
explained anywhere including Windows Help.  Please do not mark this "Fixed"
without documenting how to use the feature.
I'm all for improving the documentation (adding headers is mentioned only
briefly), but this bug is for writing the code, not improving the doc. Doc bugs
on this go to simone@netscape.com
Target Milestone: M16
M16
Would not hold beta 2 for this.  Marking M17.
Target Milestone: M16 → M17
[FEATURE] bugs past M16 are OUT for this release.  Marking M20.  If you disagree 
with this action, please help me explain it to the PDT.
Target Milestone: M17 → M20
Adding 4xp keyword since 4.5+ supported this
Keywords: 4xp
Can we at least get the really common extra headers?  I have about 100 filters
and probably half use Resent-From.

If this is not going to happen, what will happen on upgrading?  I assume N6 will
be able to use or upgrade your previous N4 filters, so will the filters be
nuked, stop working, or what?
adding jglick and alecf to cc list.
Latest filter ui spec (and in the april issues meeting) custom headers for mail
filters is listed as a P2 item.

Is M20 your final answer? (what about custom headers for search -- I haven't
looked for a search bug yet...)
I think this belongs to alecf now (at least the UI part)...I'll let him make the
final call.
Assignee: mscott → alecf
yeah, I'm going to make m18 right now, bug hopefully I'll get in for beta2
Priority: P3 → P2
Target Milestone: M20 → M18
I don't think this work is on the exception list that selmer and kmurray gave to
PDT. Is it really a feature?

nope, it's just an extension to some work I've already done.
Status: NEW → ASSIGNED
Summary: [FEATURE] Filter mail based on any header → Filter mail based on any header
Target Milestone: M18 → M17
mass moving to M18 and adding nsbeta3 keyword
Keywords: nsbeta3
Target Milestone: M17 → M18
Are we using this bug for the search messages ui having custom header
capability, too, or do you want search logged separately?
nope, all the code is shared.
Summary: Filter mail based on any header → Filter/Search mail based on any header
Whiteboard: [nsbeta3-]
Target Milestone: M18 → Future
reassigning to myself.
Assignee: alecf → bienvenu
Status: ASSIGNED → NEW
accepting
Status: NEW → ASSIGNED
*** Bug 57935 has been marked as a duplicate of this bug. ***
*** Bug 57936 has been marked as a duplicate of this bug. ***
Added mail3 keyword for 6.5 consideration since quite a few reports/comments
seem to be cropping up about custom headers
Keywords: mail3
adding nsbeta1- keyword.  We want to work on some of the more commonly used
features of search and filters before doing this.
Keywords: nsbeta1-
*** Bug 65338 has been marked as a duplicate of this bug. ***
Keywords: nsbeta3 → mozilla1.0
Whiteboard: [nsbeta3-]
Think bug 16913 may be a dup
Gah, sorry. Disregard my last comment.
Blocks: 66425
*** Bug 90420 has been marked as a duplicate of this bug. ***
*** Bug 87429 has been marked as a duplicate of this bug. ***
*** Bug 66779 has been marked as a duplicate of this bug. ***
*** Bug 94211 has been marked as a duplicate of this bug. ***
*** Bug 94598 has been marked as a duplicate of this bug. ***
bienvenu: what's the status here? Is this on any radars? Should it be?

Gerv
I'm not currently working on this and don't have any near term plans to do so. 
I would like to do this at some point, however.
Adding helpwanted based on bienvenu's comments.
Keywords: helpwanted
What would it take to support the filter part?
So imported filters or hand editing the rules
file works? The UI can wait, as long as there
is a way to support the filters in the file.

Most mailing lists use X-BeenThere or X-Loop,
and many use the List-* headers.

Filtering is almost useless without some way
to filter on any header. Most users will have
the filters from NS4 to migrate, of the skill
to edit the rules file, so only the filter engine
is "required", the UI could wait for a UI coder.


The backend isn't ported from 4.x, so there's both frontend and backend work to
do. The backend work involves both the filters/search code, and the imap/pop3
code. I agree that doing the backend work first makes sense.
*** Bug 100615 has been marked as a duplicate of this bug. ***
I am willing to work on this one if no one else has yet started on it. 
*** Bug 100858 has been marked as a duplicate of this bug. ***
naving: no-one is working on it to my knowledge, and many people would love you
to work on it :-) There are a number of bugs in our filters UI; you may want to
review those before starting work, because it may well be easy to fix a lot of
them on the way through.

Gerv
taking.
Assignee: bienvenu → naving
Status: ASSIGNED → NEW
We already have the back-end code in place. It was not working. I have 
a one liner fix for the problem. I will work on the UI part next.
Status: NEW → ASSIGNED

    
    

    
  
The fix is to compare the correct part of each header line with the arbitrary
header. I have made it caseInsensitive, can change it if someone has a good 
reason not to do so.

 

    
    

    
  
david, please review. 

    
    

    
  
I think offset is going to matter when comparing, will attach a new patch. 

    
    

    
  


  

    
    

    
  
This is how we parse other headers like To, CC etc, should work fine. 

    
    

    
  
I think that part of the issue is that the IMAP envelope doesn't give you
anything beyond the standard header, so you need to specifically request it, no?

    
    

    
  
I did notice that, but didn't know that you will have to specifically
request it for imap. For pop3, it comes by default. 

    
    

    
  
cc jglick, can you point to me the specs for this feature. I have started 
with 4x as the base. 

    
    

    
  
Navin, this what you are looking for?
http://www.mozilla.org/mailnews/specs/filters/#Headers

Would apply to Search as well.

    
    

    
  
4x is little bit different, it has an advanced button that takes you to the 
dialog for adding customized headers. I believe, that is much easier and less 
confusing to do because once you have added those headers, they will appear in
the dropDown. They will appear in the same place where we right now see  
"Customize headers..." (from specs).


    
    

    
  
The new design works similarly, except instead of an "Advanced" button, which 
gives the user no indication it is associated with adding headers, they select 
"Customize" from the Header selection menu. This opens a dialog which allows 
them to create additional Headers. Once created, the new headers will appear in 
the Header list (same as 4.x).

    
    

    
  
The new UI design is excellent, apart from the fact that it seems to omit a
section for saying exactly what should happen to the message if it's caught by
the filter. On this point, I would add a request:

People often complain about 4.x filters that it's not possible to do (A OR B)
AND (C or D) or similar. The current UI permits only ALL OF/ANY OF (A, B, C). I
think the best way of making simple UI for the generic mechanism is merely to
say, when you are deciding what to do with the passed message, have an option to
pass to another filter, i.e.:

File in [Folder from folder list]
Delete
Mark Read
...
Route to [Filter from filter list]

i.e. permit cascading of filters. This would be a minimal UI increase (one
additional option) but a great increase in functionality.

Gerv 

    
    

    
  
gerv: complex boolean logic in filters is a completely seperate feature, and
requires a lot of back end support... theres a bug around somewhere, I suggest
tracking it down and adding yourself to the CC.

    
    

    
  
I admit I'm not familiar with the internals of the code (perhaps because naving
hasn't written it yet) but I don't understand how this can be so hard.

function checkIfCaughtByFilter(filter) {
...
return (boolean) caught;
}

-> 

function checkIfCaughtByFilter(filter) {
...
if caught && (userhaschosentopasstoanotherfilter)
  return checkIfPassedFilter(filter2)
else
  return caught;
}

Gerv

    
    

    
  
Alec is suggesting, I believe, that implementing complex boolean filters is the
right way to go, though hard. He was not saying that your approach would be hard. 

    
    

    
  
Complex boolean filters would be both hard, and hard to do UI for. My idea is
both simple in concept and (probably) execution, and has almost the same level
of power as complex boolean filters. 

If no-one is stepping up to implement complex boolean filters, let's get the 95%
case by asking naving to add this while he's working in the code.

Gerv

    
    

    
  
I don't think Navin should do this - I don't believe we've agreed that this is a
good thing for the UI, among other things. Did I miss something? I personally
think this UI would be confusing and non-intuitive for users. 

    
    

    
  
enough! gerv, take this up in a bug that's actually relevant - this bug is about
supporting any header, not complex boolean filters.

    
    

    
  
ya, this bug is not about complex boolean algebra. Please take it to another 
bug.

    
    

    
  
ok, I have got imap and pop3 working for both search and filters. working 
on news...

    
    

    
  

      
      
      
      

        Attached patch
          
          
        proposed fix
        — 
          — Splinter Review
      

    


  

    
    

    
  
The fix includes fix for both backend/UI and for pop3 imap (online/offline) and
news offline. 

The main thing here was to design the UI and hook-up and make changes to the 
backend. The UI is as described in the specs and I will attach the screenshot.
The main part of the fix is to read the pref customheaders and then initialize
the table that contains all the attributes and their corresponding operators
with the custom headers. The XBL widget uses this table and prefs to build up 
the searchTerms widget. It maintains a one to one relationship between value ids 
and the label. This is needed for loading correct labels when passing 
information either from search widget to back-end or vice-versa. Add a special 
item "Customize.." to the table, when selected launches the custom headers 
add/remove dialog. The "Replace" functionality seems redundant and also does not 
work in 4x so I have left it for now. 

The backend involves downloading headers in case of imap and we need to 
download headers that are used in filters. I have made it so that we can cache
the headers until the user changes the filters and commits the changes. Moved
all the code from imapMiscellaneousSink to imapServerSink. Made changes so 
that it work for news offline. Also removed code no longer used. 

david and seth, please review. 



    
    

    
  

      
      
      
      

        Attached image
          
        screen-shot of ui
        — 
      

    


  

    
    

    
  
These screenshots are of the Search UI - I take it this also applies to Filters,
right?

While you are in the Message Search box, could you take five seconds to clean up
some of its more serious problems? For example, the fact that, by default, the
Results section is only 2 and one half messages high, that it doesn't seem to
remember if you resize the dialog or reposition the splitter, and that the box
with the search criteria in should expand and contract as you click "More" and
"Fewer" rather than starting far too large, and scrolling when you click "More"
enough times? :-)

Gerv

    
    

    
  
+// return -1 if no more local lines, length of next line otherwise.*/

shouldn't put that '*/' at the end of this comment

+    if (m_passedHeaders)
+
		m_numLocalLines--; // the line count is only for body lines

looks like you left a tab in here

This routine nsMsgFilterList::GetArbitraryHeaders assumes that the caller called 
GetShouldDownloadArbitraryHeaders first. That seems like a dangerous assumption
to me. It would be better to combine those routines into one, if you can't
figure out a way of making it so that you have to call GetShould first (or
figure out a way of throwing an assert if the client doesn't do what you've
implicitly decided they will). Or make GetShouldDownloadArbitraryHeaders call
GetArbitraryHeaders itself, so that the result is pre-calculated.

Other than that, the C++ code looks ok. I can't speak for the js.

    
    

    
  
out of morbid curiosity, what do the AOL servers do when you ask for individual
header values?

    
    

    
  
for aol imap servers "search" command is not supported. But it does allow to 
download the custom headers which is what we need for filtering. I have patch to 
make this work. 

    
    

    
  
gerv:

>While you are in the Message Search box, could you take five seconds to cleanup
>some of its more serious problems? For example, the fact that, by default, the
>Results section is only 2 and one half messages high, that it doesn't seem to
>remember if you resize the dialog or reposition the splitter

covered in another bug, and beyond the scope of this bug.

>and that the box
>with the search criteria in should expand and contract as you click "More" and
>"Fewer" rather than starting far too large, and scrolling when you click "More"
>enough times? :-)

gerv, is there a bug for this?  if not, can you log it?

    
    

    
  
> covered in another bug, and beyond the scope of this bug.

I hope he finds five seconds to change the relative flex of the two halves of
the dialog. This alone would be a great improvement, and it's a two character
change.

> gerv, is there a bug for this?  if not, can you log it?

Logged as bug 103742 in my alter ego as nobody@mozilla.org.

Gerv



    
    

    
  

      
      
      
      

        Attached patch
          
          
        proposed fix, v2
        — 
          — Splinter Review
      

    


  

    
    

    
  
the patch may have"no newline at end" in one or more places, i will fix that
before checking in. 

    
    

    
  
r=sspitzer  

naving gave me a demo, and this will be a big improvement.

and, since we used the same pref as 4.x, migration will work.

remember to remove the unneeded "xmlns:nc="http://home.netscape.com/NC-rdf#"

when you land this, make sure to log a bug with all the known UI issues so we
can get to them later, when we have more time.

    
    

    
  
if (headers && m_arbitraryHeaders)

should just be if (headers)

nsMsgSearchAttrib::CustomizeHeaders

should either be
nsMsgSearchAttrib::CustomizedHeaders

or nsMsgSearchAttrib::CustomHeaders


customize is a verb
string getArbitraryHeaders();
boolean getShouldDownloadArbitraryHeaders();

these should be readonly attribute  string arbitraryHeaders
and readonly attribute shouldDownloadArbitraryHeaders

Why do we have nsMsgSearchAttrib::OtherHeader

and nsMsgSearchAttrib::CustomizeHeader? Aren't they the same?

    
    

    
  

      
      
      
      

        Attached patch
          
          
        proposed fix, v3
        — 
          — Splinter Review
      

    


  

    
    

    
  
May have some other unrelated files in there, will not checkin those.


    
    

    
  
Comment on attachment 52852 [details] [diff] [review]
proposed fix, v3

sr=bienvenu (for the changes associated with this bug)

        Attachment #52852 -
        Flags: superreview+

    
    

    
  
navin, can you log a bug with all the known issues?

    
    

    
  
fix checked in, I will log those issues now, before I forget. 
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED

    
  
Blocks: 102231

    
    

    
  
Feature basically in and functional.
Any specific issues with the custom header feature will be logged separately.
OK using oct29 commercial trunk build: win98, linux rh6.2, mac OS X
Status: RESOLVED → VERIFIED
Product: MailNews → Core
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: