This is separated from 5938. Charset override no effect on quoting (reply or forward inline). I'll attach a test data.

1) Select the second message, it shows garbage because it has a wrong charset label. 2) Set to "ISO-2022-JP" using View -> Character Coding menu. Now you can see Japanese. 3) Reply (or forward inline), the quoted text back to garbage. Put nsbeta2 to keyword.

QA Contct to momoi.

Putting on [nsbeta2+] radar for beta2 fix.

Adding estimated fix date.

Hmm I've been looking into this today and the fix looks really hard and complex. If I don't come up with something that looks good by Monday I'm going to start asking how important this is and would we hold nsbeta2 for it.

Can you use a charset stored in nsMsgWindow (the one used for a menu checkmark)?

that's the goal but there's no association between the message window and the compose window when it later decides to quote the message. And the way the message is quoted, there's no way to pass that charset information (even if we had a way to get it) down into mime. So I'll have to figure out away to pass the message window down through the call chain (which includes through a JS window.open call) and then I have to re-write how we do quoting.

I think I see a very cheap way to get this to work for beta2 if you only have one mail window open. If you have multiple 3-panes open or are viewing messages in stand alone message windows then it's possible we would get the wrong charset from the wrong menu when quoting. Not sure if that's a good enough solution for beta2. Still trying to find teh right way to solve this problem without incurring a high risk / high change cost.

Still working on this. I thought I had a solution that wouldn't have much impact but it didn't pan out.

Naoki, I noticed the compose window has a field called charset as part of the comp fields. I'm guessing this value is used as the charset header value in outgoing messages. When I fix this bug about using the override charset for quoting the message, should I also be stomping on this value in the headers?

Yes, and that value is used for a check mark of a compose window.

Here's a patch I think gets the job done for beta2. However I think we need to test the heck out of it 'cause while it's simple it has some far reaching I18N implications. With this patch, whenever we reply to a message, if the mail window has a value for GetMailCharacterSet then we force mime to use that character set when it quotes the message AND we set this value as the outgoig charset on the message. This means that you can thinking of quoting a message as using a over ride charset (so we ignore folder charset, or a charset in the message header) in favore of the value returned by nsIMsgWindow::GetMailCharacterSet. In theory GetMailCharacterSet should be reflecting the correct charset we used to display the message so we should be okay. But we should double check all those different scenarios. Naoki, can you code review this for me today? I'll check it in and that gives Kat a day (thursday) to really bang on this with an extra day for me to take care of any problems. Also, the reason this fix is nice and short is because I'm cheating and getting the message window for the top most mail window after the compose window has already come up. We then ask this mail window for the charset it used to view the currently displayed message. So you could in theory beat the system if you are fast enough and switch to a different mail window (3-pane or stand alone message window) before the Compose window comes up then we could grab the wrong charset.

Okay, I will review the code. I applied the patch and it works for reply but not for forward inline. For standalone window case, since override is not workinng for standalone window (bug 45059), quoting cannot be tested.

Here's a new patch that now works for reply and forward as attachment. It looks like forward inline might be going down a different code path for some reason? I need to investigate more.

I think we do not need to do anything for forward as attachment. We do not do charset reflection for forward attachment.

I reviewed the first patch (since we do not need forward as attachment) and it looks fine. I will review forward inline support when it's ready.

looks like forward inline doesn't go through mime at all. it just gets the DOM from the message pane and clones it in the editor window if I'm reading the code correctly. Do you know if that's right Rich? Naoki, in the forward inline case does the text look correct to you in the message body? Is it just the charset setting in the menu that is incorrect?

In forward inline, the text is not shown correctly. It shows the same garbage as shown in the original message before the override.

Naoki, can you review and double check this new patch. It contains changes to fix forward inline (keep the first patch I gave you in your tree and apply this one in mozilla\mailnews). The change to compose\src\nsMsgCreate.cpp forces the message being forwarded to be quoted in the charset specified by nsIMsgWindow->GetMailCharacterSet. The change to mime/src/mimedrft ensures that the compose window fields object gets the character set field set based on this same value. I'd like to check this in tonight if you get a chance to review and try it out. Thanks!

I see the same garbage with the patch for forward inline (reply works). The mimedrft change was executed but nsMsgCreate was not (did not hit a break point).

