No quoted text in reply when the original message has invalid characters

VERIFIED FIXED in mozilla1.1beta

Status

VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: jmdesp, Assigned: nhottanscp)

Tracking

(Depends on: 1 bug, {intl, regression})

Trunk
mozilla1.1beta
x86
Windows NT
intl, regression
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt2 RTM])

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

17 years ago
Checked with :
Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:0.9.9+) Gecko/20020313

Has been there since a few builds, is a regression.

Step for reproduction :
- open a message that has some characters that are invalid in the charset that
is defined inside it's Content-TYpe header
- hit reply
- the composition window only has "xxx wrote:" and the rest is blank.

This might be a duplicate of bug 110754, but as the conditions are slightly
different (invalid character for the defined Content-Type here, no Content-Type
for bug 110754), I report it seperately.
(Reporter)

Comment 1

17 years ago
Created attachment 75411 [details]
sample news message for reproduction
(Reporter)

Comment 2

17 years ago
Sorry I meant this could a duplicate of bug 127628.

bug 110754 is a different problem.

I add a reproduction of the problem in an email message, instead of a news message.
This regression must be there since at least a month, but it used to work
correctly before for example with Netscape 6.2.
(Reporter)

Comment 3

17 years ago
Created attachment 75413 [details]
sample mail message for reproduction
(Reporter)

Comment 4

17 years ago
Created attachment 75416 [details]
Another sample with a valid utf-8 and an invalid utf-8.

I hope this attachment will display as utf-8 with an invalid character without
having to force the display of the browser to utf-8 as was the case with
attachment 75413 [details]. But either can be used for reproduction in mail.

Obsoletes 75413 but I don't have the rights for that.
(Reporter)

Comment 5

17 years ago
Just like in bug 127628 , forward as inline will include the whole content of
the quoted text in the reply without error.

The web server still gives a default charset of ISO-8859-1 to the page with
attachment 75416 [details] , but at least one can see from the start the content is not
ISO-8859-1, so it's better than attachment 75413 [details].

Comment 6

17 years ago
Confirmed the bug. I can reproduce with 03/20 win32 build.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: intl
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.2alpha
(Assignee)

Updated

17 years ago
Depends on: 140360

Comment 7

17 years ago
I've just experienced this too.
(Reporter)

Comment 8

16 years ago
I entered this bug, and forgot to check on it later. I now regret that.
I figured it would be solved rapidly.
This used to work and there is no reason why it should involve much change in
the code to solve the problem.

This problem is a bad regression that should not be there in Mozilla 1.0.
Well, it will be as it's late now for that, but a correction should be planned
for 1.0.x and not a future release.

I know some people that will upgrade from Netscape 6.2 to Netscape 7.0, and they
will encounter this as a regression from 6.2 to 7.0.

bug 127628 has lead me to test and verify that this problem will also impact a
chinese user with a default character coding set to EUC-TW that receive an
ISO-8859-1 mail with some accentuated characters and no Content-Type header. 

He will not to be able to include the content of the original mail in his reply
except if he manually forces it to ISO-8859-1 first.
If the mail is a long message in english with only one or two accentuated
characters, this will not be easy for him to discover.

I couldn't reproduce when the default coding set is set to EUC-JP, despite the
fact the character I used are as illegal in EUC-JP as they are in EUC-TW. With
the default character coding set to ISO-2022-JP, I reproduced that problem, but
this is not the standard setting for japanese users.
(Reporter)

Comment 9

16 years ago
Created attachment 86400 [details]
Sample ISO-8859-1 without Content--Type header that will cause problems

Problem reproduction procedure :
- in Preference/Mail&Newsgroups/Message Display, set default character coding
to Chinese Traditonnal (EUC-TW)
- Open the attachment mail 
- Click on the reply button. 
Nothing from the initial message will be quoted in the	reply.
(Assignee)

Comment 10

16 years ago
nsbeta1

I think we want to fix this for rtm because it is not easy for the user to
realize about the charset problem when encountering the blank reply.

For the case of the latest attachment, the message cannot be shown correctly
without explicitly setting to ISO-8859-1 by the menu. But it is possible that
the user may not notice that when the messsage is long and contains not many non
ASCII characters.
(Assignee)

