1716059 - "View > Repair Text Encoding" menu item on message source window is disabled on trunk for messages with CTE quoted-printable

Status: RESOLVED INVALID

(MailNews Core :: Internationalization, defect)

Description

[José M. Muñoz]

+++ This bug was initially created as a clone of Bug #1713786 +++

Looking at bug 1713786 I found another issue:

STR:
Click on any message, press Ctrl+U, or "View > Message Source".
On the message source window, try to use "View > Repair Text Encoding".

Actual:
Menu item disabled.

Expected:
Menu item enabled and functional.

Note: This was working on 1st of June 2021 when bug 1704749 landed. So it's a very recent regression. Alice, can you please find it for us.

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=de4e453066c0964e9b4e8e73c213c9241aa64754&tochange=a84d70e678b453f16894023f0d7754a60cf5fc29
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=391dbe0ceb290de3c1a6989aab62e602e55f176c&tochange=9992f4bb88c46f06d52bdeefb3bd204372e9a1ad

Comment 2

[José M. Muñoz]

Thank you, Alice. Looks like this was regressed by bug 1704749. Strangely I tried its patches in a local build back then and the menu worked on the message source window.

Comment 3

[José M. Muñoz]

There appear to be a few things wrong here. Firstly this code
https://searchfox.org/comm-central/rev/440de9c49313b0232a07148236cf1681995b4367/common/src/viewSource.js#34-45
added in bug 1704749 appears to be incorrect. The load event doesn't fire and if it did, the disabled attribute should be removed instead of set to false, like here:
https://searchfox.org/mozilla-central/rev/0c7c41109902cb8967ec3ef2c0ddb326701cfbee/browser/base/content/browser.js#4890
Thunderbird never disabled the charset menu, see https://hg.mozilla.org/comm-central/rev/a84d70e678b4#l2.31 (no onpopupshowing), but if that's desired, it should be done as M-C does.

Removing viewSource.js#34-45 will permanently enable the "repair" menu item, like it was in the past, but then viewSourceChrome.onForceCharacterSet() still doesn't work, even if - during debugging - setting the charset to the correct one instead of _autodetect_all.

Also note that bug 1713627 will remove _autodetect_all, so further adjustment will be necessary.

Comment 4

[José M. Muñoz]

Attachment: 1716059.patch

Looking at the M-C regression range again and reverting this hunk from bug 1687635
https://hg.mozilla.org/mozilla-central/rev/9992f4bb88c46f06d52bdeefb3bd204372e9a1ad#l1.12
the "repair" menu item is in fact available again, so the flaws pointed out in the previous comment aren't so severe. Henri, why does this hunk change the behaviour for TB's message source window, looks like WillIgnoreCharsetOverride() is now returning true for that document. GetMayEnableCharacterEncodingMenu() calls that function
https://searchfox.org/mozilla-central/rev/0c7c41109902cb8967ec3ef2c0ddb326701cfbee/docshell/base/nsDocShell.cpp#2044
so no surprise the menu is disabled.

Please ignore the second paragraph of the previous comment. With this code removed by the patch, the "repair" does work on the message source window. Sorry, I was testing on a QP-encoded (ASCII) message and confused myself for a while.

So is it valid to have the menu always enabled like in the past?

Comment 5

[Henri Sivonen (:hsivonen)]

(In reply to José M. Muñoz from comment #4)

Henri, why does this hunk change the behaviour for TB's message source window, looks like WillIgnoreCharsetOverride() is now returning true for that document.

(I don't have TB set up in a way experiment with this myself.)

How does TB's message source window feed the content to the HTML parser? What content type and charset does the channel claim? Does the code override the charset source?

What level of MIME parsing has happened to even have non-ASCII characters to display (considering the typical quoted-printable or base64)? Or is this about emails that have arrived all the may in 8-bit mode?

(If the message source has enough QP or base64 to be all-ASCII, that would explain why the menu item gets disabled.)

So is it valid to have the menu always enabled like in the past?

If you enable it unconditionally, it will be enabled in more cases where invoking the menu item won't change anything.

Comment 6

[José M. Muñoz]

Comment on attachment 9226668 [details] [diff] [review]
1716059.patch

I was on the completely wrong track here testing on a CTE QP (ASCII) message where there is nothing to repair. It's all working when feeding a message with CTE 8bit. Sorry about the noise. I was just used to having the menu item always enabled, even if it didn't do anything.

Comment 7

[José M. Muñoz]

I'm marking this invalid. Would it be worth to remove the attribute instead of setting it to false here?
https://searchfox.org/comm-central/rev/440de9c49313b0232a07148236cf1681995b4367/common/src/viewSource.js#36,42

Anyway, the experience shows that the charset menu is useful for messages which use CTE QP since automatic detection can't work there.