555976 - nsIMsgFolder/nsLocalMailFolder don't use the nsIMsgCopyServiceListener when calling copyMessages method.

Status: RESOLVED INVALID

(MailNews Core :: Backend, defect)

Description

[IagoSRL]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; es-ES; rv:1.9.2.2) Gecko/20100316 Firefox/3.6.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; es-ES; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3

From Javascript, programming an extension, I have a valid nsIMsgFolder instance (from an IMAP account) and try to move messages calling its method 'copyMessages' and passing an object that implements nsIMsgCopyServiceListener.
When execute, 'copyMessages' must call the listener methods 'OnStartCopy', 'OnStopCopy' and 'OnProgress' but only call 'SetMessageKey'.

But using the analogous method from the object 'nsMsgCopyService' with the same code, this works better (not perfect, see Bug 263997): methods 'OnStartCopy', 'OnStopCopy' and 'SetMessageKey' are called but not 'OnProgress'.

At first I thought that nsLocalMailFolder.copyMessages (http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsLocalMailFolder.cpp#1667) internally call nsMsgCopyService.CopyMessages (http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgCopyService.cpp#434) using the nsLocalMailFolderinstance in the 'dstFolder' parameter, but seems to be different code for the same task.

Bug 263997 is related with this but not duplicated.

Reproducible: Always




In my javascript code I have a nsIMsgFolder instance called 'toFolder', a nsIMutableArray instance called 'messages' and I run:
  toFolder.copyMessages(fromFolder, messages, true, null, new MoveMessagesListener(fromFolder.prettiestName), false, false);


With nsIMsgCopyService is:
  var msgCopyService = Components.classes['@mozilla.org/messenger/messagecopyservice;1'].getService(Components.interfaces.nsIMsgCopyService);
  msgCopyService.CopyMessages(fromFolder, messages, toFolder, true, new MoveMessagesListener(fromFolder.prettiestName), null, false);


And the listener is defined as:

function MoveMessagesListener(folderName) {
		this.folderName = folderName;
		// Interface   nsISupports
		this.QueryInterface = function(iid) {
			Application.console.log("queryinterface: " + iid);
			if (iid.equals(Components.interfaces.nsIMsgCopyServiceListener) || iid.equals(Components.interfaces.nsISupports)) return this;
			throw Components.results.NS_NOINTERFACE; return 0;
		};
		this.AddRef = function() {Application.console.log("addref");};
		this.Release = function() {Application.console.log("release");};
		// Interface   nsIMsgCopyServiceListener
		this.OnProgress = function (progress, progressMax) {Application.console.log("onprogress: " + progress);};
		this.OnStartCopy = function () {
			Application.console.log("C[" + Date() + "] Start " + this.folderName);
		};
		this.OnStopCopy = function (status) {
			if (Components.results.NS_OK === status) {
				// CORRECTO:
				Application.console.log("C[" + Date() + "] End Ok " + this.folderName);
			} else {
				// FALLO:
				Application.console.log("C[" + Date() + "] End Error: " + this.folderName);
			}
		};
		this.SetMessageKey = function (key) { Application.console.log("setmessagekey: " + key); this.key=key; };
		this.GetMessageId = function (messageId) { Application.console.log("getmessageid: " + messageid);};
}

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

I'm pretty sure you should be using the copy service for this purpose.  My impression of the code is that the folder method is intended for use by the copy service...

We should probably put an example code blob at the place-holder at:
https://developer.mozilla.org/en/Extensions/Thunderbird/HowTos/Common_Thunderbird_Use_Cases/Open_Folder#Copy_Selected_Messages

bienvenu, can you confirm that the copy service is the correct mechanism to use?

Comment 2

[David :Bienvenu]

yes, in general you want to go through the copy service.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Thanks for the confirmation, I will update the page.

Comment 4

[Ludovic Hirlimann [:Usul]]

So this is INVALID , right ?

Comment 5

[David :Bienvenu]

looks invalid to me - extensions should be using the copy service.

Comment 6

[Ludovic Hirlimann [:Usul]]

And the documentation is being updated anyway.