Non MIME header support for Forward

VERIFIED FIXED in mozilla1.0

Status

MailNews Core
Internationalization
P4
major
VERIFIED FIXED
17 years ago
9 years ago

People

(Reporter: Tomoo Nomura, Assigned: nhottanscp)

Tracking

({intl, regression})

Trunk
mozilla1.0
intl, regression
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(11 attachments, 1 obsolete attachment)

923 bytes, patch
Details | Diff | Splinter Review
4.77 KB, patch
Details | Diff | Splinter Review
2.07 KB, application/octet-stream
Details
3.35 KB, patch
Details | Diff | Splinter Review
911 bytes, patch
Details | Diff | Splinter Review
1.03 KB, patch
nhottanscp
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
1.03 KB, patch
nhottanscp
: review+
Details | Diff | Splinter Review
743 bytes, patch
nhottanscp
: review+
(not reading, please use seth@sspitzer.org instead)
: superreview+
Details | Diff | Splinter Review
933 bytes, text/plain
Details
1.01 KB, text/plain
Details
1.75 KB, application/binary
Details
(Reporter)

Description

17 years ago
In case that the header of received message contains 8 bit characters and it is
not mime encoded, mozilla can display the header correctly.
However, replying to the message, the subject field and receipient field in the
new message can not be displayed correctly. These fields can't
 handle so called 8 bit through header. NS 4.x and IE5.x can do t

Comment 1

17 years ago
This is probably a duplicate of Bug 41009.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

17 years ago
If reproducible, this should be a dup of bug 39736.
IQA please test and reopen bug 39736 and mark this as a dup.

Keywords: intl

Comment 3

17 years ago
Reopened bug 39736.
Marked this one as a dup of 39736.

*** This bug has been marked as a duplicate of 39736 ***
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → DUPLICATE

Comment 4

17 years ago
As I mentioned in the other bug.
This is not a duplicate of Bug 39736
which is about body quoting. This bug is
about header quoting.

See my comments in Bug 41009 at:

Additional Comments From Katsuhiko Momoi 2000-08-10 17:26 

If this is not strictly a duplicate of Bug 41009,
then this bug should be kept open.

Comment 5

17 years ago
Hi Naoki, the problem described in bug 41009 is about the failure of "Edit as 
New" funtion for msgs w/o MIME headers. And this one is about reply/forward.  
Are they calling the same module?  If they do, we have three bugs: this one, 
41009, 39736, to point to the same problem, although we need to modify the 
summary of bug 41009 to better reflect the problem. 
(Assignee)

Comment 6

17 years ago
I am not sure if they use the same code. 
Looks like this bug is about headers not body so let's reopen this and keep as
separate.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(Assignee)

Updated

17 years ago
Target Milestone: --- → mozilla0.9
(Assignee)

Updated

17 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Comment 7

17 years ago
Created attachment 24705 [details] [diff] [review]
Patch, for reply, need a separate patch for forward
(Assignee)

Comment 8

17 years ago
Created attachment 24810 [details] [diff] [review]
Patch, for forward inline, changed to pass default charset, removed unnecessary conversion
just one thing: in nsMsgCompose.cpp, line 966, can you remove the comment that say that encodedCharset is not 

used. Appart that, R=ducarroz

(Assignee)

Comment 10

17 years ago
Okay, I will change it before check in.

Comment 11

17 years ago
sr=bienvenu
(Assignee)

Comment 12

17 years ago
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

17 years ago
I checked on Build 20010219. The recipient field seems to have been fixed, but
cc: field still has the problem, when choosing "reply all".

Comment 14

17 years ago
Tomoo, could you attach a testcase to the report? thanks.
(Reporter)

Comment 15

17 years ago
Created attachment 25725 [details]
unzip, move to mail folder, launch Mozilla, choose "check" folder, choose the message and "reply all"

Comment 16

17 years ago
Confirmed the problem. Reopened the bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 17

