Last Comment Bug 855631 - Get New Messages for all accounts does not work
: Get New Messages for all accounts does not work
Status: RESOLVED FIXED
: regression
Product: MailNews Core
Classification: Components
Component: Networking: POP (show other bugs)
: Trunk
: All All
: -- blocker (vote)
: Thunderbird 23.0
Assigned To: neil@parkwaycc.co.uk
:
:
Mentors:
Depends on:
Blocks: 831993
  Show dependency treegraph
 
Reported: 2013-03-28 00:59 PDT by Sven Grull
Modified: 2013-04-09 02:13 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed
?


Attachments
Possible patch (7.99 KB, patch)
2013-03-28 18:16 PDT, neil@parkwaycc.co.uk
mkmelin+mozilla: review+
acelists: feedback+
standard8: approval‑comm‑aurora+
Details | Diff | Splinter Review

Description Sven Grull 2013-03-28 00:59:14 PDT
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:22.0) Gecko/20100101 Firefox/22.0 SeaMonkey/2.19a1
Build ID: 20130327003001

Steps to reproduce:

With recent trunk builds get New Messages for all accounts does not work anymore. Either invoked via menu or shortcut. It happens also in safe mode.

Output of error console:
Error: [Exception... "Not enough arguments [nsIPop3IncomingServer.downloadMailFromServers]"  nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)"  location: "JS frame :: chrome://messenger/content/mailTasksOverlay.js :: MailTasksGetMessagesForAllServers :: line 134"  data: no]
Source File: chrome://messenger/content/mailTasksOverlay.js
Line: 140
Comment 1 :aceman 2013-03-28 01:22:20 PDT
Regressed by bug 831993.
This also affects Thunderbird in the same way.

I could almost fix this myself but I am not sure what type of array needs to be passed from Javascript to the new function. Neil changed the C++ function to take an array of nsIPop3IncommingServers, which is unfamiliar to me (it is not nsIArray or similar).
Comment 2 :aceman 2013-03-28 01:23:57 PDT
The message for TB is:
[Exception... "Not enough arguments [nsIPop3IncomingServer.downloadMailFromServers]"  nsresult: "0x80570001 (NS_ERROR_XPC_NOT_ENOUGH_ARGS)"  location: "JS frame :: chrome://messenger/conte
nt/mailWindowOverlay.js :: MsgGetMessagesForAllServers :: line 1433"  data: no]

There are 2 spots where this function is used in TB.
Comment 3 :aceman 2013-03-28 01:42:43 PDT
The files where this needs to be fixed are separate for TB and SM but I think we can fix it together in this one bug.
Comment 4 neil@parkwaycc.co.uk 2013-03-28 18:16:45 PDT
Created attachment 731007 [details] [diff] [review]
Possible patch

I modernised the Thunderbird code to bring it more into line with SeaMonkey and incidentally remove another nsISupportsArray.

The interesting bit is in the very last patch hunk, the rest is just nsISupportsArray removal noise.
Comment 5 :aceman 2013-03-29 04:21:28 PDT
Comment on attachment 731007 [details] [diff] [review]
Possible patch

Yes, this seems to work for me on TB.
Comment 6 Magnus Melin 2013-04-04 03:56:59 PDT
Comment on attachment 731007 [details] [diff] [review]
Possible patch

Review of attachment 731007 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/mailWindowOverlay.js
@@ +2688,5 @@
>      {
>        inboxFolder.biffState =  Components.interfaces.nsIMsgFolder.nsMsgBiffState_NoMail;
>        inboxFolder.clearNewMessages();
>      }
> +    localFoldersToDownloadTo.push(inboxFolder);

i guess we can drop the |if (inboxFolder)| above.
or else we're just pushing null
Comment 7 neil@parkwaycc.co.uk 2013-04-04 13:40:36 PDT
Pushed comm-central changeset f3890baa9684.
Comment 8 neil@parkwaycc.co.uk 2013-04-04 13:43:04 PDT
Comment on attachment 731007 [details] [diff] [review]
Possible patch

[Approval Request Comment]
Regression caused by (bug #): 831993
Comment 9 Mark Banner (:standard8) 2013-04-09 02:09:01 PDT
Comment on attachment 731007 [details] [diff] [review]
Possible patch

Yep, a=me. I'll land this in a bit, as I'm landing other things as well.
Comment 10 Mark Banner (:standard8) 2013-04-09 02:13:32 PDT
https://hg.mozilla.org/releases/comm-aurora/rev/7c7e93acfc7d

Note You need to log in before you can comment on or make changes to this bug.