1576492 - this.tabmail is null error with browser.windows.getCurrent()

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: Extensions API, defect)

Description

[alta88]

background.js contains:

function getCurrentWindow() {
  return browser.windows.getCurrent();
}
getCurrentWindow().then((currentWindow) => {
  console.dir(currentWindow);
});

and the result is:

11:06:37.927 this.tabmail is null ext-mail.js:967
    get activeTab chrome://messenger/content/parent/ext-mail.js:967
    get title chrome://messenger/content/parent/ext-mail.js:824
    convert chrome://extensions/content/parent/ext-tabs-base.js:915
    convert chrome://extensions/content/parent/ext-tabs-base.js:2053
    getCurrent chrome://messenger/content/parent/ext-windows.js:70
    getCurrent self-hosted:1019
    result resource://gre/modules/ExtensionParent.jsm:1160
    withPendingBrowser resource://gre/modules/ExtensionParent.jsm:771
    result resource://gre/modules/ExtensionParent.jsm:1160
    callAndLog resource://gre/modules/ExtensionParent.jsm:1119
    call resource://gre/modules/ExtensionParent.jsm:1159
    InterpretGeneratorResume self-hosted:1300
    AsyncFunctionNext self-hosted:839

Comment 1

[Geoff Lankow (:darktrojan)]

Looks like we're in a state of "extension ready, window not ready". Somehow Firefox avoids this happening.

I think the right thing to do is return null, or we could throw an error.

I'm working on some other window API stuff, so I'll look at this again once that's done.

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: 1576492-windows-incomplete-1.diff

Yes, I think we should return null if the window isn't completely loaded.

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 9090271 [details] [diff] [review]
1576492-windows-incomplete-1.diff

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

It would be nice if we could just wait for it to be ready and return it then.

And do you want to return null, and not a Promise resolving null? I suppose the api handles that wrapping normally?

Comment 4

[Geoff Lankow (:darktrojan)]

I hadn't considered waiting, but I suppose it is acceptable, as I can't see any use case where returning immediately with null is better than returning soon with a result.

And yeah, the WE magic code handles just returning the result instead of a Promise.

Comment 5

[Geoff Lankow (:darktrojan)]

Attachment: 1576492-windows-incomplete-2.diff

Do you like this one better?

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 9090329 [details] [diff] [review]
1576492-windows-incomplete-2.diff

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

Looks good, thx! r=mkmelin

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d7405123c5b2
Wait for windows to load before returning from windows.getCurrent and .getLastFocused. r=mkmelin DONTBUILD

Comment 8

[Jorg K (CEST = GMT+2)]

Comment on attachment 9090329 [details] [diff] [review]
1576492-windows-incomplete-2.diff

Do we care about 68 ESR uplift? I guess it's OK to ship in beta now.

Comment 9

[alta88]

It's important the api resolves for the load otherwise it would be useless. And background.js runs very early at startup, and also when there is an install/undo disable or remove, which made for the extension having to figure these things out. Uplift to 68 would be great.

Comment 10

[Jorg K (CEST = GMT+2)]

Will be pushed to beta today, then TB 68.x with x > 1.

Comment 11

[Jorg K (CEST = GMT+2)]

TB 70 beta 1:
https://hg.mozilla.org/releases/comm-beta/rev/7ebbba95ac1a5d7f94dbb6287c64249cf3f15d69

Comment 12

[Jorg K (CEST = GMT+2)]

TB 68.1.1 ESR:
https://hg.mozilla.org/releases/comm-esr68/rev/0f180c4b528ce17f06daa7191f84a1e6f23bb4d9