bugzilla.mozilla.org will be intermittently unavailable on Saturday, March 24th, from 16:00 until 20:00 UTC.

The transaction object for undo/redo should hold a weakptr reference to the folder



MailNews Core
17 years ago
10 years ago


(Reporter: Navin Gupta, Assigned: Navin Gupta)


Firefox Tracking Flags

(Not tracked)



(1 attachment)



17 years ago

Comment 1

17 years ago
Created attachment 44490 [details] [diff] [review]
proposed fix

Comment 2

17 years ago
So the Undo object has a weakptr to the src and dest folders. Also the 
imap/local folder no longer holds the transaction manager because it 
was causing txnMgr leaks, instead I use the msgWindow object to get 
the txnMgr object.

This patch also includes the fix for one of the cases of empty hdrs on 
undo in bug 86489. Upon deleting a folder it leaks so we try to
get database when we should not, therefore check if the folder exists on

Comment 3

17 years ago
yuck, nsLocalMailFolder shouldn't have a pointer to a msgWindow - what if you
have multiple message windows open?

Comment 4

17 years ago
That is nsLocalMailCopyState that I have msgWindow to. The nsLocalMailCopyState
class definition is in nsLocalMailFolder.cpp

Comment 5

17 years ago
Ignore these changes in nsImapMailFolder. They are not needed. 

+    PRBool exists;
+    rv = pathSpec->Exists(&exists);
+    NS_ENSURE_SUCCESS(rv,rv);
+    if (!exists) return NS_ERROR_NULL_POINTER;  //mDatabase will be null at 
this point.

Comment 6

17 years ago
I know that this was messed up before you got here...but
the problem is not what source file it's defined in; it's what object holds onto
one. If an nsLocalMailFolder holds onto an nsIMsgWindow, then you're tying that
folder to just one window, and you're also introducing a great opportunity for
circular references. And I'm worried that people will start using that msgWindow
for other stuff, and it will break the whole model/view distinction that we
should be having. The copy state should belong to the url that's doing the copy.
Plus, unless I'm missing something, the nsLocalMailFolder::m_msgWindow never
gets set to anything.

Comment 7

17 years ago
could you make this an nsXPIDLCString while you're at it:

 char *uri = nsnull;
-    rv = m_srcFolder->GetURI(&uri);
+    rv = srcFolder->GetURI(&uri);

and remove the PR_FREEIF?

otherwise, looks good, r=bienvenu

Comment 8

17 years ago
marking fixed. fix checked in last week

Comment 9

17 years ago
Last Resolved: 17 years ago
Resolution: --- → FIXED
OS: Windows NT → All
QA Contact: esther → stephend
Hardware: PC → All
Verified FIXED using lxr.mozilla.org:

nsImapMailFolder.cpp 1.413
nsImapMailFolder.h 1.167
nsImapUndoTxn.cpp 1.30
nsImapUndoTxn.h 1.14
nsLocalMailFolder.cpp 1.300
nsLocalMailFolder.h 1.109
nsLocalUndoTxn.cpp 1.24
nsLocalUndoTxn.h 1.9
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.