17 years ago
Reopen, I only fixed for subject header.
Status: REOPENED → ASSIGNED
OS: Windows 2000 → All
Priority: -- → P4
Hardware: PC → All
Target Milestone: mozilla0.9 → mozilla0.9.1
(Assignee)

Comment 18

17 years ago
*** Bug 73985 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 19

17 years ago
Need forward as attachment change, should wait for the MIME decoder change bug 
65277.
Depends on: 65277

Comment 20

17 years ago
adding nsbeta1 keyword, and jenm to cc: list.
Keywords: nsbeta1
(Assignee)

Comment 21

17 years ago
Created attachment 32675 [details] [diff] [review]
Patch, separated a code to get a top most window's charset as a function, so it can be called by both reply and forward.
R=ducarroz

Comment 23

17 years ago
yuck - well, sr=bienvenu
(Assignee)

Comment 24

17 years ago
*** Bug 80830 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 25

17 years ago
checked in for forward attachment.

Future the remaining problems, non subject header support Non MIME headers.
Target Milestone: mozilla0.9.1 → Future
(Reporter)

Comment 26

17 years ago
This problem in subject and sender field seemed to be fixed once at the
beginning of May, but it has reappeared while reply, forward and attachment
field's problems have been fixed. I checked on Build 20010525 20010527 20010531
20010604 and 2001060908.
(Assignee)

Updated

17 years ago
Summary: Can't produce 8 bit through header → Non MIME header support for Reply/Forward.

Updated

16 years ago
Blocks: 104166

Comment 27

16 years ago
Nominating for nsbeta1, this problem still exists on 01/17 build.
Forward-inline for non-MIME encoded mail headers doesn't inherit the specified
folder charset. But it can inherit the charset that the user specified from
charset menu.
Reply doesn't have this problem for headers. Modified the summary accordingly.
Keywords: nsbeta1
Summary: Non MIME header support for Reply/Forward. → Non MIME header support for Forward.

Comment 28

16 years ago
Please note that forward inline once got fixed in 6.2, but now it's broken.

Steps to reproduce:
1. Select a non-MIME encoded iso-2022-jp mail.
2. Set the proper folder charset to correct the view.
3. Click on Forward button, observe the corruption in subject. 
4. Select the iso-2022-jp charset from charset menu, click on Forward button,
you can see in this case forward inline can take the right charset specified
from menu.
Keywords: regression
(Assignee)

Comment 29

16 years ago
I think this is similar to bug 115869. I think this is related to my fix for
file name support (supporting filename charset different from the mail body's
charset).
Target Milestone: Future → mozilla0.9.9
(Assignee)

Comment 30

16 years ago
nsbeta1+ per i18n triage
Keywords: nsbeta1 → nsbeta1+
(Assignee)

Comment 31

16 years ago
This problem is different from the reply/quote (bug 118740).
For forward inline, it always fails to get a folder charset. This used to be a
non problem since we always set a override flag (so folder charset was not used).
(Assignee)

Comment 32

16 years ago
*** Bug 122465 has been marked as a duplicate of this bug. ***

Comment 33

16 years ago
> I think this is similar to bug 115869.

In some sense :-) 
The problem in bug 115869 is reverse on this one - In this bug
we don't check folder encoding, while in 115869 we check only folder encoding
and not msg window override charset

Comment 34

16 years ago
This bug consitts of two parts: first, we don't use default charset when
msg charset empty and there is no msg window charset override (mimedrft.cpp);

Second, we always failing to get folder charset when sending 
draft/template/fwd inline.
Looks like problems with CreateStartupUrl()
Problem in using two notations to reference message number in urls: ?number=<num>
and #<num>. nsMailboxUrl.ParseUrl expects msg number in form ?number=, but
sometimes gets it as #. This part is will be fixed by patch for bug 110689
(despite it says it's just code cleanup)

Comment 35

16 years ago
Created attachment 67246 [details] [diff] [review]
Patch, use default charset, if there no charset specified

Fix for first problem...

Updated

16 years ago
Depends on: 110689
(Assignee)

Comment 36

16 years ago
Thanks for finding the problem. The problem is fixed after I applied the patch
in this bug and the one in bug 110689).

