438335 - CopyRequest not cleared when local folder is moved

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Rain [:rain1] (inactive)]

Attachment: patch

When a local folder is moved, a CopyRequest is created but not immediately cleared. This also means that the OnStopCopy event doesn't happen for this. (The CopyRequest clears only at shutdown, when the objects are being destroyed.)

This patch should fix this.

Notes:
- the patch won't work on trunk, the one for bug 437848 needs to get in first, there's a changed variable name
- Is this the correct place to call OnCopyCompleted()?
- the nsISupports QI is needed as the FindRequest function compares nsISupports addresses
- the nsIMsgFolderListener notification is now sent from ClearRequest()
- there's a unit test coming up for this and for nsIMsgFolderListeners for local folders

Comment 1

[Rain [:rain1] (inactive)]

s/there's a changed variable name/there's a new variable/ , whoops

Comment 2

[David :Bienvenu]

I'm going to have to check this out myself - I've applied the other patch (I'd check it in, but the tree seems closed) and now I'll apply this patch.

Comment 3

[David :Bienvenu]

for copying local folders, the nsIMsgFolderListener notification is sent from here:

http://mxr.mozilla.org/mozilla/source/mailnews/local/src/nsLocalMailFolder.cpp#2010

unless I'm missing something. I'll look into the copy request stuff this afternoon.

Comment 4

[Rain [:rain1] (inactive)]

David -- er, yes, that's exactly what I removed in the patch as it was resulting in duplicate notifications (once from there, once in OnStopCopy()).

Hmm, the change breaks notifications for deleting a folder to the trash:
http://mxr.mozilla.org/mozilla/source/mailnews/local/src/nsLocalMailFolder.cpp#1001

IMHO we should use the copy service for this too, for consistency.

Comment 5

[Rain [:rain1] (inactive)]

Just tested with the copy service, and it works correctly, giving the itemMoveCopyCompleted notification. However we don't have a copy listener object to pass in to the copy service, thus we can't get an OnStopCopy notification. Should this object be passed in as an extra parameter, like msgWindow?

Comment 6

[Rain [:rain1] (inactive)]

Attachment: sorry, wrong patch

Comment 7

[Rain [:rain1] (inactive)]

Comment on attachment 324664 [details] [diff] [review]
sorry, wrong patch

wrong patch, sorry

Comment 8

[Rain [:rain1] (inactive)]

Attachment: patch with delete to trash using copy service

Comment 9

[Rain [:rain1] (inactive)]

Attachment: Finally, hopefully the correct patch

Sorry for all the messing up above, I didn't realise I had the wrong hg patch loaded when I was making those changes :(

Comment 10

[Mark Banner (:standard8)]

Checking in mailnews/base/src/nsMsgCopyService.cpp;
/cvsroot/mozilla/mailnews/base/src/nsMsgCopyService.cpp,v  <--  nsMsgCopyService.cpp
new revision: 1.62; previous revision: 1.61
done
Checking in mailnews/local/src/nsLocalMailFolder.cpp;
/cvsroot/mozilla/mailnews/local/src/nsLocalMailFolder.cpp,v  <--  nsLocalMailFolder.cpp
new revision: 1.587; previous revision: 1.586
done

Comment 11

[Rain [:rain1] (inactive)]

Covered by the test in attachment 326455 [details] [diff] [review] in bug 436880.