217034 - Restore capability: "Label"/"Tag" as filter criterion

Status: RESOLVED FIXED

(MailNews Core :: Filters, enhancement)

Description

[Jon Roland]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4) Gecko/20030624
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4) Gecko/20030624

Up to v1.3 we could filter on Label = {Important,...,None}. In V1.4 we can't.
Trying to add the function back through Customize doesn't work.

I need this capability to archive less important messages from before a certain
date, the ones I don't flag or label, by sending them to archive folders in
Local Folders, which is on a removable hard drive.

I could filter on Flag = (off?), since I also set the flag of the messages I
don't want to archive right away, and I see that an option for filtering on Flag
has been added, but no values are suggested, and none of the ones I have tried
entering manually work (on/off,set/unset,yes/no,1/0, ...).

I would appreciate restoring the previous ability to filter on the Label, and
complete the adding of the ability to filter on the Flag.

It would also be helpful if the Search options worked the same way the Filter
options do.

Right now, I am stuck for trying to archive my messages, and my hard drive is
nearing overflow. Is there a workaround to enable me to archive the messages
from July?

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Jon Roland]

To add to this, the View field above the Subject header in the message pane
should have the option of selecting Label = None, so I could archive unlabeled
messages by selecting None and those messages remaining in view from before the
cutoff date.

Comment 2

[Mats Palmgren (inactive)]

Duplicate of bug 192582 via bug 208289?

Comment 3

[Jon Roland]

No. Bug 192582 is about IMAP. This is about POP and the email client functions.

Comment 4

[Jon Roland]

Further, Bug 208289 is not a dupe of 192582, but is an imcomplete exposition of
what I am proposing, which suggests it should be considered a dupe of this later
bug, as a lesser-included bug. It was a mistake to remove the ability to filter
on label, because it is still important for those of us who need to archive
manually on multiple field values, including those we set manually as we review
our mail, such as Label and Flag.

Comment 5

[Jon Roland]

I submit that part of the problem, and the reason the Filter on Label function
was removed, is that some have too narrow a concept of what the Filter function
should be able to do, which is to serve as a general-purpose batch processing
utility for messages that enables the user to do anything on batches of messages
that can be defined by the available attributes and changes on them, to do
almost anything to multiple messages that might be done to single messages manually.

Moreover, the same underlying functionality, and similar interfaces, should
apply to View, Sort, Search, Select, Move, Label or any other function that
involves some kind of sorting and selecting.

Moreover, the functionality should be applicable to any folder of any account,
not just to the Inbox.

For those of use who get many hundreds of messages a day, there is simply no way
we can cope with each message singly. Most just have to be set aside for later
attention, and if one can't immediately identify special folders to move them
to, then they stay in the Inbox until, perhap once a month, the messages not
marked in some way for later attention get moved to an archive folder on an
external drive.

To compete in this market, Mozilla mail needs to be able to meet the needs of
users who have to manage thousands of messages a day. Those who only get a
couple of hundred really don't have a sense of the needs of those of us who are
having to cope with a flood.

Comment 6

[Jon Roland]

To avoid any misunderstanding, I am not arguing that a Filter on Label option
need be presented among the initial list of dropdown options, if not many people
are using it. But it should be possible to add it back as an option through
Customize. Thus, one should be able to select Label (without having to type it
in) for any of the various Filter, Sort, View, etc. options, and get the second
setting options of is/is'nt (and perhaps greater than, less than, but not
"contains"), then the third setting options of 0 - None, ..., 5 - Later (or
however the user might redefine the labels).

On that second selection, I would also urge the separation of the relation from
an option to select NOT, so that "isn't" would be NOT is. All the selections
should be negatable.

Similarly for Flag. Flag is [not] [0 - off, 1 - on].

We should also be able to set mixed AND and OR boolean conditions, and set the
order for the move, so the Filter could be set as

Date less than 03/08/01

AND

(Label less than 2

OR

Label greater than 3)



 

Comment 7

[Jon Roland]