About the patch, since the next line after your patch checks for mdd->options,
you can include your change inside that if clause.


(Assignee)

Updated

16 years ago
Blocks: 117956
(Assignee)

Updated

16 years ago
No longer blocks: 117956

Comment 37

16 years ago
Created attachment 67414 [details] [diff] [review]
v2 of the above patch

I also check charset_override now. Naoki, does it seems reasonable?
(Assignee)

Comment 38

16 years ago
Comment on attachment 67414 [details] [diff] [review]
v2 of the above patch

r=nhotta
Attachment #67414 - Flags: review+
(Assignee)

Comment 39

16 years ago
The comment,
-      // mscott: aren't we leaking a bunch of trings here like the charset
strings and such?

I think that is true, so please don't remove.

Comment 40

16 years ago
You can just use PR_Free instead of PR_FREEIF, because PR_Free will check for
null, and you don't need the assigning to null behaviour of PR_FREEIF because
you're assigning it in the next statement.

Comment 41

16 years ago
Naoki, what we're leaking here?
members of MimeDisplayOptions which may leak are:
mozITXTToHTMLConv   *conv      - not used in mimedrft.cpp at all
nsIPref             *pref      - freed in mime_stream_complete
char        *part_to_load      - freed in ~MimeDisplayOptions
char     *default_charset      - ditto

What else we may be leaking?

Comment 42

16 years ago
Created attachment 68862 [details] [diff] [review]
patch v2.01; replaced PR_FREEIF with PR_Free
(Assignee)

Comment 43

16 years ago
>What else we may be leaking?

How about MimeDisplayOptions::url?
(Assignee)

Comment 44

16 years ago
Comment on attachment 68862 [details] [diff] [review]
patch v2.01; replaced PR_FREEIF with PR_Free

r=nhotta
Attachment #68862 - Flags: review+
(Assignee)

Comment 45

16 years ago
David, could you review the latest patch?

Comment 46

16 years ago
Comment on attachment 67414 [details] [diff] [review]
v2 of the above patch

sr=bienvenu
Attachment #67414 - Flags: superreview+
(Assignee)

Comment 47

16 years ago
checked in the patch but need bug 110689 to get the folder charset
(Assignee)

Comment 48

16 years ago
depends on bug 110689
Target Milestone: mozilla0.9.9 → mozilla1.0

Updated

16 years ago
No longer depends on: 110689

Comment 49

16 years ago
Created attachment 72391 [details] [diff] [review]
folder charset part 

Here is rest of needed changes to fix this.

(no more depend on 110689)
(Assignee)

Comment 50

16 years ago
Comment on attachment 72391 [details] [diff] [review]
folder charset part 

r=nhotta

This is for forward inline. By the change, PrepareMessageUrl is called and that
makes a folder charset to be retrieved correctly later, and the reply quote
code also uses GetUrlForUri instead of CreateStartupUrl.
I cc to ducarroz in case he wants to review it.
Attachment #72391 - Flags: review+
Comment on attachment 72391 [details] [diff] [review]
folder charset part 

sr=sspitzer

please test forwarding for imap, news and local.
Attachment #72391 - Flags: superreview+

Comment 52

16 years ago
Naoki, does this patch take care of CC and BCC fields? If not, I'd like to open
a seperate bug.
Summary: Non MIME header support for Forward. → Non MIME header support for Forward
(Assignee)

Comment 53

16 years ago
You mean 'cc' in the quoted text in the body for forward inline?
(Assignee)

Comment 54

16 years ago
Created attachment 72815 [details] [diff] [review]
Updated for the recent URI change (SetSpec argument type change).
Attachment #72391 - Attachment is obsolete: true

Comment 55

16 years ago
yes, the cc part in the quoted text in the body for forward inline

How about Edit as new and edit draft? Does this patch take case of those cases too?

Comment 56

