428427 - Unread count in virtual folders not updated for changed headers

Status: RESOLVED FIXED

(MailNews Core :: Search, defect)

Description

[Kent James (:rkent)]

I can see there are lots of existing bugs that deal with issues of unread counts and saved searches, but I'm approaching this from the code perspective, and none seem to indicate exactly what I want to fix.

The symptom: When I add filters associated with junk processing, particularly the filters in bug 414179, saved searches using those filters do not maintain the correct unread count.

Steps to reproduce:

1) Apply patch for bug 414179
2) Add a filter called "uncertain" with a junkpercent between 50 and 90
3) Receive an email with a junkpercent in that range.

Expected behavior: the unread count for the virtual folder is correct.
Actual behavior: the unread count for the virtual folder is not updated until you click on the virtual folder.

The problem occurs in VirtualFolderChangeListener::OnHdrChange  The changed value of junkpercent is properly reported, but that routine is only capable of processing changes that occur in message status or flags. Other changes are ignored.

OnHdrChange needs to know both the old header and the new header to correctly calculate the count changes for the folder. Unfortunately that information is not currently available. What I propose to do is to modify the upstream functions, up to NotifyHdrChangeAll, so that both the old header and the new header are passed down the chain.

Comment 1

[Kent James (:rkent)]

Here's another variation of this same problem, that can be seen on TB 2.0.0.12

Setup: Select Inbox, Search. Add a search filter for contains a tag, say "Work", and save it as a search folder "Work". Take a message and tag it as Work and Unread. Hopefully, in the new Work folder you will see 1 unread message indicated.

Go to the Inbox. Toggle the Work tag on and off while leaving the message unread.

Expected result: Unread field for the Work folder changes
Actual result: Unread field for the Work folder does not change (until you select the folder, then it becomes correct).

Comment 2

[Kent James (:rkent)]

Attachment: WIP, works but I want to review impact some more

This solves the underlying problem, namely that virtual folders must have complete access to the database before and after changes in order to properly maintain counts. I've only tested this so far using tags (my second STR). I need to review its impact, see if there are some related bugs that perhaps I could fix at the same time, and see if I can completely replace the OnFlagsChange stuff with my new OnHdrsChange approach. I also want to compile it into my current working copy, to make sure it solves my use case, the "uncertain" folder can introduce after bug 414179.

Comment 3

[Kent James (:rkent)]

Attachment: View management corrected

I generated changes in a lot of modules by renaming OnHdrChanged to OnFlagsChanged. The root of this bug is that the old OnHdrChanged really only worked for *flag* changes. The new version is the generic, so I took the generic name.

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 318314 [details] [diff] [review]
View management corrected

The OnHdrChange process seems quite expensive - though admittedly its worst for virtual folders.

Would it be possible to pass the actual change around, to the listener functions so that they don't have to get the message database twice, set up search twice etc etc?

The other thought is, are any of these functions/filters/plugins likely to change more than one string in a header at a time? If so, we're going to be doing a lot of extra work on each change, and it may be better to offer some kind of batch update function.

Index: mailnews/db/msgdb/public/nsIDBChangeAnnouncer.idl

nit: uuid change missed

With all the idl functions you are changing, please could you document them in a style similar to http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/mailnews/addrbook/public/nsIAbManager.idl&rev=1.52&mark=65-71#65

+  
+  // onHdrChange is called twice. On the first call, aPrecall is true, and
+  // aStatus is undefined. onHdrChange saves any required status in aStatus
+  // (such as a filter match), which is passed back to onHdrChange,
+  // with aPrecall false, after the database has been updated.  
+  void onHdrChange(in nsIMsgDBHdr aHdrToChange, in PRBool aPrecall, inout PRUint32 aStatus,
+    in nsIDBChangeListener aInstigator);

I don't see aInstigator being used anywhere on the new onHdrChange function, is it really needed?

-	void onParentChanged (in nsMsgKey aKeyChanged, in nsMsgKey oldParent, in nsMsgKey newParent, in nsIDBChangeListener aInstigator);
+	void onParentChanged (in nsMsgKey aKeyChanged, in nsMsgKey oldParent, in nsMsgKey newParent,
+    in nsIDBChangeListener aInstigator);

