Steps to create: Create an address book entry with the name having special characters like δλοφό. Compose a message and specify this person as the To, CC, and BCC. Save the message to the Drafts folder. Open the message in the Drafts folder. Print it. Notice that To and CC display δλοφό but BCC displays gibberish.

I can reproduce this. Reassign to ducarroz.

The problem is not limited to printing. You can see the problem in Mail View source. BCC line does not go through MIME-encoding process while the visible lines like To and CC do.

QA contact to marina.

should be simple to fix. Looks like we miss something for BCC in the code

moving to 0.9.7

I suspect this something is in nsMsgComposeAndSend::MimeDoFCC(). I have a working patch, but there's one thing I don't understand: I don't have mail.strictly_mime_headers pref set, so when I use nsMsgMIMEGetConformToStandard() for BCC field it's not get encoded. At the same time, other headers (To:, Cc:) gets encoded even if nsMsgMIMEGetConformToStandard() returns false. Jean-Francois, can you shed some light on this?

Encode BCC field before writing it out. I'm still confused by nsMsgMIMEGetConformToStandard() though, need Jean-Francois's advice...

Comment on attachment 61579 [details] [diff] [review] patch, v1 the code: + if (convBcc) + { + bcc_header = convBcc; + PR_Free(convBcc); + } sound bogus, should be: + if (convBcc) + { + PR_Free(bcc_header); + bcc_header = convBcc; + }

nsMsgMIMESetConformToStandard confuse me too, looks like we have to way to encode headers!

Damn it, I cannot believe I made SO stupid patch :-((( Here is another one. I'm not sure that we can change bcc_header, cause it points to mCompFields's member variable...

Following your mistake in the first patch (I haven't reviewed the second one), I am concerned that you are submitting a patch without testing it! How have you tested the second one?

The second patch doesn't work either because the lenght L is not correctly calculated in case you convert bcc. I have a fix for that but I am still not able to correctly edit the draft! looks like we have a problem in mime when we read the bcc field...

Yes, second patch is not working, sorry. I'm trying to make proper patch...

Here is proper patch. As for editing draft problem, this is wierd... value of Bcc: field is correctly set in nsMsgCompFields when loading draft (it's in utf8, as well as To: and other headers). There must be place, where bcc: treated differently than to:, but I cannot find it yet

the problem with bcc when editing the draft comes when we setup the bcc field in nsMsgCompose::CreateMessage. Looks like we are missing a conversion!

Comment on attachment 62944 [details] [diff] [review] patch, v3 (part 1) R=ducarroz

I removed unnecessary and bogus conversion between UTF8 and Unicode when adding extra reply-to and bcc recipients.

I still don't understand, why it's been working for replyToStr, but not for bccStr. The code looks identical to me. What I'm missing?

It's because we get the reply to from the identity wich use ASCII while we get the BCC from the compose field (in that particular case) which is in UTF8. At least that my explanation!

Comment on attachment 62948 [details] [diff] [review] patch, v3 (part 2) this fix breaks reply-to headers using multi-byte language like japanese! Don't know about bcc.

This (DecodeMimeHeader on nsMsgCompFields data) works for me (tested with koi8-r encoding, which is 8-bit). May be it's possible to decrease number of string conversion here, but I'm not expert in that field :-(. Also, I'm not sure if this is good or bad to have mime encoded headers in nsMsgCompFields - probably we should decode headers before setting them in comp fields?

This (DecodeMimeHeader on nsMsgCompFields data) works for me (tested with koi8-r encoding, which is 8-bit). May be it's possible to decrease number of string conversion here, but I'm not expert in that field :-(. Also, I'm not sure if this is good or bad to have mime encoded headers in nsMsgCompFields - probably we should decode headers before setting them in comp fields?

compose fields should not contains mime encoded string!

it turned out that comp fields wasn't mime encoded. Here is what I think is a correct patch. I think the reason of the bug is mismatched GetXXX/SetXXX calls - getter was used in ascii version (no charset conversion) and setter was used in unicode version (with charset conversion). I wonder if it's possible to avoid all that multiple string conversions (i.e. assign nsXPIDLCString to nsXPIDLString - that woodoo string magic doc is really incomplete :-( ) I also made it that we call Get/Set stuff only when really needed... (Patch tested with koi8-r encoding)

It's also possible to use opposite pair of Get/Set method: just replace m_compFields->SetBcc(bccStr.get()); with nsXPIDLCString tmp; tmp.Adopt(ToNewCString(bccStr)); m_compFields->SetBcc(tmp); which may be faster than v3.2 version

Wow, I found how to deal with reply-to and bcc fields using only nsXPIDLCString, so I get rid of ascii<->unicode conversions! Ready for review, tested with koi8-r mail

adding nsbeta1- and moving out. But since it looks like we have a patch, when it's ready and reviewed, feel free to move it back.

I'll review once I get some spare time...

The patch will fix nsbeta1+ bug 129256. The reply part looks fine. I will review the bcc part next week.

Comment on attachment 63948 [details] [diff] [review] patch v4 - the whole thing for review r=nhotta

two small nits, then sr=sspitzer 1) + PRBool aBool1, aBool2; two problems here. first, aFoo notation is for function / method arguments. these are local variables. second, these names contain no information. instead of aBool1 and aBool2, do PRBool bccSelf, bccOthers; 2) + PRInt32 L = PL_strlen(convBcc ? convBcc : bcc_header) + 20; use strlen() instead

Comment on attachment 73703 [details] [diff] [review] Changed variable names and use strlen instead of PL_strlen. sr=sspitzer

Comment on attachment 73703 [details] [diff] [review] Changed variable names and use strlen instead of PL_strlen. r=nhotta

Comment on attachment 73703 [details] [diff] [review] Changed variable names and use strlen instead of PL_strlen. a=asa (on behalf of drivers) for checkin to the 1.0 trunk

checked in to the trunk Thanks Denis.

verified with 2002-04-16 trunk build, the source and the printed page show the non-ascii chars in the Bcc field correctly