1675940 - window.close() does not work from inside the window (regardless of allowScriptsToClose in browser.windows.create())

Status: RESOLVED FIXED

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

Description

[plieser]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

1. Open an extension window with

browser.windows.create({url: "./popup.html)", type: "popup", allowScriptsToClose: true});

2. In a script (type module) loaded inside popup.html, call window.close()

Actual results:

Warning that scripts can only close windows that they open.

Expected results:

Window should close itself, as the allowScriptsToClose was set to true (which should already be the case for extensions windows anyway according to https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/windows/create).

Note that the url for some reasons is not set for the tab if you e.g. call await browser.windows.getAll({populate: true}). Not sure if this is related.

As a workaround one can use browser.windows.remove() (see e.g. example at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/windows/remove#Examples).

The ugly think with the workaround is if you close the current window that way, the error console gets polluted with the following error message:

 Error: CallResult for closed conduit dkim_verifier@pl.2993: ({childId:"dkim_verifier@pl.2993", callId:2996, path:"windows.remove", result:{}}) ConduitsChild.jsm:167:13
    receiveMessage resource://gre/modules/ConduitsChild.jsm:167

Comment 1

[John Bieling (:TbSync)]

https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=0d9b6c6253ace147928627b0ab3a2298cf4692ae

Edit : obsolete

Comment 2

[John Bieling (:TbSync)]

First attempt did not work.

The reason this works in TB79+ but not in TB78 is this change introduced in bug 1353466 (TB79):
https://searchfox.org/comm-central/source/mozilla/dom/base/nsGlobalWindowOuter.cpp#6261

[TB79+]
The added check for !IsOnlyTopLevelDocumentInSHistory seems to bypass the check for allowClose further down. That is why the value for allowScriptsToClose is ignored (setting it to false has no effect in TB79+).
Due to bug 1353466 FF79+ and TB79+ ignore allowScriptsToClose = false and allows all scripts in popup windows to close themselves.

[TB78]
TB78 never called windowUtils.allowScriptsToClose() like FF78 did, so scripts cannot close themselves for us
https://searchfox.org/comm-central/source/mozilla/toolkit/components/extensions/ext-browser-content.js#80

The provided patch attaches allowScriptsToClose to the arguments passed to extensionPopup.xhtml and calls windowUtils.allowScriptsToClose() for the content window if needed.

As long as the free pass introduced in bug 1353466 is not removed from TB79+, this patch has no effect. But it is needed for TB78.

Comment 3

[John Bieling (:TbSync)]

Attachment: bug1675940.patch

Comment 4

[Geoff Lankow (:darktrojan)]

Comment on attachment 9188928 [details] [diff] [review]
bug1675940.patch

Well done figuring this out!

This patch needs a better commit message, but apart from that is good to go.

Comment 5

[John Bieling (:TbSync)]

Attachment: bug1675940.patch

Updated commit message.

Comment 6

[John Bieling (:TbSync)]

Attachment: bug1675940.patch

update email

Comment 7

[John Bieling (:TbSync)]

Try comm-central:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3751b295c49f012efb710efa54440be0e3b78912

Try comm-esr78:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=209ca90561fa01eb55c142712d5ce421fb950a0a

Comment 8

[John Bieling (:TbSync)]

For comm-central I could only check the fails in Windows 10 locally

Without the patch:
X1: no fail
X2: fail

With the patch
X1: no fail
X2: fail

I have no idea why X1 fails on comm-central, if I run the latest tip locally it does not fail for me.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e5f25cad7939
Honour allowScriptsToClose argument in windows.create API function. r=darktrojan

Comment 10

[John Bieling (:TbSync)]

Comment on attachment 9189104 [details] [diff] [review]
bug1675940.patch

[Approval Request Comment]
User impact if declined:
windows API will not work as documented

Testing completed (on c-c, etc.):
On comm-esr78
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=209ca90561fa01eb55c142712d5ce421fb950a0a

Risk to taking this patch (and alternatives if risky):
I hope not.

Comment 11

[Wayne Mery (:wsmwk)]

Comment on attachment 9189104 [details] [diff] [review]
bug1675940.patch

[Triage Comment]
Approved for beta
Approved for esr78

Comment 12

[Dave Miller [:justdave]]

Thunderbird 84.0b2
https://hg.mozilla.org/releases/comm-beta/rev/4c0368798880

Comment 13

[John Bieling (:TbSync)]

Comment on attachment 9189104 [details] [diff] [review]
bug1675940.patch

[Approval Request Comment]
User impact if declined:
windows API will not work as documented

Testing completed (on c-c, etc.):
On comm-esr78
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=209ca90561fa01eb55c142712d5ce421fb950a0a

Risk to taking this patch (and alternatives if risky):
I hope not.

Comment 14

[Wayne Mery (:wsmwk)]

Comment on attachment 9189104 [details] [diff] [review]
bug1675940.patch

[Triage Comment]
Approved for esr78 (with correct flag)

Comment 15

[Rob Lemley [:rjl]]

Thunderbird 78.6.0:
https://hg.mozilla.org/releases/comm-esr78/rev/3f233a584196