1244430 - View->Text encoding produces exception with nsIStringBundle.GetStringFromName

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect)

Description

[:aceman]

In Thunderbird trunk, if there is no message selected, going into View->Text encoding produces an exception:

Error: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]
Source File: XStringBundle
Line: 21

The list of encodings has no item selected.
If I select a message, and go into the menu, an encoding is selected (e.g. utf-8). That one then sticks even if unselecting any message and the exception no longer occurs.

On TB38 in that situation the Encoding menu is disabled so viewing the list is not possible and no exception occurs. Possible regression from bug 1235294.

Comment 1

[Jorg K (CEST = GMT+2)]

Thanks for noticing. Yes, this is printed on the debug console:
JavaScript error: XStringBundle, line 21: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]

Certainly a regression from bug 1235294 where this code that disabled "View > Text Encoding" got removed:
-
-  // Disable the charset item if there's nothing to enable
-  document.getElementById("charsetMenu")
-          .setAttribute("disabled", !msgWindow.mailCharacterSet);
-  let appmenuCharset = document.getElementById("appmenu_charsetMenu");
-  if (appmenuCharset)
-    appmenuCharset.setAttribute("disabled", !msgWindow.mailCharacterSet);

Looks like disabling of the item is desired when no message is selected. Disabling the item because the charset is not recognised/supported is not desired, since the user then can't select a better one.

I'll see whether there is a better condition to express "no message selected".

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: Restoring code removed in bug 1235294, but with a twist ;-) (v1).

So instead of
-
-  // Disable the charset item if there's nothing to enable
-  document.getElementById("charsetMenu")
-          .setAttribute("disabled", !msgWindow.mailCharacterSet);
-  let appmenuCharset = document.getElementById("appmenu_charsetMenu");
-  if (appmenuCharset)
-    appmenuCharset.setAttribute("disabled", !msgWindow.mailCharacterSet);

we have:
+
+  // 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);

This seems to work and does not allow access to the menu when no message is displayed.

Comment 3

[:aceman]

Comment on attachment 8713971 [details] [diff] [review]
Restoring code removed in bug 1235294, but with a twist ;-) (v1).

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

Yes, works for me. Thanks.

Comment 4

[Jorg K (CEST = GMT+2)]

Thanks for the quick review and sorry about the regression ;-(

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 8713971 [details] [diff] [review]
Restoring code removed in bug 1235294, but with a twist ;-) (v1).

[Approval Request Comment]
Regression caused by (bug #): 1235294
User impact if declined: Small, exception only visible on debug console.
Testing completed (on c-c, etc.): Manual testing.
Risk to taking this patch (and alternatives if risky):
No risky, it restores a slightly modified version of the code removed in bug 1235294.

Please approve beta for this since the causing bug was landed on beta.

Comment 6

[:aceman]

https://hg.mozilla.org/comm-central/rev/d468c5939d66

Comment 7

[Kent James (:rkent)]

If the status-thunderbird flags are set, they should be updated when the bug statues changes with a checkin.

Comment 8

[Kent James (:rkent)]

https://hg.mozilla.org/releases/comm-aurora/rev/0a13d74f0643
https://hg.mozilla.org/releases/comm-beta/rev/0e667df015f6