nit: please drop the tab (and on any other lines in this patch that you're changing).

+NS_IMETHODIMP nsMsgDatabase::SetStringPropertyByHdr(nsIMsgDBHdr *msgHdr, const char *aProperty, const char *aValue)

If we're keeping the two sets of calls, then in this function I think it would be better to:

- Get the change listener count once (not twice).
- Pass the count to the statusArray constructor which will do the SetCapacitor for you.

+      m_ChangeListeners->QueryElementAt(i, NS_GET_IID(nsIDBChangeListener), (void **) getter_AddRefs(changeListener));

I know you copy/pasted this, but you should be using do_QueryElementAt.

+      nsresult rv = changeListener->OnHdrChange(msgHdr, PR_TRUE, &status, nsnull);
+      NS_ENSURE_SUCCESS(rv, rv);

No need to re-declare rv here. Also, do you really want to abort going through all listeners on a change listener failure? Think extension listeners throwing errors when we don't expect them to.

+nsMsgDBFolder::OnHdrChange(nsIMsgDBHdr *aHdrToChange, PRBool aPrecall, PRUint32 *aStatus, 
+  nsIDBChangeListener * aInstigator)

nit: the second line should start just below the n after the ( on the previous line (several places).

+    PRBool match=PR_FALSE;

nit: spaces either side of the = please

"match" can be declared inside the if (searchSession).

+    nsCOMPtr <nsIMsgSearchSession> searchSession = do_QueryReferent(m_searchSession);

nit: no space after the nsCOMPtr.

I generally prefer this form for constructors:

nsCOMPtr <nsIMsgSearchSession> searchSession(do_QueryReferent(m_searchSession));

+    if (numNewMessages + newDelta)
+      m_virtualFolder->SetHasNewMessages(PR_TRUE);
+    else
+      m_virtualFolder->SetHasNewMessages(PR_FALSE);

This can just be m_virtualFolder->SetHasNewMessages((numNewMessages + newDelta) != 0);

Comment 5

[Kent James (:rkent)]

(In reply to comment #4)
> (From update of attachment 318314 [details] [diff] [review])
> The OnHdrChange process seems quite expensive - though admittedly its worst for
> virtual folders.
> 
> Would it be possible to pass the actual change around, to the listener
> functions so that they don't have to get the message database twice, set up
> search twice etc etc?
> 
> The other thought is, are any of these functions/filters/plugins likely to
> change more than one string in a header at a time? If so, we're going to be
> doing a lot of extra work on each change, and it may be better to offer some
> kind of batch update function.
>
For the most common case, which is assignment of junk status, there are actually three changes that occur together: junkstatus, junkstatusorigin, and junkpercent. So there could be some merit to batching. However, that is adding some complexity without any real evidence it is needed. At least in the case of junk processing, OnHdrChanged is cheap compared to the bayesian process itself, which is tokenizing all of the text in the message. My preference would be to not attempt such optimization in this bug, but do it in a later bug if warranted. 
> 
> I don't see aInstigator being used anywhere on the new onHdrChange function, is
> it really needed?
All of the On ... calls seem to add aInstigator, though few really use it. I left it in to follow the same pattern. What's your sense, leave it out?

Comment 6

[Kent James (:rkent)]

Attachment: Response to review, de-bitrot, no rename of onHdrChange

I decided not to rename OnHdrChange to reduce the size and impact of this patch. I fixed unit tests to current spec, de-bit rotted, and fixed Standard8's nits. Concerning some non-nit issues:

1) "I don't see aInstigator being used anywhere" : I don't think this bug is the time to make the decision to remove this from the interface, as all other similar functions include this. There are cases where this is useful. I prefer to keep things consistent.

2) "Would it be possible to pass the actual change around ... it may be better to offer some kind of batch update function" I looked into the batching. There is some incomplete support for that now, but I'm reluctant to extend the support for that for this bug, which is already fairly complex. It would be a good thing to add as a followup bug. Concerning passing the change around (which is the existing approach used for flag changes) that could be used for performance optimization at the expense of complexity and loss of generality. I'm just not sure it's worth the cost for the current situation, where the most common use of this is processing junk changes, which themselves are fairly slow. I'd really prefer to get the most general case in place, and deal with any performance issues as special cases later.

Comment 7

[David :Bienvenu]

Comment on attachment 326772 [details] [diff] [review]
Response to review, de-bitrot, no rename of onHdrChange

I'm going to want to sr this...

+    if (match) *aStatus |= kMatch;
+    if (flags & MSG_FLAG_READ) *aStatus |= kRead;
+    if (flags & MSG_FLAG_NEW) *aStatus |= kNew;

generally, we put the if() condition on a line by itself.

prevailing braces style in this file is not K&R:

+    if (searchSession) {
+      nsMsgViewIndex index = FindHdr(aHdrChanged);

aPreCall might be more meaningfully called aPreChangeCall, or just aPreChange (assuming I understand the sense of it correctly)

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 326772 [details] [diff] [review]
Response to review, de-bitrot, no rename of onHdrChange


>+NS_IMETHODIMP
>+VirtualFolderChangeListener::OnHdrChangeFull(nsIMsgDBHdr *aHdrChanged, PRBool aPrecall, PRUint32 *aStatus, 
>+                                             nsIDBChangeListener * aInstigator)
>+{
...
>+  nsCOMPtr <nsIMsgDatabase> msgDB;
>+  nsresult rv = m_folderWatching->GetMsgDatabase(nsnull, getter_AddRefs(msgDB));

Here you're getting the return value but not checking it.

> NS_IMETHODIMP VirtualFolderChangeListener::OnHdrChange(nsIMsgDBHdr *aHdrChanged, PRUint32 aOldFlags, PRUint32 aNewFlags, nsIDBChangeListener *aInstigator)

>     rv = m_virtualFolder->GetDBFolderInfoAndDB(getter_AddRefs(dbFolderInfo), getter_AddRefs(virtDatabase));
>+    NS_ENSURE_SUCCESS(rv, rv);

This change is now redundant (i.e. bitrot)

> NS_IMETHODIMP
>+nsMsgQuickSearchDBView::OnHdrChangeFull(nsIMsgDBHdr *aHdrChanged, PRBool aPrecall,
>+                                        PRUint32 *aStatus, nsIDBChangeListener *aInstigator)
>+{
...
>+        aHdrChanged->GetFlags(&flags);
>+        (void) aHdrChanged->GetStringProperty("junkscoreorigin", getter_Copies(originStr));
>+        if (!match && originStr.get()[0] == 'p' && flags & MSG_FLAG_NEW)

I think you should add a comment here that 'p' stands for 'plugin'. It took me a few minutes to work out what it was referring to, and in other places where you do this.

>-[scriptable, uuid(1d409e71-3b4e-4611-9759-6335c7362f5c)]
>+/**
>+ * These callbacks are provided to allow listeners to the message database
>+ * to update their status when message changes occur.
>+ */
>+[scriptable, uuid(F2E12285-79D4-4692-AFEF-92415FCC6C5E)]
> 
>+  /*
>+   * Callback when message flags are changed
>+   *
>+   * @param aHdrChanged: the changed header
>+   * @param aOldFlags:   message flags prior to change
>+   * @param aNewFlags:   message flags after change
>+   * @param aInstigator: object that initiated the change
>+   */

Please drop the ':' I expect that may confuse doxygen (unless you know something I don't)

>+NS_IMETHODIMP nsMsgDatabase::SetStringPropertyByHdr(nsIMsgDBHdr *msgHdr, const char *aProperty, const char *aValue)
>+{
...
>+  // Precall OnHdrChangeFull to store prechange status
>+  nsTArray<PRUint32> statusArray;
>+  statusArray.SetCapacity(m_ChangeListeners.Length());

Please make this:

nsTArray<PRUint32> statusArray(m_ChangeListeners.Length());


r=me with those fixed.

Comment 9

[Kent James (:rkent)]

Attachment: More nits fixed, renamed parm to aPreChange

Carrying over r, ready for sr.

Comment 10

[Kent James (:rkent)]

I have a small problem with this that for some reason is occurring in recent builds. Modified patch coming.

Comment 11

[David :Bienvenu]

Comment on attachment 327153 [details] [diff] [review]
More nits fixed, renamed parm to aPreChange

instead of iter1, how about calling the variable listeners? Similarly for iter2

+  nsTObserverArray<nsCOMPtr<nsIDBChangeListener> >::ForwardIterator iter1(m_ChangeListeners);
+  while (iter1.HasMore())
+  {
+    listener = iter1.GetNext();
+    listener->OnHdrChangeFull(msgHdr, PR_TRUE, &status, nsnull);
+    // ignore errors, but append element to keep arrays in sync
+    statusArray.AppendElement(status);
+  }
+

I think you can use the nsIMsgFolderFlags.idl instead of this const:

+const MSG_FOLDER_FLAG_VIRTUAL = 0x0020;                      

extra space on first line - maybe use *** or something instead of a space (and maybe instead of "sanity checks that worked before the bug fix", which made me think they know longer work after the bug fix :-) maybe just call them "basic functionality tests:"
+{
+    // sanity checks that worked before the bug fix
+  
+  // total messages matching search
+  do_check_eq(4, virtualFolder.getTotalMessages(false));

don't need braces here:
+  if (term.matchAll)
+  {
+      condition = "ALL";
+  }

In nsMsgDBFolder::RemoveKeywordsFromMessages,

+      // we're using the message, not a key, to set the string property, because
+      // in the case of filters running on incoming pop3 mail with quarantining
+      // turned on, the message key is wrong.
+      mDatabase->SetStringPropertyByHdr(message, "keywords", keywords.get());

Are we really removing keywords via message filters? If not, this comment is a bit confusing.

Is OnHdrChangeFull only called if a property changes? If so, then the method should be called OnHdrPropertyChanged, and as discussed over IRC, OnHdrChange would be OnHdrFlagedChange

Comment 12

[David :Bienvenu]

Comment on attachment 327153 [details] [diff] [review]
More nits fixed, renamed parm to aPreChange

minusing since we've identified some improvements to make the method names more readable going forward.

Comment 13

[Kent James (:rkent)]

Here's the issue that I mentioned in Comment 10. I thought I could develop a unit test for it, but it only occurs when you use the new mail processing sections in the filter section of new emails (IMAP for example) for which I cannot currently do a unit test. The issue also does not occur in the old code. It is fixed in the patch to be submitted after this comment, but it occurred in previous patches. I mention it here as a possible aid to reviewers.

The underlying problem is that there are many cases where database code (with notifications) is being used to manipulate message headers prior to their being added to the database. There is then a risk of a change (such as an unread count change) getting two notifications - once when the header is manipulated, and a second time when the message header is being added to the database. Really we should make sure that all manipulations of the message header, prior to being added to the database, are done using code that does not result in database notifications. But instead here I approach the problem defensively, by adding a new check in SetStringPropertyByHdr to skip notifications if the message key has not yet been added to the database.

Scenario I am preventing:

1. Create an IMAP account with a filter to add tag "Important" to any message with subject containing "Important"
2. Create a virtual search folder, searching the IMAP inbox, with search Tag Contains Important.
3. Send a message to the IMAP inbox that matches the filter.

Expected result: Unread count on virtual folder increments by one.
Actual result: Unread count on virtual folder increments by two.

Comment 14

[Kent James (:rkent)]

Attachment: Fixed Bienvenu's issues, added defensive check for premature notification

Carrying over r. Added checks in SetStringPropertyByHdr to defend against callers who are using this before adding the header to the database. Fixed David's issues, made some minor formatting changes to better line up some second statement lines.

Comment 15

[David :Bienvenu]

Comment on attachment 328334 [details] [diff] [review]
Fixed Bienvenu's issues, added defensive check for premature notification

thx for doing this, Kent.

Some nits - all the other notifications are past tense, like Added, Deleted, onJunkScoreChanged, etc. So It would be more consistent to say OnHdrFlagsChanged, OnHdrPropertyChanged. Oh, and it should be onHdrPropertyChanged, not OnHdrPropertyChange (note lower case o) likewise, onHdrFlagsChanged.

Do we really need onJunkScoreChanged now? Could that just be OnHdrPropertyChanged and the listener can check if the junk property changed?

Comment 16

[[e-mail]]

Comment on attachment 328334 [details] [diff] [review]
Fixed Bienvenu's issues, added defensive check for premature notification

>+  void OnHdrPropertyChange(in nsIMsgDBHdr aHdrToChange, in PRBool aPreChange, inout PRUint32 aStatus,
>+                       in nsIDBChangeListener aInstigator);
Having one listener called twice seems to be unusual. I had a look through other interfaces and the most common pattern seems to be OnHdrPropertyWillChange / OnHdrPropertyDidChange although my personal preference would be OnHdrPropertyChanging / OnHdrPropertyChanged.

Comment 17

[Kent James (:rkent)]

(In reply to comment #16)
>
> Having one listener called twice seems to be unusual. I had a look through
> other interfaces and the most common pattern seems to be
> OnHdrPropertyWillChange / OnHdrPropertyDidChange although my personal
> preference would be OnHdrPropertyChanging / OnHdrPropertyChanged.
> 
I'm going to have to disagree with this. Your scheme has the advantage that the naming makes it clearer what is going on. But the disadvantage is that, in the primary use of the pre-change call in virtual folder listeners, there is significant code that is identical in both the prechange and post change versions. Eliminating duplicated code is a Good Thing. There's also the issue of the definitions of the aStatus variable flags (the constants kMatch, kRead, and kNew), which are encapsulated in a single function in the my current version, but would need to be external if we split the routine. That's also a Good Thing.

Comment 18

[[e-mail]]

Comment on attachment 328334 [details] [diff] [review]
Fixed Bienvenu's issues, added defensive check for premature notification

>+  PRInt32 totalDelta, unreadDelta, newDelta;
>+
>+  if (wasMatch && match)  // both previous and new header are in virtual folder
>+  {
>+    totalDelta = 0;
>+    if (flags & MSG_FLAG_READ)
>+      unreadDelta = *aStatus & kRead ? 0 : -1;
>+    else
>+      unreadDelta = *aStatus & kRead ? 1 : 0;
>+    if (flags & MSG_FLAG_NEW)
>+      newDelta = *aStatus & kNew ? 0 : 1;
>+    else
>+      newDelta = *aStatus & kNew ? -1 : 0;
>+  }
>+
>+  else if (match)  // only changed header matches
>+  {
>+    totalDelta = 1;
>+    unreadDelta = flags & MSG_FLAG_READ ? 0 : 1;
>+    newDelta = flags & MSG_FLAG_NEW ? 1 : 0;
>+  }
>+
>+  else  // only previous header matches
>+  {
>+    totalDelta = -1;
>+    unreadDelta = *aStatus & kRead ? 0 : -1;
>+    newDelta = *aStatus & kNew ? -1 : 0;
>+  }
I feel sure that there's a simpler way of doing this, something like this:
PRInt32 totalDelta = 0, unreadDelta = 0, newDelta = 0;
if (match) {
  totalDelta++;
  if (!(flags & MSG_FLAG_READ)) unreadDelta++;
  if (flags & MSG_FLAG_NEW) newDelta++;
}
if (wasMatch) {
  totalDelta--;
  if (!(aStatus & kRead)) unreadDelta--;
  if (aStatus & kNew) newDelta--;
}

Comment 19

[[e-mail]]

Comment on attachment 328334 [details] [diff] [review]
Fixed Bienvenu's issues, added defensive check for premature notification

>+    *aStatus = 0;
>+    if (match)
>+      *aStatus |= kMatch;
>+    if (flags & MSG_FLAG_READ)
>+      *aStatus |= kRead;
>+    if (flags & MSG_FLAG_NEW)
>+      *aStatus |= kNew;
>+    return NS_OK;
A pity there's no MSG_FLAG_MATCH otherwise you could write
*aStatus = flags | MSG_FLAG_MATCH;

Comment 20

[[e-mail]]

Comment on attachment 328334 [details] [diff] [review]
Fixed Bienvenu's issues, added defensive check for premature notification

>+  if (totalDelta)
>+    dbFolderInfo->ChangeNumMessages(totalDelta);
...
>+  if (totalDelta)
>+  {
>+    nsCString searchUri;
>+    m_virtualFolder->GetURI(searchUri);
>+    msgDB->UpdateHdrInCache(searchUri.get(), aHdrChanged, totalDelta == 1);
>+  }
Is it not possible to do these in the same block?

>+    PRInt32 numNewMessages;
>+    m_virtualFolder->GetNumNewMessages(PR_FALSE, &numNewMessages);
>+    m_virtualFolder->SetNumNewMessages(numNewMessages + newDelta);
>+    m_virtualFolder->SetHasNewMessages(numNewMessages + newDelta > 0);
Use numNewMessages += newDelta; perhaps?

Comment 21

[Kent James (:rkent)]

Attachment: Fixed Neil's nits, patch to checkin

Carrying over reviews. I implemented all of Neil's suggestions except the one to split OnHdrPropertyChanged into two routines. Carrying over review flags, ready for check-in.

Comment 22

[Kent James (:rkent)]

Asking for checkin-needed. I see I ignored Neil's nit:

>+    PRInt32 numNewMessages;
>+    m_virtualFolder->GetNumNewMessages(PR_FALSE, &numNewMessages);
>+    m_virtualFolder->SetNumNewMessages(numNewMessages + newDelta);
>+    m_virtualFolder->SetHasNewMessages(numNewMessages + newDelta > 0);
Use numNewMessages += newDelta; perhaps?

I'm hoping to change the way new counts are kept in bug 441932, so I'm not going to redo the patch at this point for Neil's minor nit.

Comment 23

[Mark Banner (:standard8)]

Checking in mailnews/base/src/nsMsgAccountManager.cpp;
/cvsroot/mozilla/mailnews/base/src/nsMsgAccountManager.cpp,v  <--  nsMsgAccountManager.cpp
new revision: 1.340; previous revision: 1.339
done
Checking in mailnews/base/src/nsMsgDBView.cpp;
/cvsroot/mozilla/mailnews/base/src/nsMsgDBView.cpp,v  <--  nsMsgDBView.cpp
new revision: 1.319; previous revision: 1.318
done
Checking in mailnews/base/src/nsMsgGroupView.cpp;
/cvsroot/mozilla/mailnews/base/src/nsMsgGroupView.cpp,v  <--  nsMsgGroupView.cpp
new revision: 1.55; previous revision: 1.54
done
Checking in mailnews/base/src/nsMsgGroupView.h;
/cvsroot/mozilla/mailnews/base/src/nsMsgGroupView.h,v  <--  nsMsgGroupView.h
new revision: 1.12; previous revision: 1.11
done
Checking in mailnews/base/src/nsMsgQuickSearchDBView.cpp;
/cvsroot/mozilla/mailnews/base/src/nsMsgQuickSearchDBView.cpp,v  <--  nsMsgQuickSearchDBView.cpp
new revision: 1.38; previous revision: 1.37
done
Checking in mailnews/base/src/nsMsgQuickSearchDBView.h;
/cvsroot/mozilla/mailnews/base/src/nsMsgQuickSearchDBView.h,v  <--  nsMsgQuickSearchDBView.h
new revision: 1.11; previous revision: 1.10
done
Checking in mailnews/base/src/nsMsgXFVirtualFolderDBView.cpp;
/cvsroot/mozilla/mailnews/base/src/nsMsgXFVirtualFolderDBView.cpp,v  <--  nsMsgXFVirtualFolderDBView.cpp
new revision: 1.24; previous revision: 1.23
done
Checking in mailnews/base/src/nsMsgXFVirtualFolderDBView.h;
/cvsroot/mozilla/mailnews/base/src/nsMsgXFVirtualFolderDBView.h,v  <--  nsMsgXFVirtualFolderDBView.h
new revision: 1.9; previous revision: 1.8
done
RCS file: /cvsroot/mozilla/mailnews/base/test/unit/test_bug428427.js,v
done
Checking in mailnews/base/test/unit/test_bug428427.js;
/cvsroot/mozilla/mailnews/base/test/unit/test_bug428427.js,v  <--  test_bug428427.js
initial revision: 1.1
done
Checking in mailnews/base/util/nsMsgDBFolder.cpp;
/cvsroot/mozilla/mailnews/base/util/nsMsgDBFolder.cpp,v  <--  nsMsgDBFolder.cpp
new revision: 1.353; previous revision: 1.352
done
Checking in mailnews/db/msgdb/public/nsIDBChangeListener.idl;
/cvsroot/mozilla/mailnews/db/msgdb/public/nsIDBChangeListener.idl,v  <--  nsIDBChangeListener.idl
new revision: 1.10; previous revision: 1.9
done
Checking in mailnews/db/msgdb/public/nsIMsgDatabase.idl;
/cvsroot/mozilla/mailnews/db/msgdb/public/nsIMsgDatabase.idl,v  <--  nsIMsgDatabase.idl
new revision: 1.55; previous revision: 1.54
done
Checking in mailnews/db/msgdb/src/nsMsgDatabase.cpp;
/cvsroot/mozilla/mailnews/db/msgdb/src/nsMsgDatabase.cpp,v  <--  nsMsgDatabase.cpp
new revision: 1.405; previous revision: 1.404
done
Checking in mailnews/local/src/nsParseMailbox.cpp;
/cvsroot/mozilla/mailnews/local/src/nsParseMailbox.cpp,v  <--  nsParseMailbox.cpp
new revision: 1.307; previous revision: 1.306
done