Closed Bug 1638339 Opened 10 months ago Closed 9 months ago

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

Categories

(Thunderbird :: Message Compose Window, defect)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 78.0

People

(Reporter: ishikawa, Assigned: mkmelin)

Details

Attachments

(2 files, 1 obsolete file)

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).

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.

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.

Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #9151240 - Flags: review?(patrick)
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)..
Attachment #9151240 - Flags: review?(patrick) → review+

(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.

Updated.

Attachment #9151240 - Attachment is obsolete: true
Attachment #9151291 - Flags: review+
Target Milestone: --- → Thunderbird 78.0

Pushed by geoff@darktrojan.net:
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

Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.