It was my mistake, nsMsgCreate change was actually executed (but override did not work).

Wow, this is interesting, according to the debugger, mime never attempts character conversion for open draft or in this case forward as inline. It skips right past all the code for charset conversion.

I think the problem is that libmime does not character conversion in the case of open draft / forward inline. It writes the message to disk without converting anything. Then it reads the contents back into memory and we convert it all at once to unicode saying the source charset is UTF-8: // convert from UTF-8 to UCS2 nsAutoString ucs2; if (NS_SUCCEEDED(nsMsgI18NConvertToUnicode("UTF-8", body, ucs2))) fields->SetBody(ucs2.GetUnicode()); Unfortunately, the content really isn't in UTF-8, it's in whatever content the message is naturally in because the mime draft / forward inline code path skipped the normal mime conversion code. A quick fix I tried which seems to work was to replace "UTF-8" with the charset we want to use (default or an override or one from the message header). I'm not sure if that's the right thing to do. Or if we should instead be making sure the mimedrft code path includes the character conversion routines that displaying a message and quoting a message do. What do you think Rich?

There is a code in mimedrft to convert the body from mail charset to UTF-8. I think the code is executed for forward inline, please check. 1032 // convert body from mail charset to UTF-8 1033 char *utf8 = NULL; 1034 nsAutoString ucs2; 1035 if (NS_SUCCEEDED(nsMsgI18NConvertToUnicode(mailcharset, *body, ucs2))) {

Right that was the code I was citing in the comment above yours. The problem is this is the only conversion that's going on. So the content really isn't UTF-8. It's in the charset of the message. I've changed it to be such and things appear to work. I don't know if that's the right thing though or if instead we should be making drafts / forward inline flow through the same conversion code as message display. Rich?

There are two conversions. First, the body is converted from mail charset to UTF-8. This UTF-8 string is combined with headers and localized resources (e.g. "From", "To" in UTF-8). Then the second conversion converts from UTF-8 to UCS2. You mentioned the second conversion and I mentioned the first one. They are different. I think if the override is used for the first conversion then it should work.

Right that's what I've been saying. In the case of draft or forward inline, the first conversion to UTF-8 never happens!! We aren't going through the same code that message display does. We never enter that code path. So instead we end up thinking the char * data is in UTF-8 when we convert to unicode in mimedrft.cpp when it really is in the native charset. =)

Okay, but without the first conversion, forward inline for Japanese does not work. I don't have a build available now but I will check tomorrow if that's working in the current build.

Okay here's the problem. you are right there is a conversion going on from the mail charset to UTF-8 then from UTF-8 to unicode. The problem is that mimedrft uses mdd->mailCharset to convert to UTF-8. This value is set by the ContentType header in mimedrft.cpp: charset = MimeHeaders_get_parameter(contentType, "charset", NULL, NULL); mdd->mailcharset = nsCRT::strdup(charset); So it's totally ignoring the over ride or default charset. That's why my initial patch for forward inline attachment didn't work. If you forward a message as inline that you used an over ride charset to view, the mimedrft code would ignore the override charset and always use the charset in the message header. This looks easy to fix. I'll try to fix it right now.

Okay here are the changes I'm going to check into mimedrft tonight that make the whole thing come together for the forward as inline case. When we set mdd->mailCharset in mimedrft, I added code to use the over ride charset if one is present, otherwise get it from the header. I've verified that forward inline now works for the following two cases: 1) Forwarding a japanese message with the correct charset (used to work b4 this fix too) 2) Forwarding a japanese message with the wrong charset (used charset over ride to display it correctly first). Both scenarios are now working for me. I'm going to check this in and then mark this bug fixed to get it off the radar. It'd be good if some other folks could bang on this tomorrow.

Marking fixed.

uggh i'm slowing down. marking fixed for real.

Just to confirm, since currently override does not work for headers, this fix only applies to body quoting.

Unfortunately my change to mimedrft.cpp broke the Mac so they backed me out this morning. *sigh* my bad. So this won't be working for forward inline in todays builds.

But both reply and forward inline are working with today's build (win32 ID 20002008). Override works for Quoted Japanese text, it's showing fine for both reply and forward inline.

