Closed
Bug 435228
Opened 16 years ago
Closed 16 years ago
nsMsgMailSesssion::AddFolderListener allows duplicate listeners
Categories
(MailNews Core :: Backend, defect)
MailNews Core
Backend
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9
People
(Reporter: asuth, Assigned: asuth)
References
(Depends on 1 open bug)
Details
Attachments
(1 file)
1.05 KB,
patch
|
Bienvenu
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
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•16 years ago
|
Attachment #322123 -
Flags: review?(bienvenu) → review+
Assignee | ||
Updated•16 years ago
|
Attachment #322123 -
Flags: superreview?(neil)
Updated•16 years ago
|
Attachment #322123 -
Flags: superreview?(neil) → superreview+
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Comment 1•16 years ago
|
||
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
Assignee | ||
Comment 2•16 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
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Target Milestone: --- → mozilla1.9
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•