Any further action on this. It is still a hassle for me to have to move all
messages received during the previous month to an archive folder for that month,
then extract the labeled ones and move them back, putting them ahead of the new
messages for the current month.

Comment 8

[Mike Cowperthwaite]

Resummarizing to add "criterion" for searching.

(In reply to comment #6)
> it should be possible to add it back as an option through Customize. 

Unlikely -- Customize adds headers, Label is not a header.
  

(In reply to comment #7)
> Any further action on this.

My confirming this bug now is the "further action" to date.  Asking "why isn't 
my bug fixed" is not going to get the bug fixed any sooner, and generates 
unnecessary bugmail to people who get far too much of it already.

See also bug 201433, bug 177032.

Comment 9

[Mike Cowperthwaite]

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

Comment 10

[Jo Hermans]

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

Comment 11

[Jim Meyer]

Hello!

Here's a list of the bugs which lead to this one:

bug 67421, bug 161509, bug 177032, bug 177294, bug 192582, bug 201433, bug
208289, bug 217034, bug 231679, bug 264108, bug 272883, bug 287777.

A fair number of people care about this issue and have for a while.

I think I've found a workaround for most of the pain:

* Make a new search. Don't worry if it's the search you want, just make any
search and hit the "Search" button.
* Click the "Save as Search Folder" button.
* In the Saved Search window you can search by label, age in days, header
begins/ends with, body, etc., regardless of your mail store.

I've tested the Label search and it works; I have no reason to think the others
won't. This means the functionality is there under the hood; it just needs to be
re-exposed at the top level.

Two questions:

1. This bug is marked as "enhancement" ... is it really an enhancement when the
functionality existed previously?

2. Now that we know the underlying code still supports this, could someone raise
the priority of re-exposing this in the top-level interfaces?

Thanks!

--j

Comment 12

[Jim Meyer]

Please see https://bugzilla.mozilla.org/show_bug.cgi?id=217034#c11 for a
possible workaround to this problem.

--j

Comment 13

[Mike Cowperthwaite]

> I've tested the Label search and it works; I have no reason to think the
> others won't.

The Body case (bug 67421) does not work for IMAP in a MailView or Saved Search.

Most of the other bugs you reference in fact are not helped by the workaround 
either, because the bug has been fixed (bug 201433 = bug 272709, bug 177294), or 
the bug is duped to either this one or to bug 192582, or the bug really has 
nothing to do with what you're talking about (bug 161509).  In the future, pls. 
curb your enthusiasm a bit.

Comment 14

[Magnus Melin [:mkmelin]]

Anyone know if this got removed intentionally? And if so, why?

Comment 15

[David :Bienvenu]

I don't know for sure - I suspect it was removed because at the time, messages wouldn't have labels set at the time filters run. Now that filters can be run at any time, it does make sense to tag as a filter criteria.

Comment 16

[Kent James (:rkent)]

I think this is a fairly simple change to make. Does anybody still care about this? Is there any controversy? I would be happy to do the patch if that is all that's needed.

Comment 17

[Karsten Düsterloh]

> Does anybody still care about this?

Of course! Filtering on (maybe even just) set "status" like tags would be a powerful enhancement.

> Is there any controversy?

None I see.

> I would be happy to do the patch if that is all that's needed.

That'd be truly awsome. :)

Comment 18

[Kent James (:rkent)]

I've been testing a patch to this, and I've got a problem. In the IMAP case, the IS function does not work for two different reasons:

1) The keyword stream that is received may include junk fields. So a keyword string of "junk $label1" does not equal "$label1" and the IS test fails. The user has no hint that junk is a keyword, so this is not the expected behavior.

2) IMAP seems to be maintaining in some cases both old-style labels and keywords. So at http://mxr.mozilla.org/seamonkey/source/mailnews/base/search/src/nsMsgLocalSearch.cpp#564 a label "$label1" is appended to the existing keyword, which is already "$label1". So "$label1 $label1" also does not equal "$label1" and the IS fails.

I'm tempted to take the easy way out, that is just enable the CONTAINS function, and file these problems as a separate bug.

Comment 19

[Kent James (:rkent)]

