nsMsgMailSesssion::AddFolderListener allows duplicate listeners

RESOLVED FIXED in mozilla1.9

Status

MailNews Core
Backend
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: asuth, Assigned: asuth)

Tracking

(Depends on: 1 bug)

Trunk
mozilla1.9

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
Created attachment 322123 [details] [diff] [review]
don't add duplicate listeners, assert.

nsMsgMailSession::AddFolderListener uses AppendElement without checking if the listener is already in the list (or using AppendElementUnlessExists).  This allows duplicate listeners to be added; it's hard to conceive of a situation where this would be desirable.  If there is such a case, it seems like maintaining a counter for each listener rather than having it around multiple times would be better.

Bug 296453 is an example of a case where using AppendElementUnlessExists would have saved us from a javascript error.

In order to assist in detecting attempts to add duplicate listeners, the patch adds an assertion along the lines of the one used by RemoveFolderListener and only adds non-duplicate listeners.  The checking only considers the listener itself, not the notification flags.  (I am assuming no one would be crazy enough to intentionally add the same listener multiple times with different flags, although that case could be handled.)
Attachment #322123 - Flags: review?(bienvenu)

Updated

10 years ago
Attachment #322123 - Flags: review?(bienvenu) → review+
(Assignee)

Updated

10 years ago
Attachment #322123 - Flags: superreview?(neil)

Updated

10 years ago
Attachment #322123 - Flags: superreview?(neil) → superreview+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
Checking in mailnews/base/src/nsMsgMailSession.cpp;
/cvsroot/mozilla/mailnews/base/src/nsMsgMailSession.cpp,v  <--  nsMsgMailSession.cpp
new revision: 1.92; previous revision: 1.91
done
Keywords: checkin-needed
Depends on: 436366
(Assignee)

Comment 2

10 years ago
Marking fixed since it's checked in.  We'll deal with the mischievous code in bug 436366 that triggers this assertion.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Updated

10 years ago
Target Milestone: --- → mozilla1.9
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.