UI hang with Not enough arguments [nsIMsgImapMailFolder.updateFolder]

RESOLVED FIXED in Thunderbird 3.0b3

Status

Thunderbird
Mail Window Front End
--
critical
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: rkent, Assigned: rkent)

Tracking

({hang, regression})

Trunk
Thunderbird 3.0b3
x86
Windows XP
hang, regression

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

10 years ago
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.
Blocks: 436718
Keywords: hang, regression

Comment 1

10 years ago
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.
(Assignee)

Comment 2

10 years ago
(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

9 years ago
(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?
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 4

9 years ago
(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 :-)
(Assignee)

Updated

9 years ago
Assignee: nobody → kent
Status: NEW → ASSIGNED
(Assignee)

Comment 5

9 years ago
Created attachment 382364 [details] [diff] [review]
Renamed to UpdateFolderWithListener
Attachment #382364 - Flags: superreview?(bienvenu)
Attachment #382364 - Flags: review?(bienvenu)
(Assignee)

Updated

9 years ago
Whiteboard: [needs r/sr bienvenu]

Comment 6

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

this doesn't look like the right patch.
(Assignee)

Comment 7

9 years ago
Created attachment 382785 [details] [diff] [review]
Right patch this time.

You're right, sorry, here's the correct one.
Attachment #382364 - Attachment is obsolete: true
Attachment #382785 - Flags: superreview?(bienvenu)
Attachment #382785 - Flags: review?(bienvenu)
Attachment #382364 - Flags: superreview?(bienvenu)
Attachment #382364 - Flags: review?(bienvenu)

Updated

9 years ago
Attachment #382785 - Flags: superreview?(bienvenu)
Attachment #382785 - Flags: superreview+
Attachment #382785 - Flags: review?(bienvenu)
Attachment #382785 - Flags: review+

Comment 8

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

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

Comment 9

9 years ago
Created attachment 383539 [details] [diff] [review]
Fixed nits, patch to checkin (carrying over reviews)
Attachment #382785 - Attachment is obsolete: true
Attachment #383539 - Flags: superreview+
Attachment #383539 - Flags: review+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Whiteboard: [needs r/sr bienvenu] → [needs checkin]
Checked in: http://hg.mozilla.org/comm-central/rev/6129e94e5870
Keywords: checkin-needed
Whiteboard: [needs checkin]
Target Milestone: --- → Thunderbird 3.0b3
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.