Here's another issue, perhaps alluded to in comment 15. Enabling contains label for a filter works when applied as a manual filter, but does not seem to work when I download an IMAP message header that already has a label applied (by a separate TB running on a different computer). Seems like that should work.

Comment 20

[Kent James (:rkent)]

Here's another update of issues. The deeper I go, the more complexity I find. Maybe that's why this was disabled in the first place.

My comment 19 occurs because filters only work when the message is unread. If I set the unread flag with IMAP, then it works. This is by design.

I've made a first pass at fixing the problems described in comment 18. But there's complexity. Tags are stored as IMAP custom flags with a key $label1 - $label5 for the first five tags, then a fixed key related to the original tag description for higher numbered tags. In the UI, we are careful to only display  as tags IMAP flags that are recognized as part of the current set of tag definitions. There are other sources of IMAP flags - we add "junk" and "nonjunk", also there could be flags for old deleted tags that we added, or another program could have added flags. The issue is with the IsEmpty and Is search criteria. Is a tag empty (or does a tag match exactly) if there are leftover tag keys that we no longer us, or other flags?  Since the UI only displays currently defined tags, I think most people would say those are the only ones that count. So to make this work well, I would need to scan all flags, and compare them to the official list of tags, before I could determine a match for IsEmpty or Is. There's some extra infrastructure needed to do this quickly in a search function (like preloading the list of existing tag keys).

Comment 21

[Kent James (:rkent)]

Attachment: Enable tag filter, add IsEmpty, assure tags are real

This should solve the problems mentioned in my previous comments.

Comment 22

[Mark Banner (:standard8)]

Comment on attachment 317849 [details] [diff] [review]
Enable tag filter, add IsEmpty, assure tags are real

>Index: mailnews/base/public/nsIMsgTagService.idl
>+  // Is key a valid tag?
>+  PRBool IsValidKey(in ACString key);

Please use comment format similar to http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/mailnews/addrbook/public/nsIAbManager.idl&rev=1.52&mark=65-71#65

You should also use boolean as the return value in idl files.

>Index: mailnews/base/search/src/nsMsgImapSearch.cpp
>Index: mailnews/base/search/src/nsMsgSearchTerm.cpp
> nsresult nsMsgSearchTerm::MatchKeyword(const char *keyword, PRBool *pResult)

>   nsresult rv = NS_OK;
>   PRBool matches = PR_FALSE;
>+  
>+  // Can we skip expensive valid keywords check?
>+  if (m_operator == nsMsgSearchOp::DoesntContain ||
>+      m_operator == nsMsgSearchOp::Contains) {
>+      
>+    const char *keywordLoc = PL_strstr(keyword, m_value.string);
>+    const char *startOfKeyword = keyword;
>+    PRUint32 keywordLen = strlen(m_value.string);
>+    while (keywordLoc)
>+    {
>+      // if the keyword is at the beginning of the string, then it's a match if
>+      // it is either the whole string, or is followed by a space, it's a match.
>+      if (keywordLoc == startOfKeyword || (keywordLoc[-1] == ' '))
>+      {
>+        matches = keywordLen == strlen(keywordLoc) || (keywordLoc[keywordLen] == ' ');
>+        if (matches)
>+          break;
>+      }
>+      startOfKeyword = keywordLoc + keywordLen;
>+      keywordLoc = PL_strstr(keyword, keywordLoc + keywordLen + 1);
>+    }
>+    *pResult = (m_operator == nsMsgSearchOp::DoesntContain) ? !matches : matches;
>+    return rv;
>+  }

This seems a little complicated. Are we guaranteed a space at the end of all the tags?

Why can't you just use something along the lines of:

1) find location within string
2) if (location = 0 or location - 1 = ' ') and (endlocation != end or endlocation + 1 = ' ') then matches = true.

Also, you don't seem to use the rv. Could you just return NS_OK here?

>+  // Only accept valid keys in tokens.
>+  nsCStringArray keywordArray;
>+  keywordArray.ParseString(keyword, " ");