** Checked with 7/21/2000 Win32 build ** There is a problem that has not been fixed. It relates to this exchange by mscott and nhotta: >> When I fix this bug about using the override charset for quoting >> the message, should I also be stomping on this value in the headers? > Yes, and that value is used for a check mark of a compose window. The remaining problem is that we are not reflecting the the corrected charset on the Character Coding menu "checkmark". Take aan exmple: 1. A message is incorrectly marked as ISO-8859-1 even though it is really an ISO-2022-JP message. 2. You can't see the text vody well and so you correct it by overriding the message charset from ISO-8859-1 to ISO-2022-JP by choosing the Character Coding menu item. 3. Now engage the reply button. The text gets quoted correctly. 4. But when you look in the Character Coding menu of the Composer window, you see that it is relfecting the incorrect original mesg. 5. This leads to corrupt msgs being sent. Re-opening for this remaining problem...

Naoki, in the reply case my code for setting the charset on the compose window is getting over-written by some of your code in nsMsgQuoteListener. There is a big routine in that method that attempts to extract the charset from the content type of the message we are quoting. It then does some string atom and other magic that I don't quite groke before setting a new charset on the compose window. This code ends up guessing the wrong content type so we end up getting the original content type set on the charset menu. Now that I've added code to set it earlier in the process, I think this routine isn't necessary anymore. I"m going to try removing it tonight and I'll check that in if things look good.

Removing that block of code in nsMsgQuoteStreamListener::OnStopRequest fixes the problem for me. I'm attaching a patch showing the code I removed.

I just checked in the changes I mentioned above. This is now working for me again.

** Checked with 8/2/2000 Win32 M17 build ** The problem cases involved messages which contained incorrect MIME charset headers. With the above build, I can correct incorrect display of body and then reply or forward/inline the corrected message and then am able to see the data inserted correctly into the quoted portion. Marking the fix verified as fixed.

Checked 2001-01-19-06-mtrunk win32 build. The charset override doesn't apply to the un-MIME's header on the reply/forward mail. The charset override for quoted body is working.

Changed QA contact to myself, and changed the summary according to the current behavior.

*** Bug 66098 has been marked as a duplicate of this bug. ***

I believe that this bug was about quoting the body text, not about the non-MIME'ed header. We should not tack on a new problem into this bug.

Look at the test cases attached. It does not contain any 8-bit characters for the subject header. The test case which was verified as fixed contained body text data which were 1) correctly labeled, 2) incorrectly labeled and 3) not labeled.

And the body data was in iso-2022-jp.

Closed this one, since the problem described originally is fixed. We have bug 66098 for the header problem and bug 41009 for the problem in "Edit as New".

Marked it as verified.

Charset override for quoting body is not working for me. I am using win32 RTM, I will attach an example.

Checked 2001-01-19-06-mtrunk win32 build with the testcase that Naoki attached. I noticed some interesting things going on here. Even Naoki's testcase has right MIME charset info, if you select View | Character Encoding, the checkmark is not marked before ISO-2022-JP. So when you do reply, the quoted body is garbled on the reply compose window. But if you correct the charset mark first by going to the 2nd msg in the smoketest folder and coming back to Naoki's testcase, the checkmark is correctly marked before ISO-2022-JP, and if you do reply after that, the quoted body will be displayed alright. It looks like the charset mark for a MIME'd mail is broken.

Okay, then it's a separate problem, could you file a new bug for the charset mark problem?

Filed bug 66227 for the checkmark problem.

In 2001-04-02 build the charset override is broken again for reply or forward inline, so reopening

Let me investigate this and I may return this to mail eng.

nsIMsgQuote::QuoteMessage takes charset but original header's charset is always used. I think we can get the override charset because we do this for the subject header already.

*** Bug 74403 has been marked as a duplicate of this bug. ***

nsIMsgQuote::QuoteMessage is actually using nsMsgCompFields charset which is set properly for quoting. There is a place in libmime which parses the original charset and overwrites the override charset value. It has to change to do that only for non-override case.

it is possible to replace NULL by nsnull? R=ducarroz

You mean all instances in the file? "NULL" are mostly used in that file (I guess because it's inherited from 4.x).

replace the NSNULLs you added with nsnull and then sr=bievnenu

checked in

Verified with 05/07 trunk builds. It's fixed.