We should probably do this now. Even Japanese set the mailnews.send_default_charset to UTF-8 since back in 2017. https://dxr.mozilla.org/l10n-central/source/ja/mail/chrome/messenger/messenger.properties#281
If they can do it, and Gmail has done it since 4-5 years already, it doesn't seem likely it would cause any problems.

Let's take this on now, and save ourselves from some pointless charset complications.

Can you please post to the relevant mailing lists announcing this change.

This patch

• removes the Text Encoding menu from compose window
• removes mailnews.send_default_charset preference

Do we want to remove mailnews.reply_in_default_charset as well?

(In reply to Jorg K (CEST = GMT+2) from comment #21)

Can you please post to the relevant mailing lists announcing this change.

OK, I will post to the maildev list after my patch got reviewed, so that I know I understand the problem correctly.

Do we want to remove mailnews.reply_in_default_charset as well?

It seems to me that pref needs to be removed (in the sense of always replying in UTF-8) in order to end up it a state where all outgoing email is in UTF-8 and the non-UTF-8 email generation code paths become irrelevant.

Yes that should be removed as well.

• Remove mailnews.reply_in_default_charset and mailnews.disable_fallback_to_utf8
• Remove characterSet from nsIMsgCompFields
• Fix tests

Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3a5a7916a054ec9d1d1096c98168254fcaf87cbc

Fix Windows build. Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=149179d49660f1d14bce26b17c2db9

I don't think it's very useful to split up backend, UI and test changes. We could split up removal of delsp though.

::: mailnews/compose/test/unit/test_autoReply.js
@@ +215,5 @@

// The text gets converted to UTF-8 (regardless of what it is) at some point.
// Suspect a bug with how BinaryInputStream handles the strings.
if (templateHdr.Charset == "windows-1252") {
// XXX: should really check for " åäö"

• if (!body.includes("åäö xlatin1")) {

I don't think this should change?
Old templates/drafts can have outdated charsets. We'll have to deal with that

Yes, it's a bit confusing, but the change is needed. Take https://searchfox.org/comm-central/source/mailnews/test/data/template-utf8#26 for example, åäö xutf8 became "åäö xutf8" in the test file https://searchfox.org/comm-central/source/mailnews/compose/test/unit/test_autoReply.js#230. Maybe due to https://searchfox.org/comm-central/source/mailnews/compose/test/unit/test_autoReply.js#214-216

I can split up if that makes reviewing easier. But if you don't mind, I will not split then, and will remove delsp in a separate patch.

I'm surprised that https://searchfox.org/comm-central/source/mail/test/browser/composition/browser_charsetEdit.js doesn't need to change.

I thought it was irrelevant so I removed it, seems I'm wrong, will update.

Sorry, I missed the removal. Maybe it is irrelevant now, I'd have to check closer. What happens with you receive a message in windows-1252 or ISO-2022-JP and you go "edit as new message". That needs to be successfully converted into UTF-8 now. That case is a little similar to that test, no?

(In reply to Jorg K (CEST = GMT+2) from comment #28)

Also, keep in mind that the outgoing delsp processing was only introduced
for ISO-2022-JP, so if you scrap that, the delsp processing can be removed,
for example here:

I was going to say that's not entirely true, but then I went back searching through the bugs and remembered that we have this giant cesspool of interaction between format=flowed, plaintext serializer, charset, and Content-Transfer-Encoding selection that is overcomplicated and way too deferential to supporting legacy issues that may not matter anymore, and so what ought to be the design in a smart modern system has too little implications for our existing codebase. When we can watch all of this crap code go up in a merry blaze, I will be quite happy.

I haven't looked at this patch in detail yet (splinter review is crapping out on me), but I will ask that any time you assign "UTF-8" to a nsString only to immediately pass it into a UTF-16-to-ASCII (or UTF-8) conversion function, just strip out the middleman and assign it to a nsCString instead. Better yet, try to go forward and pass "UTF-8" literal strings to directly wherever the results are being used.

• Update browser_charsetEdit.js
• Fix some comments and pass "UTF-8" directly in a few places

Remove delsp related code.

Comment on attachment 9169733 [details] [diff] [review]
862292-delsp.patch

You missed https://searchfox.org/comm-central/rev/b3767e49a6acf1f66abd0a276bd71579eada4787/mailnews/compose/src/MessageSend.jsm#359 (that's new, I know, but Searchfox finds it). Actually, there is more in that file:
https://searchfox.org/comm-central/search?q=DelSp&path=&case=false&regexp=false

Not that the stuff in mimetpfl.cpp is for incoming delsp, so that needs to stay.

Thanks, if it's OK with you, I'm planning to update MessageSend.jsm related stuff in bug 1211292, after this bug is closed.

Comment on attachment 9169733 [details] [diff] [review]
862292-delsp.patch

Sure. Sorry.

Remove unnecessary charset and disallowBreaks arguments from GetSerialiserFlags.

This change breaks Chinese, Japanese and Korean languages (CJK), and probably some others.

The issue is that Unicode is broken for CJK. Early on they made the mistake of trying to merge many characters in these languages that are in fact not the same, so it becomes impossible to create a true Unicode font that works for more than one of the three.

It's a severe issue because it prevents people writing their names correctly and other basic stuff like that. Worse still the receiver of the email may see a different character on their screen to the one the sender wrote.

So until Unicode gets fixed for CJK Thunderbird should default to their preferred national character encodings. I don't know if it would be easier to have an allowlist or denylist of broken Unicode languages because I'm not sure how widespread this problem is. I seem to recall some Thai people complaining they couldn't render their names in Unicode a few years back.

Thai uses UTF-8 already so I can't think that's a problem. Japanese also uses UTF-8.
zh-TW uses UTF-8 as well. I'm assuming zh-CN uses gbk from old habit or more or less political reasons?

Korean uses EUC-KR, I don't know if there's a reason. If there's a reason, what does Gmail do for Korean?

https://dxr.mozilla.org/l10n-central/search?q=mailnews.send_default_charset&redirect=false

(In reply to Magnus Melin [:mkmelin] from comment #41)

Thai uses UTF-8 already so I can't think that's a problem.

It's not a problem. Han unification is irrelevant to Thai. The way Unicode analyzes Thai is based on the Thai national standard that existed before. There's no benefit from using the legacy encoding.

Japanese also uses UTF-8.

Moreover, absent a hint of how to resolve CJK ambiguity, Gecko resolves in favor of Japanese by default. This makes sense for platforms that have distinct font designs for different CJK locales, since this way kana and kanji match design-wise.

zh-TW uses UTF-8 as well.

So there's no change from this patch for Traditional Chinese.

I'm assuming zh-CN uses gbk from old habit or more or less political reasons?

There are potential relevance to font selection for incoming email that was labeled as gbk (if the CJK resolution hint from encoding even survives in Thunderbird the way it survives in Firefox; I didn't check). However, if the result is unsatisfactory, users can change the font Thunderbird uses to a font that resolves the issue. In any case, zh-CN users have been receiving UTF-8 email from non-Thunderbird senders for ages and, therefore, must have dealt with this already.

Korean uses EUC-KR, I don't know if there's a reason. If there's a reason, what does Gmail do for Korean?

Han characters are rarely used in Korean these days. They mostly appear as abbreviations in newspaper headlines and in parenthetical clarifications. I don't have statistics for email, but it's indicative that the Android input method for Korean doesn't even support the input of Han characters. Hangul doesn't have anything to unify with C or J, so there are no unification issues.

This is incorrect, Japan still uses a lot of Shift-JIS because of the Unicode issues and I believe China is BIG-5, but I'm not an expert on Chinese.

Again though, the issue is less of a problem for say Japanese people who only communicate with other Japanese people in Japanese. Where it becomes an issue is when they need to talk to a Chinese or Korean person.

Imagine if you booked a flight from Xiamen to Tokyo but the ticket had your name printed wrong because of Unicode. Now you have to explain to the staff that it's a character encoding issue, your browser displays Chinese Unicode and the airline uses Japanese Unicode and please look at the Latin transliteration and ignore the fact that there is a mismatch with my passport.

I remembered wrong, it was Bengali that was the issue: https://modelviewculture.com/pieces/i-can-text-you-a-pile-of-poo-but-i-cant-write-my-name

That was 2015, I don't know if they have fixed it and all relevant fonts have been updated and all relevant software that processes and displays Unicode has been fixed.

I suggest that Thunderbird displays a notification to the user asking them if they want to make the change, and warning them that it could cause potential issues with certain languages (Chinese, Japanese, Korean, Bengali, possibly others).

(In reply to kuro68k from comment #43)

This is incorrect, Japan still uses a lot of Shift-JIS because of the Unicode issues and I believe China is BIG-5, but I'm not an expert on Chinese.

I request that you refrain from arguing about this further on this bug report. The above sentence is indicative that it is not going to be productive.

I remembered wrong, it was Bengali that was the issue: https://modelviewculture.com/pieces/i-can-text-you-a-pile-of-poo-but-i-cant-write-my-name

That article is unfortunate in its attribution of issues. Unicode's analysis of Bengali is based on ISCII, which the federal government of India issued in 1988. The approach of analyzing non-Devanagari Brahmic scripts used in India as isomorphisms of Devanagari comes from there.

In any case, Thunderbird doesn't support any non-Unicode encodings for Bengali. Also, Thunderbird processes all text as Unicode internally, so the notion that not using UTF-8 for interchange somehow avoided Unicode is mistaken.

That article is unfortunate in its attribution of issues.

Not really, it either works or it doesn't and Unicode doesn't. Naturally people who can't write their own names in it care little where the blame is assigned, only that it gets fixed.

Anyway I realized that this isn't an issue as long as Thunderbird adds the right RFC 3282 header. Well, it is an issue because at least outside of HTML email you can't mix CJK, but it addresses the main problem here.

(In reply to Henri Sivonen (:hsivonen) from comment #42)

Japanese also uses UTF-8.

Moreover, absent a hint of how to resolve CJK ambiguity, Gecko resolves in favor of Japanese by default. This makes sense for platforms that have distinct font designs for different CJK locales, since this way kana and kanji match design-wise.

It is no longer true. We changed the precedence recently (see bug 1581822) and made it configurable (bug 1596875).

(In reply to kuro68k from comment #43)

This is incorrect, Japan still uses a lot of Shift-JIS because of the Unicode issues and I believe China is BIG-5, but I'm not an expert on Chinese.

Did you forget that this bug is about text encoding for e-mails? We (Japanese) do not use Shift-JIS encoding for e-mail messages. We used to use ISO-2022-JP exclusively.

(In reply to Joshua Cranmer [:jcranmer] from comment #47)

An interesting test to add to conduct (and add to our test suite) is
replying inline to a non-UTF-8 message. A further test would be to reply
inline to a message whose charset has been manually overridden by the UI.

Right, and, as I said in comment #32, we also need to cover the other code path, which is forward, "edit as new message", edit draft/template. You could have an old non-UTF-8 draft/template. Remember that composition has two very distinct code paths: One is reply and one is the other lot.

EDIT: Looks like there are some forwarding tests here: https://searchfox.org/comm-central/source/mail/test/browser/composition/browser_forwardUTF8.js which can be extended to non-UTF-8 messages.
There is also https://searchfox.org/comm-central/source/mail/test/browser/composition/browser_replyMultipartCharset.js which uses body-greek.eml. So you'd have to check what is covered and what is not and extend what is already there.

Updates according to comment 47. Fixed QuoteMessage argument, added browser_quoteMessage.js.

An interesting test to add to conduct (and add to our test suite) is replying inline to a non-UTF-8 message. A further test would be to reply inline to a message whose charset has been manually overridden by the UI.

I've tested replying to non-UTF8 message a few times before, it worked well. After your comment, I became aware of the Options > Quote Message menu, it did corrupt non-UTF8 text, thanks. I've fixed argument of QuoteMessage and added browser_quoteMessage.js for this case. For the case of manually changing the view charset to an incorrect one, then click reply, the text is corrupted as well. But I think that is expected, at least it's the same on nightly, do I need to do anything?

(In reply to Jorg K (CEST = GMT+2) from comment #32)

Sorry, I missed the removal. Maybe it is irrelevant now, I'd have to check closer. What happens with you receive a message in windows-1252 or ISO-2022-JP and you go "edit as new message". That needs to be successfully converted into UTF-8 now. That case is a little similar to that test, no?

Sorry, I forgot to reply this message earlier, click "edit as new message" on a ISO-2022-JP message works well.

(In reply to Jorg K (CEST = GMT+2) from comment #48)

Right, and, as I said in comment #32, we also need to cover the other code path, which is forward, "edit as new message", edit draft/template. You could have an old non-UTF-8 draft/template. Remember that composition has two very distinct code paths: One is reply and one is the other lot.

Seems to me browser_replyMultipartCharset.js already covered reply, edit as new and forward. And test_autoReply covered non-UTF8 template.

The case I missed was Options > Quote Message, which is fixed in this patch.

Updates according to comment 50.

• use var
• use NS_ConvertUTF16toUTF8
• remove unused test

We usually use var for the top level variables.

Seems to me const is used more in m-c now.

Maybe the forceMsgEncoding and isAsciiOnly are not something we need anymore?

I think isAsciiOnly can be removed, maybe we should do it separately. But forceMsgEncoding is used to determine QP. https://searchfox.org/comm-central/source/mailnews/compose/src/nsMsgAttachmentHandler.cpp#334

eslint

(In reply to Magnus Melin [:mkmelin] from comment #54)

mOriginalHTMLBody = NS_ConvertUTF16toUTF8(origHTMLBody).get() ?

= NS_ConvertUTF16toUTF8(bodyText).get()?

Wouldn't using .get() result in use-after-free? What would keep the pointee of mOriginalHTMLBody alive in that case?

Why isn't mOriginalHTMLBody an nsCString in the first place?

I think you're right. Looks like we should just make mOriginalHTMLBody an nsCString.

Updates according to comment 54.

Maybe you don't need attachment1_body anymore either

I think it's still needed since there is rv = SnarfAndCopyBody(attachment1_body, TEXT_HTML); later.

(In reply to Magnus Melin [:mkmelin] from comment #56)

I think you're right. Looks like we should just make mOriginalHTMLBody an nsCString.

I didn't make this change. There are several other char* and ToNewCStrings in nsMsgSend, I don't see the immediate benefit of only changing mOriginalHTMLBody to nsCString in this patch.

Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=32a5ce888a39968972d2b1be17efef

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/58159743bab4
Use UTF-8 for all outgoing email. r=mkmelin
https://hg.mozilla.org/comm-central/rev/0d9bc3ca1452
Remove delsp since UTF-8 is always used. r=mkmelin

There's a new failure in comm/mailnews/compose/test/unit/test_longLines.js on Windows, which seems likely to be related to this bug. Please check it out.

Example failure log:
https://treeherder.mozilla.org/logviewer.html#?job_id=314049833&repo=comm-central&lineNumber=4043

(It's there in your Try run too. I didn't spot it before landing.)

Fix test failure in test_longLines.

There is also a failure in browser_quoteMessage.js related to open menu on macOS, looking into it.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/a6bcebe11fbc
followup - Fix newline character in test_longLines. r=mkmelin DONTBUILD

Call goDoCommand("cmd_quoteMessage") directly on macOS to fix browser_quoteMessage.
Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=08fae1a373dc2579e3e59dad3a3deac5549a56ab

Just found browser_quoteMessage still failed for macOS on Try server, will take another look.

sleep(1) seems more reliable. https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=dd65416e54766e5131970a3d1e83c62a1268

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/e79e0ea42b97
followup - Workaround clicking menu on mac to make browser_quoteMessage pass. r=mkmelin