44635 - [Regression] Charset Reflection not working for Reply/Forward-inline

Status: VERIFIED FIXED

(MailNews Core :: Internationalization, defect, P3)

Description

[Katsuhiko Momoi]

** Observed with 7/5/2000 Win32 build **

In reply and forward/inline, we are supposed to reflect the 
original charset of the main body part, but with the above build,
this is no longer true. It was working at one time but apparently
this regressed.
This is a must function for Beta2. Since without it, mail content
will be messed up, e.g. sending iso-8859-1 when you really mean to
send a Japanese message, i.e. iso-2022-jp. Most users expect this
function to work like this.

Comment 1

[Katsuhiko Momoi]

I traced back the builds and it seems that this function
broke between 6/21 and 6/22.

Comment 2

[Katsuhiko Momoi]

Additional information:

1. For Reply, I see the charset = default send charset no matter
   what the charset of the original message is.
2. For Forward/inline, I don't see any charset indicated 
   in the menu.

Comment 3

[Katsuhiko Momoi]

I need to correct the date when this problem started. It is
6/23. On the build the day before, the charset reflection is
working OK.

Comment 4

[nhottanscp]

nsMsgCompose gets the original charset by parsing a string returned by 
GetContentType(). The string used to contain "charset" if that's specified in 
the message. But now I don't see it in the debugger (only "text/plain" or 
"text/html" is returned, no "charset=xx").
Adding mscott to cc. Scott, did you recently change this area for open 
attachment?

Comment 5

[Scott MacGregor]

Yes i did change the implementation of nsStreamConverter::GetContentType.
Why are we storing a charset as part of the content type string? That seems
kinda strange to me?

Comment 6

[nhottanscp]

Because a charset is specified in that way in the original message. Charset can 
be specified in the content type string, see rfc 2045.

Comment 7

[nhottanscp]

Reassign to mscott.

Comment 8

[Jay Patel [:jay]]

Putting on [nsbeta2+] radar for beta2 fix. 

Comment 9

[Scott MacGregor]

Adding estimated fix date.

Comment 10

[scottputterman]

marking nsbeta2- as decided by mail triage.

Comment 11

[Katsuhiko Momoi]

Scott, this is a regression from Beta 1. The EST also used to
say 7/21. Today is only 7/12. I don't think we can let this bug
go as this has such a big impact on usability.

Comment 12

[Katsuhiko Momoi]

Requesting reconsideration for nsbate2. This feature was working since
before Beta 1 until it was broken by a recent check-in.

Without this feature working, we have been seeing users reporting
receving unreadable messages. All the data in the text body
look good but the mail is in fact sent in wrong encoding and
will become unreadable. Average users should not have to 
tinker with Character Coding menu in replying messages. We 
take care of it for them with this feature. 

This is very bad. There is a simple way to avoid this problem --
that is restoring this to PR1 status and have the automatic
charset reflection of the original message working. This is a
major feature that the International Mail team is *extremely* proud of 
and should not have been tiraged without ii18n participation.

Comment 13

[Scott MacGregor]

Kat don't worry. Scott marked this bug nsbeta- by accident. He and I 
had a miscommunication about another I18N bug that we were going to 
minus (I already did it yesterday with I18N and PDT approval). Restoring the 
nsbeta2+. 

Comment 14

[Scott MacGregor]

Naoki, do I need a special kind of message to see the =charset as part of the
content type? I have an I18N folder of test messages including messages kat
wrote in japanese. If I try to reply to one of these messages, I don't see
anyone trying to set a content type on the stream converter with a =charset
value in it. And the message appears to be quoted correctly. So I'm guessing I'm
not using the right type of message.  Can you send me a msg I can put in a
folder and try?

Comment 15

[nhottanscp]

Quote is working correctly (so you can see Japanese correctly), what's broken is 
a charset of the quoted message. That charset is used to label an outgoing 
message.
The Japanese message in the smoke test data is a multipart message and does not 
have charset specified in the main body. The first message "Santé Nouveau!" has 
a charset specified as "ISO-8859-1". So you can use this for the test case.
For a multipart message, I see a code to do the special case in mememult.cpp, 
MimeMultipart_parse_line. It calls mimeEmitterUpdateCharacterSet to put a part 
charset into the Content-Type.

Comment 16

[Scott MacGregor]

mimeEmitterUpdateCharacterSet is the piece of information that I was missing. 
Looks like this routine is now setting the content type on the wrong channel.

should have a patch shortly.

Comment 17

[Scott MacGregor]

Attachment: the fix for the reply case

Comment 18

[Scott MacGregor]

compose was passing in a bogus channel into the stream converter. So mime was
setting the content type on this bogus channel instead of the real channel that
compose was later asking for the character set.

This fix gets rid of the bogus channel and passes in the channel we are going to
actually use for extracting the data.

I haven't tested the forward case yet. It might fix that too. I'm looking at
that next.

Comment 19

[Scott MacGregor]

I'll check this in as soon as the tree opens today. It now works for me for both
reply and forward with this patch.

Comment 20

[Scott MacGregor]

fix checked in. 

Comment 21

[ji]

Verified with 2000-07-28-04-M17 linux and 2000-07-28-12-M17 win32 builds.
This is fixed.

Comment 22

[ji]

This is not working for multipart/alternative mail. (Bug 46881)