Updated

16 years ago
Keywords: nsbeta1

Comment 11

16 years ago
data lost in reply. Sounds like a very common case for mail users. 
Blocks: 141008
Keywords: nsbeta1 → nsbeta1+
Whiteboard: [adt2]
(Assignee)

Comment 12

16 years ago
Created attachment 86532 [details] [diff] [review]
Added error handling for the Unicode conversion.

Updated

16 years ago
Attachment #86532 - Flags: review+
Comment on attachment 86532 [details] [diff] [review]
Added error handling for the Unicode conversion.

Looks good. R=ducarroz.

Just one comment, instead of:
+	   unichars[unicharLength++] = (PRUnichar)0xFFFD;
+	   unichars += unicharLength;

I would have done:

+	   unichars += unicharLength;
+
	  *unichars = (PRUnichar)0xFFFD;
+	  unichars ++;

That would avoid the processor to calculate twice the offset.
(Reporter)

Comment 14

16 years ago
Wrong code, you need to do this instead to have the same result.

+
   unichars += unicharLength++;
+
	  *unichars = (PRUnichar)0xFFFD;
+
  unichars ++;

I'm not sure it's really better after optimisation for a heavily pipelined
processor.
but 
+
   unichars += unicharLength;
+
	  *unichars = (PRUnichar)0xFFFD;
+
  unichars ++;
+         unicharLength ++;

is probably the easiest version to understand.
(Assignee)

Comment 15

16 years ago
Created attachment 86728 [details] [diff] [review]
Changed per reviewer's comment.

Also changed to use '?' instead of 0xFFFD since that character cannot be mapped
by the outgoing mail charset except for UTF-8.
Attachment #86532 - Attachment is obsolete: true
Comment on attachment 86728 [details] [diff] [review]
Changed per reviewer's comment.

R=ducarroz
Attachment #86728 - Flags: review+

Comment 17

16 years ago
Comment on attachment 86728 [details] [diff] [review]
Changed per reviewer's comment.

sr=bienvenu
Attachment #86728 - Flags: superreview+

Comment 18

16 years ago
QA contact to Marina. Thanks.
QA Contact: ji → marina
(Assignee)

Comment 19

16 years ago
checked in to the trunk
Target Milestone: mozilla1.2alpha → mozilla1.1beta
(Assignee)

Comment 20

16 years ago
checked in to the trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Updated

16 years ago
Keywords: adt1.0.1, mozilla1.0.1

Comment 21

16 years ago
marina - can you pls verify this on the trunk? thanks!
Blocks: 143047
Keywords: approval
Whiteboard: [adt2] → [adt2 RTM]

Comment 22

16 years ago
can IQA verify this by this week?
(Reporter)

Comment 23

16 years ago
Verified/fixed in 
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1a) Gecko/20020613

I hope this goes in 1.0.1

BTW I love the new display of invalid charaters as a small question mark inside
a black diamond, and no more simply as a question mark when reading. Great job.
It's just simply unfortunates it has to turn to '?' in the text of the reply.
Status: RESOLVED → VERIFIED

Updated

16 years ago
Keywords: regression

Comment 24

16 years ago
adding adt1.0.1+.  Please get drivers approval before checking in.
Keywords: adt1.0.1 → adt1.0.1+
Comment on attachment 86728 [details] [diff] [review]
Changed per reviewer's comment.

Please land this on the 1.0.1 branch.  Once there, remove the
"mozilla1.0.1+" keyword, and add the "fixed1.0.1"
Attachment #86728 - Flags: approval+
Keywords: mozilla1.0.1 → mozilla1.0.1+
(Assignee)

Comment 26

16 years ago
checked in to 1.0.1 branch
Keywords: fixed1.0.1

Updated

16 years ago
Blocks: 146292
No longer blocks: 141008

Updated

16 years ago
Keywords: mozilla1.0.1+

Comment 27

16 years ago
verified on the branch, there is no data loss any more, the reply text is quoted
(used the email Jean-Marc provided)
Keywords: fixed1.0.1 → verified1.0.1
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.