1799347 - Consider `browserAction.openPopup` behaviour when a popup is already open

Status: RESOLVED FIXED

(WebExtensions :: General, task, P3)

Description

[[e-mail]]

In https://phabricator.services.mozilla.com/D139796 I have been updating the browserAction.openPopup API. One question which came up as part of this is what the behaviour should be when a popup (either from the same addon or another) is already open.

Currently:

• If the popup is already open for the same addon in a given window, that window is simply focused and the call silently returns.
• If another addon has a popup open, the popup for the addon calling the API is opened in addition to this.

In each case we have a few different options:

• Focus the window and show the popup if it is not already showing.
• Do nothing.
• Throw an exception.

In the case where the current addon already has a popup open in the given window, my initial preference is to simply focus the window. This matches the behaviour in Safari (https://github.com/w3c/webextensions/issues/160#issuecomment-1127042759). It also makes the most sense to me because without an easy way of detecting if the popup is open (you can use getViews but that's a little hidden) we otherwise risk lots of "errors" in background logs for something which isn't really causing any harm. Chrome currently does throw an error but I suspect this is a bug, since it is not about the popup already being open but instead that there is no active window to show the popup for.

In the case where another addon has a popup open, I suggest that is closed. Arguably this could lead to data loss but I feel that avoiding stacking is desirable and there is already an understanding that the popup can close at any time because this happens as soon as the user focuses another window.

I plan to update this bug soon with more information about the behaviour when multiple addons are showing popups across browsers.

Comment 1

[[e-mail]]

I've updated the WECG issue with some more information on this.

In Chrome, this always throws. If no windowId is specified the error is about there being no active window. If a windowId is specified, there is instead an error about failing to open the popup.

In Safari, any popups from other extensions are closed and the extension calling the API has its popup shown.

Comment 2

[Rob Wu [:robwu]]

If something unambiguously meaningful can be done, then we should do that. But otherwise we should throw IMO.

I'll continue the discussion in the WECG issue.

Comment 4

[Rob Wu [:robwu]]

Attachment: Bug 1799347 - Reject openPopup() if already open r=rpl

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/794935504471
https://hg.mozilla.org/integration/autoland/rev/e812222966a3
Reject openPopup() if already open r=rpl,geckoview-reviewers,ohall

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/febc17c38165
https://hg.mozilla.org/integration/autoland/rev/21aa86a031e2
Reject openPopup() if already open r=rpl,geckoview-reviewers,ohall

Comment 7

[Atila Butkovits]

Backed out for cauisng failures at test_ext_action_openPopup_multiple.html.

Backout link: https://hg-edge.mozilla.org/integration/autoland/rev/f98e369bc86f5d8949f40597b06907421ae76857

Push where failures started: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=43b25e0614b5e27fcc88a04f28c3bb644d5773d2&selectedTaskRun=eCZ3kH9nSWO5_XlSmttMvw.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=548246737&repo=autoland&task=eCZ3kH9nSWO5_XlSmttMvw.0&lineNumber=5815

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ca68dfe2a8db
https://hg.mozilla.org/integration/autoland/rev/c8cc6ed0d771
Reject openPopup() if already open r=rpl,geckoview-reviewers,ohall

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/c8cc6ed0d771