How likely is it (on an average user's set up) that for the IsEmpty case, that the keyword will be ""? I'm just wondering if its worth special-casing.

>+  nsCOMPtr<nsIMsgTagService> tagService(do_GetService(NS_MSGTAGSERVICE_CONTRACTID, &rv));
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  // Loop through tokens in keywords
>+  for (PRUint32 i = 0; i < keywordArray.Count(); i++)

keywordArray.Count() should be assigned to a temporary variable to avoid Count() being called multiple times.

>+      // Does this valid tag key match our search term?
>+      matches=keywordArray[i]->Equals(m_value.string);
>+

Please put spaces either side of equals.

>+  if (m_operator == nsMsgSearchOp::Is) {
>+    if (matches)
>+      *pResult = PR_TRUE;
>+    else
>+      *pResult = PR_FALSE;

What's wrong with *pResult = matches ?

>+  if (m_operator == nsMsgSearchOp::Isnt) {
>+    if (matches)
>+      *pResult = PR_FALSE;
>+    else
>+      *pResult = PR_TRUE;
>+    return NS_OK;
>+  }

Ditto but with *pResult = !matches ?


>Index: mailnews/base/src/nsMsgTagService.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/base/src/nsMsgTagService.cpp,v
>retrieving revision 1.18
>diff -u -8 -p -r1.18 nsMsgTagService.cpp
>--- mailnews/base/src/nsMsgTagService.cpp	6 Aug 2007 18:43:56 -0000	1.18
>+++ mailnews/base/src/nsMsgTagService.cpp	26 Apr 2008 06:27:09 -0000
>@@ -154,16 +154,17 @@ NS_IMPL_ISUPPORTS1(nsMsgTagService, nsIM
> nsMsgTagService::nsMsgTagService()
> {
>   m_tagPrefBranch = nsnull;
>   nsCOMPtr<nsIPrefService> prefService(do_GetService(NS_PREFSERVICE_CONTRACTID));
>   if (prefService)
>     prefService->GetBranch("mailnews.tags.", getter_AddRefs(m_tagPrefBranch));
>   // need to figure out how to migrate the tags only once.
>   MigrateLabelsToTags();
>+  GetKeys();
> }
> 
> nsMsgTagService::~nsMsgTagService()
> {
>   /* destructor code */
> }
> 
> /* wstring getTagForKey (in string key); */
>@@ -259,16 +260,18 @@ NS_IMETHODIMP nsMsgTagService::AddTagFor
> {
>   nsCAutoString prefName(key);
>   ToLowerCase(prefName);
>   prefName.AppendLiteral(TAG_PREF_SUFFIX_TAG);
>   nsresult rv = SetUnicharPref(prefName.get(), tag);
>   NS_ENSURE_SUCCESS(rv, rv);
>   rv = SetColorForKey(key, color);
>   NS_ENSURE_SUCCESS(rv, rv);
>+  rv = GetKeys();
>+  NS_ENSURE_SUCCESS(rv, rv);
>   return SetOrdinalForKey(key, ordinal);
> }
> 
> /* void addTag (in wstring tag, in long color); */
> NS_IMETHODIMP nsMsgTagService::AddTag(const nsAString  &tag,
>                                       const nsACString &color,
>                                       const nsACString &ordinal)
> {
>@@ -354,17 +357,20 @@ NS_IMETHODIMP nsMsgTagService::SetOrdina
> 
> /* void deleteTag (in wstring tag); */
> NS_IMETHODIMP nsMsgTagService::DeleteKey(const nsACString &key)
> {
>   // clear the associated prefs
>   nsCAutoString prefName(key);
>   if (!gMigratingKeys)
>     ToLowerCase(prefName);
>-  return m_tagPrefBranch->DeleteBranch(prefName.get());
>+  nsresult rv;
>+  rv = m_tagPrefBranch->DeleteBranch(prefName.get());
>+  NS_ENSURE_SUCCESS(rv, rv);
>+  return GetKeys();
> }
> 
> /* void getAllTags (out unsigned long count, [array, size_is (count), retval] out nsIMsgTag tagArray); */
> NS_IMETHODIMP nsMsgTagService::GetAllTags(PRUint32 *aCount, nsIMsgTag ***aTagArray)
> {
>   // preset harmless default values
>   *aCount = 0;
>   *aTagArray = nsnull;
>@@ -531,8 +537,37 @@ nsresult nsMsgTagService::MigrateLabelsT
>       rv = AddTagForKey(labelKey, ucsval, csval, EmptyCString());
>       NS_ENSURE_SUCCESS(rv, rv);
>       labelKey.SetCharAt(++i + '1', 6);
>     }
>   }
>   m_tagPrefBranch->SetIntPref(TAG_PREF_VERSION, 2);
>   return rv;
> }
>+
>+NS_IMETHODIMP nsMsgTagService::IsValidKey(const nsACString &key, PRBool *result)
>+{
>+  *result = PR_TRUE;
>+  if (m_keys.IndexOf(key) >= 0)
>+    return NS_OK;
>+  *result = PR_FALSE;
>+  return NS_OK;

You need to NS_ENSURE_ARG_POINTER on result.

Please could you use the "a"rgument notation as well, i.e. aKey and aResult.

This function would be much clearer with *result = (m_keys.IndexOf(key) >= 0);

>+// refresh the local tag key array m_keys from preferences
>+nsresult nsMsgTagService::GetKeys()
>+{
>+  nsIMsgTag **tagArray;
>+  PRUint32 numTags;
>+  GetAllTags(&numTags, &tagArray);
>+  m_keys.Clear();
>+
>+  for (PRUint32 tagIndex = 0; tagIndex < numTags; tagIndex++)
>+  {
>+    nsCAutoString key;
>+    nsIMsgTag *tag = tagArray[tagIndex];
>+    tag->GetKey(key);
>+    if (!m_keys.InsertCStringAt(key, tagIndex))
>+      return NS_ERROR_FAILURE;
>+  }
>+  NS_Free(tagArray);
>+  return NS_OK;
>+}

You've used the wrong free, it should be NS_FREE_XPCOM_ISUPPORTS_POINTER_ARRAY. You should also free it if you return NS_ERROR_FAILURE.

You may also want to check that tagArray[tagIndex] isn't null. If not, then you don't need the temporary.

Comment 23

[Kent James (:rkent)]

Thanks for the comments. Just one question:

>Index: mailnews/base/search/src/nsMsgImapSearch.cpp
>Index: mailnews/base/search/src/nsMsgSearchTerm.cpp
> nsresult nsMsgSearchTerm::MatchKeyword(const char *keyword, PRBool *pResult)

>   nsresult rv = NS_OK;
>   PRBool matches = PR_FALSE;
>+  
>+  // Can we skip expensive valid keywords check?
>+  if (m_operator == nsMsgSearchOp::DoesntContain ||
>+      m_operator == nsMsgSearchOp::Contains) {
>+      
>+    const char *keywordLoc = PL_strstr(keyword, m_value.string);
>+    const char *startOfKeyword = keyword;
>+    PRUint32 keywordLen = strlen(m_value.string);
>+    while (keywordLoc)
>+    {
>+      // if the keyword is at the beginning of the string, then it's a match if
>+      // it is either the whole string, or is followed by a space, it's a match.
>+      if (keywordLoc == startOfKeyword || (keywordLoc[-1] == ' '))
>+      {
>+        matches = keywordLen == strlen(keywordLoc) || (keywordLoc[keywordLen] == ' ');
>+        if (matches)
>+          break;
>+      }
>+      startOfKeyword = keywordLoc + keywordLen;
>+      keywordLoc = PL_strstr(keyword, keywordLoc + keywordLen + 1);
>+    }
>+    *pResult = (m_operator == nsMsgSearchOp::DoesntContain) ? !matches : matches;
>+    return rv;
>+  }

This section was copied from existing code. I left it alone as "proven" code. Would you like for me to rewrite it along the lines that you suggested?

Comment 24

[Kent James (:rkent)]

(In reply to comment #22)
> 
> This seems a little complicated. Are we guaranteed a space at the end of all
> the tags?
> 
> Why can't you just use something along the lines of:
> 
> 1) find location within string
> 2) if (location = 0 or location - 1 = ' ') and (endlocation != end or
> endlocation + 1 = ' ') then matches = true.
> 
The issue here is the case where we have two tags: IAmATag and IAmATagToo. Your algorithm would miss IAmATag when keyword = "IAmATagToo IAmATag". You really need to iterate through the string, as the original code does, to protect against this.

I'm going to leave this as is unless there are further objections.

Comment 25

[Mark Banner (:standard8)]

(In reply to comment #24)
> I'm going to leave this as is unless there are further objections.

That's ok, I can't see a better way of doing it at the moment.

Comment 27

[Kent James (:rkent)]

Unit tests will be rewritten to be compatible when bug 432812 is available.

Comment 28

[Kent James (:rkent)]

Attachment: Added unit tests, fixed reviewer issues

Unit tests have been added. Note that I had to reverse an rv check in nsMsgSearchSession.cpp that was added in bug 434493 as it eliminated the possibility to test search without an available window.

Comment 29

[Kent James (:rkent)]

Attachment: Fixed incorrect lines in unit test

Oops, had some incorrect lines in the unit tests. Fixed.

Comment 30

[Mark Banner (:standard8)]

Comment on attachment 322222 [details] [diff] [review]
Fixed incorrect lines in unit test

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

As you're actually testing most of nsIMsgTagService in this file, lets call it test_nsIMsgTagService.js then its easier for folks to know what it is testing. We can still have comments in the file saying that these tests were added via bug 217034.

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

How about test_searchTag.js or test_tagSearch.js ?

As these are testing quite significant areas of functionality I think its better to give them a name rather than bug numbers.

+function notTag(aKey)
+function isTag(aKey)

I'm not sure we really need these helper functions, do_check_false(tagService.isValidKey(key)); seems just as readable.

+  return true;

nit: AFAIK run_test doesn't need to return true.

-  m_msgWindowWeak = do_GetWeakReference(aWindow, &rv);
-  if (NS_FAILED(rv))
-    return rv;
+  m_msgWindowWeak = do_GetWeakReference(aWindow);

This, of course, is no longer needed.

+var have_434810 = false;

I think now we have agreement on the way forward for bug 434810 could you change these to the new way before check in. Hopefully we'll get bug 434810 pretty soon as well.

r=me with the comments addressed.

Comment 31

[Mark Banner (:standard8)]

Comment on attachment 322222 [details] [diff] [review]
Fixed incorrect lines in unit test

Index: mailnews/test/resources/bugmail1

One other thought, would it be worth putting these files in mailnews/test/data and having the resources specifically for helper functions?

Comment 32

[Kent James (:rkent)]

Attachment: Renamed test files, fixed nits

Carrying over Mark's r, ready for sr

Comment 33

[David :Bienvenu]

Comment on attachment 323464 [details] [diff] [review]
Renamed test files, fixed nits

looks good in general, just a few nits:

+  // Can we skip expensive valid keywords check?
+  if (m_operator == nsMsgSearchOp::DoesntContain ||

Can you change this to "Check if we can..." and remove the ? Otherwise, it looks a little bit like a question left in the code :-)

+   * Determines if the token in aKey is a current valid tag

Maybe "corresponds to a current valid tag"? Or just "is a current valid key". Tags are 16 bit; keywords are imap mod utf7 so "is" isn't quite right.

Can you get rid of the K&R braces since I think the prevailing style in nsMsgSearchTerm.cpp is

if (a)
{
...
}

Instead of calling it GetKeys, maybe RefreshKeyCache would be clearer?

Comment 34

[[e-mail]]

Comment on attachment 323464 [details] [diff] [review]
Renamed test files, fixed nits

>-        // if the keyword is at the beginning of the string, then it's a match if
>-        // it is either the whole string, or is followed by a space, it's a match.
>-        if (keywordLoc == startOfKeyword || (keywordLoc[-1] == ' '))
>-        {
>-          matches = keywordLen == strlen(keywordLoc) || (keywordLoc[keywordLen] == ' ');
I would have gone for matches = keywordLoc[keywordLen] == '\0' || keywordLoc[keywordLen] == ' '; as being cheaper than strlen.

Comment 35

[David :Bienvenu]

Comment on attachment 323464 [details] [diff] [review]
Renamed test files, fixed nits

minusing based on nits, but it's almost there.

Comment 36

[Kent James (:rkent)]

Attachment: Fixed David and Neil's nits, disabled some online mail table entries

Fixed nits from David and Neil.

I tested the "Is" "Isnt" and "IsEmpty" terms using online mail search, and found that they do not work. (Actually I could see no difference between the existing "contains" and "is" functions for online, which is not correct). So I disabled everything except "contains" and "doesntcontain" for online mail search of keywords.

The situation with online mail filters is more confusing. As far as I can tell, this is not enabled anyway. Also, all of the other search terms are defined using capabilities that exist offline, but not online. So I left the terms there the same as for offline mail filter.

Note that a regression from bug 436718 is causing saved search for a single folder to quit working on my system. This patch was tested with that bug's patch from 2008-06-10 17:54 backed out.

Comment 37

[David :Bienvenu]

Comment on attachment 324731 [details] [diff] [review]
Fixed David and Neil's nits, disabled some online mail table entries

thx, Kent.

Comment 38

[Mark Banner (:standard8)]

Checking in mailnews/base/public/nsIMsgTagService.idl;
/cvsroot/mozilla/mailnews/base/public/nsIMsgTagService.idl,v  <--  nsIMsgTagService.idl
new revision: 1.4; previous revision: 1.3
done
Checking in mailnews/base/resources/content/mailWidgets.xml;
/cvsroot/mozilla/mailnews/base/resources/content/mailWidgets.xml,v  <--  mailWidgets.xml
new revision: 1.127; previous revision: 1.126
done
Checking in mailnews/base/search/src/nsMsgImapSearch.cpp;
/cvsroot/mozilla/mailnews/base/search/src/nsMsgImapSearch.cpp,v  <--  nsMsgImapSearch.cpp
new revision: 1.53; previous revision: 1.52
done
Checking in mailnews/base/search/src/nsMsgSearchTerm.cpp;
/cvsroot/mozilla/mailnews/base/search/src/nsMsgSearchTerm.cpp,v  <--  nsMsgSearchTerm.cpp
new revision: 1.160; previous revision: 1.159
done
Checking in mailnews/base/src/nsMsgTagService.cpp;
/cvsroot/mozilla/mailnews/base/src/nsMsgTagService.cpp,v  <--  nsMsgTagService.cpp
new revision: 1.21; previous revision: 1.20
done
Checking in mailnews/base/src/nsMsgTagService.h;
/cvsroot/mozilla/mailnews/base/src/nsMsgTagService.h,v  <--  nsMsgTagService.h
new revision: 1.4; previous revision: 1.3
done
RCS file: /cvsroot/mozilla/mailnews/base/test/unit/test_nsIMsgTagService.js,v
done
Checking in mailnews/base/test/unit/test_nsIMsgTagService.js;
/cvsroot/mozilla/mailnews/base/test/unit/test_nsIMsgTagService.js,v  <--  test_nsIMsgTagService.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/mailnews/base/test/unit/test_searchTag.js,v
done
Checking in mailnews/base/test/unit/test_searchTag.js;
/cvsroot/mozilla/mailnews/base/test/unit/test_searchTag.js,v  <--  test_searchTag.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/mailnews/test/resources/searchTestUtils.js,v
done
Checking in mailnews/test/resources/searchTestUtils.js;
/cvsroot/mozilla/mailnews/test/resources/searchTestUtils.js,v  <--  searchTestUtils.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/mailnews/test/data/bugmail1,v
done
Checking in mailnews/test/data/bugmail1;
/cvsroot/mozilla/mailnews/test/data/bugmail1,v  <--  bugmail1
initial revision: 1.1
done