1235294 - Text Encoding menu disabled when charset not supported

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Lu Wei]

Attachment: cannotchangeencoding.eml

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20151221130713

Steps to reproduce:

View the message as in the attachment


Actual results:

Display is incorrect; cannot decode. I know the message can be viewed in GBK|GB18030 encoding, but Thunderbird do not let me: the View->Character Encoding menu is gray.


Expected results:

TB should enable the option to let user change the encoding. I am not asking for adding an old charset support; just enable the menu.

Comment 1

[Jorg K (CEST = GMT+2)]

Comment on attachment 8702172 [details]
cannotchangeencoding.eml

Hmm, the message says:
Content-Type: text/plain; charset=cp936; format=flowed

If I change that manually to UTF-8 before importing, the menu becomes available and I can switch to "Chinese, Simplified" (GBK).

Magnus knows about which support for which character sets has been recently removed.

Should the "Text Encoding" menu option be disabled when the charset of the message is not supported? That doesn't make sense since then the user can never try using a different charset.

Comment 2

[Jorg K (CEST = GMT+2)]

Oh, when I change the charset to GBK before importing, the message gets displayed correctly straight away.

Comment 3

[Lu Wei]

For history reason, there are many old|deprecated|redundant|similar encodings|character sets, and many texts with wrong type assertion.  TB could help to decide which one to use, but should give the option to let user decide for other chances.

Comment 4

[Magnus Melin [:mkmelin]]

(In reply to Jorg K (GMT+1) from comment #1)
> Should the "Text Encoding" menu option be disabled when the charset of the
> message is not supported? That doesn't make sense since then the user can
> never try using a different charset.

Yeah should probably be enabled but without any selection.

Bug 1217161 is adding cp936 support.

Comment 5

[Jorg K (CEST = GMT+2)]

Attachment: Removing the disabling (v1).

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 8713097 [details] [diff] [review]
Removing the disabling (v1).

Review of attachment 8713097 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM r=mkmelin

Seems changning encoding in this case doesn't work for an opened .eml though (only works when it's in a folder)

Comment 7

[Jorg K (CEST = GMT+2)]

(In reply to Magnus Melin from comment #6)
> Seems changning encoding in this case doesn't work for an opened .eml though
> (only works when it's in a folder)
Changing the encoding after opening a saved message doesn't work in general. I tried to display a windows-1252 encoded message in UTF-8 and nothing changed in the display. That's another bug, sadly.

Comment 8

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/2a6797a9c8aa129a7a46f39da52098998a83a09f
Bug 1235294 - Always enable text encoding menu. r=mkmelin

Comment 9

[Jorg K (CEST = GMT+2)]

Comment on attachment 8713097 [details] [diff] [review]
Removing the disabling (v1).

[Approval Request Comment]
Regression caused by (bug #): No regression.
User impact if declined: Can't select text alternative text encoding if charset of message is not supported.
Testing completed (on c-c, etc.): None.
Risk to taking this patch (and alternatives if risky):
No risk, just removing a few lines that disable the menu.

Comment 10

[Jorg K (CEST = GMT+2)]

Beta (TB 45):
https://hg.mozilla.org/releases/comm-beta/rev/5707268d5185
Aurora (TB 46) is coming.

Comment 11

[Jorg K (CEST = GMT+2)]

Aurora (TB 46):
https://hg.mozilla.org/releases/comm-aurora/rev/8bbc5143a6f1

Comment 12

[Lu Wei]

Thank you for the fix! I'll waiting for the release.

Comment 13

[Jorg K (CEST = GMT+2)]

I pushed it to beta, so it will come out in mid-March in TB 45 (unless you want to use a pre-release version now).

Comment 14

[Jorg K (CEST = GMT+2)]

I've raised bug 1244110 for the issue with opening a save message (.eml file) and changing the text encoding.

Comment 16

[infinity0]

Thanks for the pointer to the duplicate - I had come to the same solution as 8bbc5143a6f1 myself yesterday. But do you think it should also be applied here? https://hg.mozilla.org/releases/comm-aurora/file/tip/suite/mailnews/mailWindowOverlay.js#l158 That is:

~~~~
--- a/suite/mailnews/mailWindowOverlay.js
+++ b/suite/mailnews/mailWindowOverlay.js
@@ -155,10 +155,6 @@
   if (threads_menuitem)
     threads_menuitem.setAttribute("disabled", gAccountCentralLoaded);
 
-  var charset_menuitem = document.getElementById("charsetMenu");
-  if (charset_menuitem)
-    charset_menuitem.setAttribute("disabled", !msgWindow.mailCharacterSet);
-
   // Initialize the Message Body menuitem
   var isFeed = gFolderDisplay.selectedMessageIsFeed;
   document.getElementById('viewBodyMenu').hidden = isFeed;
~~~~

Comment 17

[Magnus Melin [:mkmelin]]

That's for SeaMonkey (suite). In another bug, if anyone wants to persue that.

Comment 18

[Jorg K (CEST = GMT+2)]

Magnus forgot to say "yes": "Yes", in another bug. You need to poke the SM people, see:
http://www.seamonkey-project.org/about
Ratty, would you care to port this to SM?

Comment 19

[Jorg K (CEST = GMT+2)]

Actually, this bug here caused a regression. So the port would need to involve the fix from bug 1244430:
+
+  // Disable the charset item if there's nothing to enable
+  document.getElementById("charsetMenu")
+          .setAttribute("disabled", !gMessageDisplay.displayedMessage);
+  let appmenuCharset = document.getElementById("appmenu_charsetMenu");
+  if (appmenuCharset)
+    appmenuCharset.setAttribute("disabled", !gMessageDisplay.displayedMessage);

See attachment 8713971 [details] [diff] [review].

Comment 20

[Philip Chee]

I filed Bug 1260035 for SeaMonkey.