Text Encoding menu disabled when charset not supported

RESOLVED FIXED in Thunderbird 47.0

Status

Thunderbird
Mail Window Front End
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: Lu Wei, Assigned: Jorg K (GMT+2))

Tracking

38 Branch
Thunderbird 47.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird45 fixed, thunderbird46 fixed, thunderbird47 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

a year ago
Created attachment 8702172 [details]
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.
(Assignee)

Comment 1

a year ago
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.
Attachment #8702172 - Attachment mime type: message/rfc822 → text/plain
Flags: needinfo?(mkmelin+mozilla)
(Assignee)

Comment 2

a year ago
Oh, when I change the charset to GBK before importing, the message gets displayed correctly straight away.
Summary: character encoding menu gray → Test Encoding menu disabled when charset not supported
(Reporter)

Comment 3

a year ago
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

a year ago
(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.
Flags: needinfo?(mkmelin+mozilla)
(Assignee)

Comment 5

a year ago
Created attachment 8713097 [details] [diff] [review]
Removing the disabling (v1).
Attachment #8713097 - Flags: review?(mkmelin+mozilla)
(Assignee)

Updated

a year ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 6

a year ago
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)
Attachment #8713097 - Flags: review?(mkmelin+mozilla) → review+
Component: Untriaged → Mail Window Front End
(Assignee)

Comment 7

a year ago
(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.
Keywords: checkin-needed

Comment 8

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

Updated

a year ago
Assignee: nobody → mozilla
Status: NEW → RESOLVED
Last Resolved: a year ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 47.0
(Assignee)

Comment 9

a year ago
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.
Attachment #8713097 - Flags: approval-comm-beta+
Attachment #8713097 - Flags: approval-comm-aurora+
(Assignee)

Comment 10

a year ago
Beta (TB 45):
https://hg.mozilla.org/releases/comm-beta/rev/5707268d5185
Aurora (TB 46) is coming.
status-thunderbird45: --- → fixed
status-thunderbird46: --- → affected
status-thunderbird47: --- → fixed
(Assignee)

Comment 11

a year ago
Aurora (TB 46):
https://hg.mozilla.org/releases/comm-aurora/rev/8bbc5143a6f1
status-thunderbird46: affected → fixed
Summary: Test Encoding menu disabled when charset not supported → Text Encoding menu disabled when charset not supported
(Reporter)

Comment 12

a year ago
Thank you for the fix! I'll waiting for the release.
(Assignee)

Comment 13

a year ago
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).
(Assignee)

Comment 14

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

Updated

a year ago
Depends on: 1244430

Updated

a year ago
Duplicate of this bug: 1258875

Comment 16

a year ago
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

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

Comment 18

a year ago
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?
Flags: needinfo?(philip.chee)
(Assignee)

Comment 19

a year ago
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].

Updated

a year ago
Blocks: 1260035

Comment 20

a year ago
I filed Bug 1260035 for SeaMonkey.
(Assignee)

Updated

a year ago
Flags: needinfo?(philip.chee)
You need to log in before you can comment on or make changes to this bug.