View->Text encoding produces exception with nsIStringBundle.GetStringFromName

RESOLVED FIXED in Thunderbird 47.0

Status

Thunderbird
Toolbars and Tabs
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: aceman, Assigned: Jorg K (GMT+2))

Tracking

({regression})

Trunk
Thunderbird 47.0
regression
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird44 unaffected, thunderbird45 fixed, thunderbird46 fixed, thunderbird47 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

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

Comment 1

a year ago
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".
Blocks: 1235294
(Assignee)

Comment 2

a year ago
Created attachment 8713971 [details] [diff] [review]
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.
Attachment #8713971 - Flags: review?(acelists)
(Reporter)

Comment 3

a year ago
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.
Attachment #8713971 - Flags: review?(acelists) → review+
(Reporter)

Updated

a year ago
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Keywords: regression
OS: Linux → All
Hardware: x86 → All
(Assignee)

Comment 4

a year ago
Thanks for the quick review and sorry about the regression ;-(
Keywords: checkin-needed
(Assignee)

Comment 5

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

Updated

a year ago
status-thunderbird45: --- → affected
status-thunderbird46: --- → affected
status-thunderbird47: --- → affected
(Reporter)

Comment 6

a year ago
https://hg.mozilla.org/comm-central/rev/d468c5939d66
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 47.0
If the status-thunderbird flags are set, they should be updated when the bug statues changes with a checkin.
status-thunderbird47: affected → fixed
https://hg.mozilla.org/releases/comm-aurora/rev/0a13d74f0643
https://hg.mozilla.org/releases/comm-beta/rev/0e667df015f6
status-thunderbird44: --- → wontfix
status-thunderbird45: affected → fixed
status-thunderbird46: affected → fixed

Updated

a year ago
Attachment #8713971 - Flags: approval-comm-beta? → approval-comm-beta+
(Assignee)

Updated

a year ago
status-thunderbird44: wontfix → unaffected

Updated

a year ago
Blocks: 1260035
You need to log in before you can comment on or make changes to this bug.