1638339 - JavaScript Error: "TypeError: can't access property "key", gCurrentIdentity is null" {file: "chrome://messenger/content/messengercompose/MsgComposeCommands.js" line: 5257

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[ISHIKAWA, Chiaki]

Attachment: Excerpt from local mochitest log where the errors occur.

During mochitest execution with FULL DEBUG version of TB under Debian
GNU/Linux 64-bit, I saw the following error three times.

JavaScript error: chrome://messenger/content/messengercompose/MsgComposeCommands.js, line 5257: TypeError: can't access property "key", gCurrentIdentity is null

I think the error is architecture neutral and so I set it to ALL.

The source code in question is as follows.: (I am afraid the line
number from my few days old tree is one line off from searchfox line.)
https://searchfox.org/comm-central/source/mail/components/compose/content/MsgComposeCommands.js#5258

function getCurrentIdentityKey() {
  // Get the identity key.
  return gCurrentIdentity.key; <=== 
}

The errors happen in the following tests.

29:40.96 INFO Entering test bound testPlaintextComposeWindowSwitchSignatures
29:45.89 GECKO(539290) JavaScript error: chrome://messenger/content/messengercompose/MsgComposeCommands.js, line 5257: TypeError: can't access property "key", gCurrentIdentity is null
29:45.90 INFO Leaving test bound testPlaintextComposeWindowSwitchSignatures

29:45.90 INFO Entering test bound testPlaintextComposeWindowSwitchSignaturesWithSuppressedSeparator
29:45.91 INFO Console message: [JavaScript Error: "TypeError: can't access property "key", gCurrentIdentity is null" {file: "chrome://messenger/content/messengercompose/MsgComposeCommands.js" line: 5257}]
29:51.24 GECKO(539290) JavaScript error: chrome://messenger/content/messengercompose/MsgComposeCommands.js, line 5257: TypeError: can't access property "key", gCurrentIdentity is null

29:51.24 INFO Leaving test bound testPlaintextComposeWindowSwitchSignaturesWithSuppressedSeparator
29:51.24 INFO Entering test bound testHTMLComposeWindowSwitchSignatures
29:51.25 INFO Console message: [JavaScript Error: "TypeError: can't access property "key", gCurrentIdentity is null" {file: "chrome://messenger/content/messengercompose/MsgComposeCommands.js" line: 5257}]
29:57.13 INFO Leaving test bound testHTMLComposeWindowSwitchSignatures
         ...
30:10.40 INFO Entering test bound testHTMLComposeWindowSwitchSignaturesWithSuppressedSeparatorParagraphFormat
30:26.20 GECKO(539290) JavaScript error: chrome://messenger/content/messengercompose/MsgComposeCommands.js, line 5257: TypeError: can't access property "key", gCurrentIdentity is null
30:26.24 INFO Console message: [JavaScript Error: "TypeError: can't access property "key", gCurrentIdentity is null" {file: "chrome://messenger/content/messengercompose/MsgComposeCommands.js" line: 5257}]
30:26.24 INFO Leaving test bound testHTMLComposeWindowSwitchSignaturesWithSuppressedSeparatorParagraphFormat
30:26.27 TEST_END: Test OK. Subtests passed 70/70. Unexpected 0

The above was produced by

egrep "(TEST_START|TEST_END|Entering test|Leaving test|can't access property \"key\")" /FF-NEW/log1220-mochitest.txt

I am attaching an excerpt from my local mochitest execution with FULL
DEBUG version of TB.
The log is more verbose than normal execution because I have added
local debug dump.

What irks me is that mochitest framework did not fail this test even
though this TypeError happened. We really need to fail a test that
produces TypeError(s).

Comment 1

[Magnus Melin [:mkmelin]]

Tip: to find out where some problem happens, add console.trace() near the problem. If I do that and run ./mach test comm/mail/test/browser/composition/browser_signatureUpdating.js I get

0:20.61 GECKO(16920) console.trace:
0:20.61 GECKO(16920) chrome://messenger/content/messengercompose/MsgComposeCommands.js 5275 getCurrentIdentityKey
0:20.61 GECKO(16920) chrome://messenger/content/messengercompose/MsgComposeCommands.js 5285 getCurrentIdentity
0:20.61 GECKO(16920) chrome://openpgp/content/ui/enigmailMsgComposeOverlay.js 394 setIdentityDefaults
0:20.61 GECKO(16920) chrome://openpgp/content/ui/enigmailMsgComposeOverlay.js 266 setIdentityCallback/<
0:20.61 GECKO(16920) chrome://openpgp/content/modules/timer.jsm 30 callbackWrapper
0:20.61 GECKO(16920) resource://gre/modules/Timer.jsm 62 notify
0:20.61 GECKO(16920) resource://testing-common/mozmill/utils.jsm 425 waitFor
0:20.61 GECKO(16920) resource://testing-common/mozmill/WindowHelpers.jsm 433 WindowWatcher_waitForWindowClose
0:20.61 GECKO(16920) resource://testing-common/mozmill/WindowHelpers.jsm 699 wait_for_window_close
0:20.61 GECKO(16920) resource://testing-common/mozmill/WindowHelpers.jsm 710 close_window
0:20.61 GECKO(16920) resource://testing-common/mozmill/ComposeHelpers.jsm 236 close_compose_window
0:20.61 GECKO(16920) chrome://mochitests/content/browser/comm/mail/test/browser/composition/browser_signatureUpdating.js 259 HTMLComposeWindowSwitchSignatures
0:20.61 GECKO(16920) chrome://mochitests/content/browser/comm/mail/test/browser/composition/browser_signatureUpdating.js 268 testHTMLComposeWindowSwitchSignaturesWithSuppressedSeparator
0:20.61 GECKO(16920) chrome://mochikit/content/browser-test.js 1064 Tester_execTest/<
0:20.61 GECKO(16920) chrome://mochikit/content/browser-test.js 1104 Tester_execTest
0:20.61 GECKO(16920) chrome://mochikit/content/browser-test.js 927 nextTest/<
0:20.61 GECKO(16920) chrome://mochikit/content/tests/SimpleTest/SimpleTest.js 918 SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<

So from here: https://searchfox.org/comm-central/rev/d492eaa594d697580eb7da55f2880acd9fd2fa7a/mail/extensions/openpgp/content/ui/enigmailMsgComposeOverlay.js#266 which is (likely) from https://searchfox.org/comm-central/source/mail/extensions/openpgp/content/ui/enigmailMsgComposeOverlay.js#873

But none of that is needed. It's long since we cached compose windows so every composition will get a new fresh context.

Comment 2

[Magnus Melin [:mkmelin]]

Attachment: bug1638339_enigmail_identity.patch

Remove some unneeded calls. Since we do not cache the compose window, clearing variables is pointless. Next window will have new variables.

Removing event listeners is also not needed - they clear out when the window closes.

Comment 3

[Patrick Brunschwig]

Comment on attachment 9151240 [details] [diff] [review]
bug1638339_enigmail_identity.patch

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

The removals are all fine; there is still more obsolete code that can be removed, see comments.

::: mail/extensions/openpgp/content/ui/enigmailMsgComposeOverlay.js
@@ +3813,1 @@
>    gMsgCompose.UnregisterStateListener(Enigmail.composeStateListener);

Why do you keep this? Is there a situation in which OpenPGP could be disabled or upgraded independently of Thunderbird?

@@ +3818,5 @@
>    once: true,
>  });
>  
>  window.addEventListener(
>    "unload-enigmail",

I don't think that this event would still be triggered. It stems from Enigmail's overlay.jsm function (which makes all of composeUnload obsolete)..

Comment 4

[Magnus Melin [:mkmelin]]

(In reply to Patrick Brunschwig from comment #3)

gMsgCompose.UnregisterStateListener(Enigmail.composeStateListener);

Why do you keep this? Is there a situation in which OpenPGP could be
disabled or upgraded independently of Thunderbird?

I want to keep removing the statelistener on close. For XPCOM stuff something can otherwise stick around...
But you're right, it's now in a slightly wrong place.

Comment 5

[Magnus Melin [:mkmelin]]

Attachment: bug1638339_enigmail_identity.patch

Updated.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e31d162b318b
remove some unnneded enigmail calls. Not needed since we do not cache the compose window. r=PatrickBrunschwig