Closed Bug 1244430 Opened 5 years ago Closed 5 years ago

View->Text encoding produces exception with nsIStringBundle.GetStringFromName

Categories

(Thunderbird :: Toolbars and Tabs, defect)

defect
Not set
normal

Tracking

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

RESOLVED FIXED
Thunderbird 47.0
Tracking Status
thunderbird44 --- unaffected
thunderbird45 --- fixed
thunderbird46 --- fixed
thunderbird47 --- fixed

People

(Reporter: aceman, Assigned: jorgk-bmo)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
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
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)
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+
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Keywords: regression
OS: Linux → All
Hardware: x86 → All
Thanks for the quick review and sorry about the regression ;-(
Keywords: checkin-needed
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+
https://hg.mozilla.org/comm-central/rev/d468c5939d66
Status: ASSIGNED → RESOLVED
Closed: 5 years 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.
Attachment #8713971 - Flags: approval-comm-beta? → approval-comm-beta+
Blocks: 1260035
You need to log in before you can comment on or make changes to this bug.