57034 - Search News msgs works only with "contains" operator.

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P3)

Description

[laurel]

Using oct16 mn6 branch commercial build

Side note:The criteria selection is broken (see bug 52299), in that mail and
news criteria are swapped.


Search News messages only works with "contains" as the operator.

1.  Open a newsgroup, download headers.
2.  Search|Search mail/news messages and select the newsgroup in the search
scope dropdown.
3.  Enter a simple search using any operator but contains, i.e. subject doesn't
contain x.  Click search button.

Result:  nothing happens, even after waiting awhile.

Note:  doesn't contain DOES work in mail searches.

Not sure when this regressed.

Comment 1

[Alec Flett]

reassign all search/filter UI bugs to gayatrib, part 2

Comment 2

[laurel]

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

Comment 3

[laurel]

nominating for 6.5

Comment 4

[scottputterman]

marking nsbeta1+ and moving to mozilla0.8

Comment 5

[scottputterman]

moving to mozilla0.9

Comment 6

[scottputterman]

marking nsbeta1-

Comment 7

[scottputterman]

reassigning to naving

Comment 8

[Navin Gupta]

nominating for 0.9.2

Comment 9

[(not reading, please use [e-mail] instead)]

I've got a two line fix for this (and probably several other problems)

basically, it boils down to this:

the back end search code is using nsIMsgSearchValidityManager.news and
nsIMsgSearchValidityManager.onlineMail

and the front end is using:

nsMsgSearchScope.MailFolder and nsMsgSearchScope.Newsgroup

that's bad since the consts don't match up.

my simple (hack) fix, is to make it so the two interfaces match up.

I'm not sure what the ideal fix is yet, since I don't know if we should
combine, or remove, or what.

I can also fix a hack in the back end where we check the ioService
to see if we are offline, and then turn news searching into offline news 
searching.  (we can do the IOService check in the javascript and pass back the 
right thing.)

Comment 10

[(not reading, please use [e-mail] instead)]

notes on the problem:

 30 interface nsIMsgSearchValidityManager : nsISupports {
 31 
 32     // eventually it would be nice to use atoms or something
 33     const long onlineMail = 0;
 34     const long onlineMailFilter = 1;
 35     const long offlineMail = 2;
 36     const long localNews = 3;
 37     const long news = 4;
 38     const long newsEx = 5;
 

384 function GetScopeForFolder(folder) {
385     if (folder.server.type == "nntp")
386         return nsMsgSearchScope.Newsgroup;
387     else
388         return nsMsgSearchScope.MailFolder;
389 }


    if (type == "nntp") return nsIMsgSearchValidityManager.news;
    if (type == "pop3") return nsIMsgSearchValidityManager.offlineMail;
    return nsIMsgSearchValidityManager.onlineMailFilter;


 36 [scriptable, uuid(5fe70a74-304e-11d3-9be1-00a0c900d445)]
 37 interface nsMsgSearchScope {
 38     const long MailFolder = 0;
 39     const long Newsgroup = 1;
 40     const long LdapDirectory = 2;
 41     const long OfflineNewsgroup = 3;
 42     const long AllSearchableGroups = 4;
 43 };
 44 


    // hack alert...currently FEs are setting scope to News even if it should 
be set to OfflineNewsgroups...
    // i'm fixing this by checking if we are in offline mode...
    if (whichTable == news || whichTable == newsEx || whichTable == onlineMail)
    {
      nsresult rv;
      nsCOMPtr <nsIIOService> ioService = do_GetService(kIOServiceCID, &rv);
      if (NS_SUCCEEDED(rv) && ioService)
      {
        PRBool offline;
        ioService->GetOffline(&offline);
        if (offline)
        {
          if (whichTable == news || whichTable == newsEx)
            whichTable = localNews;
          else
            whichTable = offlineMail;
        }
      }
    }
 

Comment 11

[(not reading, please use [e-mail] instead)]

Attachment: a hack (for nsBranch)

Comment 12

[(not reading, please use [e-mail] instead)]

dirty deeds, done dirt cheap.

It looks like this hack fixes #60514 and #77181

I'm eager to pound on search and filters and make sure everything is looking 
good.

this would be good (and low risk) for nsBranch to fix a lot of search problems.

I'll need to look at the code more and the interfaces to see what the proper 
fix is.

Comment 13

[(not reading, please use [e-mail] instead)]

ah, a little more to do here.  

when searching pop, we don't see the proper criteria.

(priority is only an offline mail search).

working on a fix that addresses that...

Comment 14

[(not reading, please use [e-mail] instead)]

Attachment: untested draft of better fix.

Comment 15

[(not reading, please use [e-mail] instead)]

more coming on that fix...

Comment 16

[(not reading, please use [e-mail] instead)]

Attachment: draft fix for the trunk.

Comment 17

[(not reading, please use [e-mail] instead)]

the first patch (http://bugzilla.mozilla.org/showattachment.cgi?attach_id=40592)
is suitable for the nsBranch.  the only down side to it (that I've seen so far)
is that when online, you can't do offline criteria (like priority) on pop mail 
and local folders.  I didn't see a simple, low risk fix for that.  but that 
proble already exists in the product, and that little patch fixes seom bad 
problems.  I think it should be safe for the nsBranch, but I still need to test 
more.

the last patch (http://bugzilla.mozilla.org/showattachment.cgi?attach_id=40601) 
is for the trunk.  it does address the "when online, can't do offline criteria 
searches on pop and local folders" and more.  I need to discuss it with 
bienvenu and naving.

Comment 18

[(not reading, please use [e-mail] instead)]

I talked with bienvenu.

I'm going to clean up the trunk patch. I'll going to rename everything to scope
and do something like GetValidityMgrForScope(), and then use the enum in
nsMsgSearchCore and then get rid of the enum in the validity mgr.

also, I've got:

nsMsgSearchScopeTerm* scope = new nsMsgSearchScopeTerm(nsnull,
nsIMsgSearchValidityManager::onlineMail, folder);

I was thinking I'd need to get the proper scope from the folder (and use the
offline state) but I better debug and see what MatchHdr() uses the search term for.

Comment 19

[David :Bienvenu]

sr=bienvenu for the branch patch.

Comment 20

[(not reading, please use [e-mail] instead)]

Attachment: patch for the trunk

Comment 21

[(not reading, please use [e-mail] instead)]

I've removed the hard coded check of server type ("nntp", "pop3") from 
FilterEditor.js and SearchDialog.js

now, nsIMsgIncomingServer has attributes (searchScope, filterScope) that 
default to nsMsgSearchScope::offlineMail and are override in 
nsImapIncomingServer and nsNntpIncomingServer.

this is good clean up, and is a step closer to get filters to work for movemail 
(and other future potential server types)

this required me to move nsMsgSearchScopeValue into MailNewsTypes2.idl

I moved WeAreOffline() into nsMsgUtils.cpp (from nsImapUtils.cpp) so that both 
imap and news could use it.  instead of having the FE determine if we are on or 
offline, the back end determines it when the FE calls GetSearchScope().

this patch fixes this problem, and several other search / filter ui problems 
with improper search criteria and search terms.

naving / bienvenu, please review.

Comment 22

[David :Bienvenu]

sr=bienvenu

Comment 23

[(not reading, please use [e-mail] instead)]

this is going to be trunk only.

Comment 24

[Navin Gupta]

r=naving

Comment 25

[(not reading, please use [e-mail] instead)]

fix landed on trunk.

Comment 26

[laurel]

News searches now work with other than contains operator. Any specific problems
with certain criteria will be reported separately.
OK using aug09 commercial trunk builds: win98, linuxrh6.2 and mac OS 9.0