16 years ago
Created attachment 72819 [details]
Another testcase: a Ja SJIS mail with non-MIME encoded CC and BCC field.

Comment 57

16 years ago
Created attachment 72823 [details]
One more case: a Ja sjis mail with non-MIME encoded To/Cc/Bcc/Reply-to/Organization header.
(Assignee)

Comment 58

16 years ago
Comment on attachment 72815 [details] [diff] [review]
Updated for the recent URI change (SetSpec argument type change).

r=nhotta
Attachment #72815 - Flags: review+
(Assignee)

Comment 59

16 years ago
With the patch, the last two test cases quoted fine if I set the folder charset
to Shift_JIS. Not all the headers are quoted (e.g. bcc is not quoted), I think
that is by design.

Comment 60

16 years ago
Per discussion with Naoki, filed bug 129349 for the display problem with the
non-MIME encoded Reply-To and Bcc header when editing message as new.
Comment on attachment 72815 [details] [diff] [review]
Updated for the recent URI change (SetSpec argument type change).

sr=sspitzer
Attachment #72815 - Flags: superreview+

Comment 62

16 years ago
Comment on attachment 72815 [details] [diff] [review]
Updated for the recent URI change (SetSpec argument type change).

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #72815 - Flags: approval+
There seems to be at least one more issue (cf bug 129256 & bug 129349) not yet
under consideration: BCC header isn't MIME-encoded by Mozilla (as shown when we
save a message and "Edit Draft" again).  Maybe another bug has to be filed?
Oh, by the way, is there a bug already filed about Mozilla crashed by forwarded
mails?  I don't know how to reproduce the exact situation.  I think I'd better
make a test case for that if you think a new bug is needed.

nhotta: why is BCC unquoted by design?

Comment 65

16 years ago
Reply-To and BCC problems is bug #64092
(mozilla1.2, nsbeta-, patch, review).
(Assignee)

Comment 66

16 years ago
>nhotta: why is BCC unquoted by design?

I was wrong, you can change it to show all headers by View->Headers.
(Assignee)

Comment 67

16 years ago
checked in to the trunk

Thanks for the contribution.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago16 years ago
Resolution: --- → FIXED

Comment 68

16 years ago
Verified as fixed with 03/20 builds.
Status: RESOLVED → VERIFIED
(Reporter)

Comment 69

16 years ago
This problem has occured again since 2002032203 to 2002033109.

The problem occures when the mail has the header of
Content-Type: text/plain; charaset="iso-2022-jp" . 

When forwarding to this mail, Japanese characters can't display correctly.
In case of reply, there is no problem.

Tomoo

Comment 70

16 years ago
Tomoo, I don't see this on 03/28 build. Did you see the corruption in subject or
body?
(Reporter)

Comment 71

16 years ago
I saw the corruption in body.
(Reporter)

Comment 72

16 years ago
Created attachment 77111 [details]
Test Data

unzip, move to mail folder, launch Mozilla, choose check3 folder, choose
message and forward.

Comment 73

16 years ago
Tomoo, the mail you just attached is an ISO-2022-JP mail with MIME header,  and
it's also a reply mail with original one having no MIME header and it has added
some lines above the quoted mail.
It has two problems when doing forward on this mail:
1. The added lines is garbled
2. The quoted part in the mail is dropped when forwarding.

I think this is a different one with this bug since the mail you try to forward
has MIME header.
Could you file a seperate bug on this or do you want me to do it?
Please let me know. Thanks.
(Reporter)

Comment 74

16 years ago
#134762 is a new entry.
Please follow it up.

Tomoo
(Assignee)

Comment 75

15 years ago
Attached test case,
http://bugzilla.mozilla.org/attachment.cgi?id=25725&action=view
"To:" has problem when forward as inline. Other test cases do not have that
problem. I guess it is releated to the non-MIME encoded multiple line header.
Xianglan, could you try that using NS 7 and file as a separate bug?

Comment 76

15 years ago
Filed bug 180025 for above problem.
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.