Expression-Search-NG icon indicating copy to clipboard operation
Expression-Search-NG copied to clipboard

[Bug] ESNG does not initialize properly in some context(s?)

Open oleole39 opened this issue 1 year ago • 8 comments

Hello @opto,

Good news, I am finally getting ESNG working again for me since I last reported a bug, thanks to the little fix described below.

1. Symptom

Now using TB 102.7.1 & ESNG 3.8.10, the issue was the same than at the time of the bug report mentionned, i.e. :

  • no ESNG search box displayed
  • nothing happening when clicking on the toolbar's "Expression" button (of which the icon had disappeared).
  • no tool icon on the add-on card displaying to set the extension's preferences
  • related console error message similar to the one shown in my previous bug report:
    wind is null implementation.js:54
        onStartup jar:file:///{my_TB_profile_folder}/extensions/[email protected]!/api/ExpressionSearch/implementation.js:54
        onStartup resource://gre/modules/ExtensionCommon.jsm:1464
        openWindowPrompt resource://gre/modules/Prompter.jsm:1226
        openPrompt resource://gre/modules/Prompter.jsm:1067
        openPromptSync resource://gre/modules/Prompter.jsm:1046
        confirmEx resource://gre/modules/Prompter.jsm:1465
        confirmEx resource://gre/modules/Prompter.jsm:305
        onProfileStartup resource:///modules/OfflineStartup.jsm:94
        observe resource:///modules/OfflineStartup.jsm:123
    

2. Context

I eventually understood that this issue was only happening if TB preference offline.startup_state was set to 1 (i.e. show an prompt box at startup to ask whether to go online).

3. Problem

I don't really know what returns call such as Services.wm.getMostRecentWindow("mail:3pane") but my guess is that when making this call at startup in the given context, ESNG actually finds an object relative to the prompt box instead of the mail pane, resulting to a null value (it may be similar to the issue discussed there). This case is not foreseen in the onStartup() function which cannot handle the case and thus fails to initialize ESNG properly.

4. Proposed fix

In file /[email protected]/api/ExpressionSearch/implementation.js, replace line 53:

if (typeof (wind) != "undefined") {

with:

if (wind != null) { // checks that 'wind' is neither null nor undefined, cf. https://stackoverflow.com/questions/2559318/how-to-check-for-an-undefined-or-null-variable-in-javascript#answer-15992131

This seems to solve my issue.

Here is a packaged version of the fix if it can help some other people. Install instructions:

  • download,
  • change zip extension into xpi (Github wouldn't accept attaching xpi),
  • select "install from file" via Thunderbird

5. Going further

I notice there are in ESNG's code other checks for other contexts in which typeof(var) compares to "undefined" . Would there be some potential other bugs which would require the null case to be foreseen as well ?

oleole39 avatar Mar 07 '23 06:03 oleole39