439480 - UI hang with Not enough arguments [nsIMsgImapMailFolder.updateFolder]

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Kent James (:rkent)]

Using a TB trunk build based on 2008-06-15 download, the UI hung after I:

1) Created a subfolder "subtest" of an IMAP server as such: Inbox/test/subtest
2) Exited TB, and re-started to get subtest to appear
3) Redefined a filter that previously moved to "test" to move instead to "subtest"
4) Sent a message matching the filter, tried to open subtest folder.

UI hangs with an hourglass. In the console, I have the error:

JavaScript error: chrome://global/content/bindings/tree.xml, line 951: uncaught
exception: [Exception... "Not enough arguments [nsIMsgImapMailFolder.updateFolde
r]"  nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)"  location: "JS frame
:: chrome://messenger/content/commandglue.js :: ChangeFolder :: line 214"  data:
 no]

The commandglue.js location referenced is the following code:

251     //Need to do this after rerooting folder.  Otherwise possibility of receiving folder loaded
252     //notification before folder has actually changed.
253     if (viewType != nsMsgViewType.eShowVirtualFolderResults)
254       folder.updateFolder(msgWindow);

Investigating, updateFolder is defined in two different interfaces with the same name, but different number of parameters. Probably we shouldn't do that.

Comment 1

[Joey Minta]

This strikes me not as a regression, but rather as a previously undiscovered bug.  In any case, I can't actually find anyone who uses nsIMsgImapMailFolder's version of updateFolder.  The method was added in bug 393727, ostensibly for extensions to use.  I'm adding Nick to the cc list to hopefully confirm.

Comment 2

[Kent James (:rkent)]

(In reply to comment #1)
> This strikes me not as a regression, but rather as a previously undiscovered
> bug.  In any case, I can't actually find anyone who uses nsIMsgImapMailFolder's
> version of updateFolder.  The method was added in bug 393727, ostensibly for
> extensions to use.  I'm adding Nick to the cc list to hopefully confirm.
> 
You may be right. In any case, even if it was manifested by some of the recent checkins, it probably is still considered a bug in either commandglue.js, or of the original definition of the two-parameter version of updateFolder.

It would be very easy to just rename the updateFolder method in nsIMsgImapMailFolder to something different (such as UpdateFolderWithNotification) to prevent this problem. I would be happy to do that patch with a little encouragement.

Comment 3

[Wayne Mery (:wsmwk)]

(In reply to comment #2)
> It would be very easy to just rename the updateFolder method in
> nsIMsgImapMailFolder to something different (such as
> UpdateFolderWithNotification) to prevent this problem. I would be happy to do
> that patch with a little encouragement.

need bienvenu input?

Comment 4

[David :Bienvenu]

(In reply to comment #2)

> It would be very easy to just rename the updateFolder method in
> nsIMsgImapMailFolder to something different (such as
> UpdateFolderWithNotification) to prevent this problem. I would be happy to do
> that patch with a little encouragement.

yes, consider yourself encouraged :-)

Comment 5

[Kent James (:rkent)]

Attachment: Renamed to UpdateFolderWithListener

Comment 6

[David :Bienvenu]

Comment on attachment 382364 [details] [diff] [review]
Renamed to UpdateFolderWithListener

this doesn't look like the right patch.

Comment 7

[Kent James (:rkent)]

Attachment: Right patch this time.

You're right, sorry, here's the correct one.

Comment 8

[David :Bienvenu]

Comment on attachment 382785 [details] [diff] [review]
Right patch this time.

nit - should be aMsgWindow in the impls. Thx for the patch.

Comment 9

[Kent James (:rkent)]

Attachment: Fixed nits, patch to checkin (carrying over reviews)

Comment 10

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/6129e94e5870