Closed Bug 1444371 Opened 3 years ago Closed 3 years ago

mail.label_ascii_only_mail_as_us_ascii does not work with ISO-2022-JP


(MailNews Core :: Composition, defect)

Not set


(Not tracked)

Thunderbird 60.0


(Reporter: rokuyama, Assigned: rokuyama)



(Keywords: regression, Whiteboard: [regression:TB45])


(1 file, 1 obsolete file)

Attached patch thunderbird.patch (obsolete) — Splinter Review
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20180206200532

Steps to reproduce:

Set mail.label_ascii_only_mail_as_us_ascii to true, and outgoing encoding to
ISO-2022-JP. Then, send a mail which does not contain any non-ASCII
characters in its message body.

Actual results:

The message is sent with charset=iso-2022-jp in Content-Type header.

Expected results:

It should be charset=us-ascii, and it was before 2015. This bug seems to be
introduced by following sequence of commits:

After these commits, any messages are determined as "not ASCII only" with
stateful outgoing encoding (i.e., ISO-2022-JP).

If I understand correctly, non-ASCII characters can more easily determined with
UTF-8, rather than with outgoing encoding. The attached patch works fine at
least for me. It is for 52.6.0, but it can be applied to the master branch.
So that is Bug 1202401 - Prepare for m-c removal of nsISaveAsCharset
Component: Message Compose Window → Composition
Keywords: regression
Product: Thunderbird → MailNews Core
Summary: [patch] mail.label_ascii_only_mail_as_us_ascii does not work with ISO-2022-JP → mail.label_ascii_only_mail_as_us_ascii does not work with ISO-2022-JP
Version: 52 Branch → 52
Attachment #8957527 - Attachment is patch: true
Attachment #8957527 - Attachment mime type: text/x-patch → text/plain
Comment on attachment 8957527 [details] [diff] [review]

Thanks for the patch. The next step is to get it reviewed. I'll do this in the next few days.
Attachment #8957527 - Flags: review?(jorgk)
Thank you very much for your quick response!
Attached patch 1444371.patchSplinter Review
A few remarks before I start the review:
- Please give patches a meaningful name, I deal with dozens of
  "Thunderbird patches" every day.
- We accept patches only if the apply to current trunk. If approved,
  we can back-port to prior versions.

I've rebased the patch to current trunk and also changed this:
bool isAsciiOnly = NS_IsAscii(static_cast<const char16_t*>(msgBody.get()));
The cast is necessary, see:

Now, the patch basically changes the "is ASCII" check to run on the UFT-16 original string instead of the string that was encoded into the target charset. Note that nsMsgI18NConvertFromUnicode() encodes from the internal UTF-16 format to a given charset, for example ISO-2022-JP.

It also reverts this change that was done to get some tests to pass. That change wasn't awfully well motivated. I can't see that this change was correct, since when ISO-2022-JP was requested, isAsciiOnly was never true. So in other words, whatever you write in ISO-2022-JP would always go out in ISO-2022-JP, even if it was plain ASCII.

Personally I also don't understand why an "is ASCII" test makes any sense after the conversion. Surely ISO-2022-JP is a 7bit charset, and anything in the target string will *always* be ASCII. So checking before the conversion makes more sense, as you pointed out.

Let me do some testing here and also ship this off to a try run to see whether it causes any problems. Maybe we have to fix any test failure in a different way.

Try here:
Attachment #8957527 - Attachment is obsolete: true
Attachment #8957527 - Flags: review?(jorgk)
Attachment #8957832 - Flags: review?(jorgk)
Blocks: 1202401
Ever confirmed: true
Whiteboard: [regression:TB45]
Comment on attachment 8957832 [details] [diff] [review]

The try run isn't finished, but I ran the test that failed in bug 1202401, test-charset-edit.js, locally and it passed. I don't expect further trouble, but I'll check the try run again later.

Thanks again for the contribution.
Attachment #8957832 - Flags: review?(jorgk) → review+
Try also green.
Assignee: nobody → rokuyama
Pushed by
check "is ASCII" before encoding to target charset and remove ISO-2022-JP special case. r=jorgk
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 60.0
Thank you very much for your kind response, and I'm so sorry for bothering you with the patch for wrong trunk and with inappropriate file name.

I tried a nightly build of 60.0a1 (2018-03-11), which already contains the fix (, then everything works just fine for me.

Let me thank you again for kind cooperation.
Thanks for fixing the bug!

I can't include the fix in TB 52.7 due out next week or so, but it could go into TB 52.8. But at that time, TB 60 ESR will be released.
Thank you for the information. I look forward to TB 60 ESR :-).
You need to log in before you can comment on or make changes to this bug.