647699 - Make ::addMessage return an nsIMsgDBHdr on successful add.

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[alta88]

from https://bugzilla.mozilla.org/show_bug.cgi?id=522645#c22

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

Comment 2

[David :Bienvenu]

Comment on attachment 594754 [details] [diff] [review]
proposed fix

Thx for the patch! A single reviewer can't do both r and sr anymore, so I'll ask Neil for sr on the interface change. The patch looks good to me; just running through the tests now.

Comment 3

[David :Bienvenu]

Comment on attachment 594754 [details] [diff] [review]
proposed fix

this seems to break mailnews/base/test/unit/test_retention.js, perhaps because it causes a db to get held open. I'll try to see if I can fix that.

Comment 4

[David :Bienvenu]

Attachment: fix retention test failing

OK, I tweaked test_retention.js so that it should pass, by adding a forceGC. Magnus, please make sure that you include that change before landing (after Neil's sr, of course :-))

Comment 5

[[e-mail]]

Comment on attachment 597231 [details] [diff] [review]
fix retention test failing

>   const char *aMessages[] = {aMessage};
>-  return AddMessageBatch(1, aMessages);
>+  nsresult rv = AddMessageBatch(1, aMessages, getter_AddRefs(hdrs));
[Can you not use &aMessage instead?]

>+  nsCOMPtr<nsIMsgDBHdr> hdr(do_QueryElementAt(hdrs, 0, &rv));
[It's a pity there's no CallQueryElementAt(hdrs, 0, aHdr)]

>+  NS_IF_ADDREF(*aHdr = hdr);
hdr.forget(aHdr);

>-[test_localSubFolders.js]
>+[test_nsIMsgLocalMailFolder.js]
This needs to be an hg rename as well, no?

Comment 6

[Magnus Melin [:mkmelin]]

(In reply to [e-mail] from comment #5)
> Comment on attachment 597231 [details] [diff] [review]
> fix retention test failing
> 
> >   const char *aMessages[] = {aMessage};
> >-  return AddMessageBatch(1, aMessages);
> >+  nsresult rv = AddMessageBatch(1, aMessages, getter_AddRefs(hdrs));
> [Can you not use &aMessage instead?]

I didn't get this to work.

> >-[test_localSubFolders.js]
> >+[test_nsIMsgLocalMailFolder.js]
> This needs to be an hg rename as well, no?

Yes, my .hgrc was missing git=1

Comment 7

[Magnus Melin [:mkmelin]]

Attachment: proposed fix v2 (for checkin)

Comment 8

[Magnus Melin [:mkmelin]]

http://hg.mozilla.org/comm-central/rev/77c812a9a820

->FIXED

Comment 9

[Jens Hatlak (:InvisibleSmiley)]

(In reply to Magnus Melin from comment #8)
> http://hg.mozilla.org/comm-central/rev/77c812a9a820
> 
> ->FIXED

Hmm, but the Status field still says ASSIGNED, so what is it? :-/

Comment 10

[Magnus Melin [:mkmelin]]

Fixed, thx!