235584 - vCards no longer being attached to outgoing mail

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Scott MacGregor]

summary says it all

Comment 1

[Scott MacGregor]

I wonder if this might be a subtle string regression. The calling code looks
like this:

nsXPIDLCString escapedVCard;
// make sure, if there is no card, this returns an empty string, or failure
if (NS_SUCCEEDED(identity->GetEscapedVCard(getter_Copies(escapedVCard))) &&
!escapedVCard.IsEmpty()) 
{
  do stuff
}

What I'm now seeing after the string changes is that GetScapedVCard IS called
first, we return from that (I even see us return from getter_Copies) then we
enter escapedVCard.IsEmpty but the xpidlstring still doesn't show the vCard
string internally so isEmpty turns out to be true. As soon as the if statement
loses scope, the idl string suddenly shows the vCard string internally.

I'm sure the fix is an easy break down of the if statement but I thought it
interesting to point out a possible run time difference in the new string class
in case others have issues to..

Comment 2

[Scott MacGregor]

Attachment: work around

this work around breaks the old if statement into two lines...

Comment 3

[Scott MacGregor]

Comment on attachment 142250 [details] [diff] [review]
work around 

darin, I'm worried that the fact that this now fails with the new string APIs
could also be breaking lots of other callers that were using the old pattern
here that we don't know about yet. 

Hence asking you to r= the workaround so you can determine if you think
something needs to change in the new string classes to make this case work like
it used to.

Comment 4

[Darin Fisher]

Comment on attachment 142250 [details] [diff] [review]
work around 

r=darin

this is actually the same problem that was uncovered in bug 234908.

the correct fix is what you have done here.  see bug 234908 for more details.

Comment 5

[Scott MacGregor]

thanks for the clarification Darin! Fix checked in.

It may be worthwhile for me to try to search through all the getter_Copies code
in mailnews to try to catch any more patterns like this....