Closed
Bug 647699
Opened 14 years ago
Closed 13 years ago
Make ::addMessage return an nsIMsgDBHdr on successful add.
Categories
(MailNews Core :: Database, defect)
MailNews Core
Database
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 13.0
People
(Reporter: alta88, Assigned: mkmelin)
References
Details
Attachments
(1 file, 2 obsolete files)
|
(deleted),
patch
|
Details | Diff | Splinter Review |
|
Assignee | |
Comment 1•13 years ago
|
||
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #594754 -
Flags: superreview?(dbienvenu)
Attachment #594754 -
Flags: review?(dbienvenu)
|
||
Comment 2•13 years ago
|
||
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.
Attachment #594754 -
Flags: superreview?(dbienvenu) → superreview?(neil)
|
|
||
Comment 3•13 years ago
|
||
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•13 years ago
|
||
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 :-))
Attachment #594754 -
Attachment is obsolete: true
Attachment #594754 -
Flags: superreview?(neil)
Attachment #594754 -
Flags: review?(dbienvenu)
Attachment #597231 -
Flags: superreview?(neil)
Attachment #597231 -
Flags: review+
|
||
Comment 5•13 years ago
|
||
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?
Attachment #597231 -
Flags: superreview?(neil) → superreview+
|
|
Assignee | |
Comment 6•13 years ago
|
||
(In reply to neil@parkwaycc.co.uk 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
|
|
Assignee | |
Comment 7•13 years ago
|
||
Attachment #597231 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 8•13 years ago
|
||
Flags: in-testsuite+
Target Milestone: --- → Thunderbird 13.0
|
||
Comment 9•13 years ago
|
||
(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? :-/
|
|
Assignee | |
Comment 10•13 years ago
|
||
Fixed, thx!
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•