Port bug 1193762 to Mailnews (bustage fix)

RESOLVED FIXED in Thunderbird 49.0

Status

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jorgk, Assigned: frg)

Tracking

Trunk
Thunderbird 49.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Now that bug 1193762 has landed, we have fresh bustage, for example:

c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/db/msgdb/src/nsMsgDatabase.cpp(4765): error C2662: 'nsCOMPtr<nsIMsgThread>::operator T(void) const &&': cannot convert 'this' pointer from 'nsCOMPtr<nsIMsgThread>' to 'const nsCOMPtr<nsIMsgThread> &&'

/builds/slave/tb-c-cen-m64-ntly-000000000000/build/mailnews/db/msgdb/src/nsMsgDatabase.cpp:4764:17: error: conversion function from 'nsCOMPtr<nsIMsgThread>' to 'nsIMsgThread *'
It is not yet visible on trunk/try. How many occurrences are there?
And what is the pattern to convert there cases?
Posted patch 1269316-WIP.patch (obsolete) — Splinter Review
Just tried to compile suite and stumbled over this. 

Fix for first three errors. Got the next one in ldap. 

My c++ knowledge is limited. Fix might be rubbish.
(In reply to Frank-Rainer Grahl from comment #2)
> Created attachment 8747670 [details] [diff] [review]
> 1269316-WIP.patch
> 
> Just tried to compile suite and stumbled over this. 
> 
> Fix for first three errors. Got the next one in ldap. 
> 
> My c++ knowledge is limited. Fix might be rubbish.

I'm sure you could f?aryeh if you have a patch.
Blocks: 1193762
Posted patch 1269316-c-c-nscomptr.patch (obsolete) — Splinter Review
ldap blagg comparing an object to 0... Hope the is the right approach.

Seamonkey including lightning compiles and runs with this one. Only a quick test. Any other issues in TB or I could take the bug and ask for review?
Attachment #8747670 - Attachment is obsolete: true
Thanks for taking this on so quickly. I'm not at my desk right now, so I can't try it in TB. I should be back in two hours to try it if no one else has done it by then.

Otherwise I'd f?:Aryeh Gregor.
Comment on attachment 8747683 [details] [diff] [review]
1269316-c-c-nscomptr.patch

Looks good to me, I've kicked off a compile now.

Aryeh, do you have three minutes to look this over? All straight forward.
Attachment #8747683 - Flags: feedback?(ayg)
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Well, there is at least one more:
 4:13.91 c:/mozilla-source/comm-central/mailnews/imap/src/nsImapMailFolder.cpp(2237): error C2666: 'operator !=': 3 over
loads have similar conversions

nsCOMPtr<nsIMsgFolder> trashFolder;
...
NS_ASSERTION(trashFolder != 0, "couldn't find trash");
Hmm suite must not use this one. 

Couldn't this just be 

NS_ASSERTION(trashFolder, "couldn't find trash");

?

Seems to be a simple null pointer check but there we are with my c++ non knowledge :)

FRG
(In reply to Frank-Rainer Grahl from comment #8)
> Couldn't this just be 
> NS_ASSERTION(trashFolder, "couldn't find trash");
Yep. I'll update the patch in a minute.
Another one ;-)
 1:17.69 c:/mozilla-source/comm-central/calendar/base/backend/libical/calDateTime.cpp(477): error C2280: 'nsCOMPtr<calITimezone>::operator T(void) const &&': attempting to reference a deleted function
Didn't do a full build. Looks like I should have. Not at the desktop right now. Should be similar to one of the first ones i the patch.
Sure. I found more. I'll keep going and attach the patch when I'm done.
One review will do. All straight forward.
Attachment #8747683 - Attachment is obsolete: true
Attachment #8747683 - Flags: feedback?(ayg)
Attachment #8747853 - Flags: review?(rkent)
Attachment #8747853 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8747853 [details] [diff] [review]
FRG's work plus a few more hunks he missed

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

LGTM, r=mkmelin

::: ldap/xpcom/src/nsLDAPOperation.cpp
@@ +519,5 @@
>  {
>      nsresult rv;
>      nsresult retStatus = NS_OK;
>  
> +    if (!mMessageListener || mMsgID == 0 ) {

mind removing the space before ) too?
Attachment #8747853 - Flags: review?(rkent)
Attachment #8747853 - Flags: review?(mkmelin+mozilla)
Attachment #8747853 - Flags: review+
Sure. I'll wait until Kent has finished building with it, then I push.
https://hg.mozilla.org/comm-central/rev/66dea6d980f6
(Kent's build never worked, but Richard confirmed a working build.)
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 49.0
Thanks a lot to Frank-Rainer Grahl! I've just added a few hunks to his patch and it got landed in his name.
(In reply to Jorg K (GMT+2) from comment #17)
> Thanks a lot to Frank-Rainer Grahl! I've just added a few hunks to his patch
> and it got landed in his name.
And when Thunderbird emails your credit card information to the Russian Mafia, everyone will blame frg.
>> And when Thunderbird emails your credit card information to the Russian Mafia, everyone will blame frg.

Nahh. I am good friends with them. They wouldn't do this to me.

FRG (now back to serious business)
You need to log in before you can comment on or make changes to this bug.