465985 - nsMessengerOSXIntegration registers for root folder changes twice

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[David Humphrey (:humph)]

I've noticed that we get two BiffState notifications in the past, but as I was working on bug 465381 I saw that this is a more general problem.  Here's nsMessengerOSXIntegration::Init()

  // because we care if the default server changes                                                                                                                              
  rv = accountManager->AddRootFolderListener(this);
  NS_ENSURE_SUCCESS(rv,rv);

  nsCOMPtr<nsIMsgMailSession> mailSession = do_GetService(NS_MSGMAILSESSION_CONTRACTID, &rv);
  NS_ENSURE_SUCCESS(rv,rv);

  // because we care if the unread total count changes                                                                                                                          
  return mailSession->AddFolderListener(this, nsIFolderListener::boolPropertyChanged | nsIFolderListener::intPropertyChanged);

If I get rid of the call to accountManager->AddRootFolderListener(this), the problem goes away.  I think this is the case because when we biff on a folder, we pass back the root folder vs. the inbox, therefore we don't lose anything by not getting ItemInt changes from the account/root itself.

I notice that nsMessengerUnixIntegration doesn't have this duplication, but nsMessengerWinIntegration does, since you watch for the DefaultServerAtom in nsMessengerWinIntegration::OnItemBoolPropertyChanged.

I favour ripping out the AddRootFolderListener call, since I currently don't care if the server changes.  I'm not sure if I should, like Windows.  

Let me know and I'll do the patch.

Comment 1

[David :Bienvenu]

On Windows, we do seem to care if the default server changes, because that causes us to do a bunch of things with registry keys where we store unread counts for the servers. I'm not sure why we care about the default server differently than we do for other servers, though.

I don't think MacOSX integration cares, like you say, so I think we can rip that out.

Comment 2

[David Humphrey (:humph)]

Attachment: Removes first root folder listener

Comment 3

[David :Bienvenu]

Comment on attachment 349273 [details] [diff] [review]
Removes first root folder listener

is it now the case that we don't have to include "nsIMsgAccountManager.h"?

Comment 4

[David Humphrey (:humph)]

Attachment: Patch with nsIMsgAccountManager.h include removed

Indeed, that can safely come out.  Thanks for spotting that.

Comment 5

[David Humphrey (:humph)]

Attachment: Patch with nsIMsgAccountManager.h include removed (comment removed)

Sorry, left in a comment.  This one is right.

Comment 6

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/06aafa6c4646