action.openPopup() should reject if the window is not a foreground window
Categories
(WebExtensions :: General, enhancement, P2)
Tracking
(firefox149 fixed)
| Tracking | Status | |
|---|---|---|
| firefox149 | --- | fixed |
People
(Reporter: robwu, Assigned: robwu)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete, Whiteboard: [addons-jira])
Attachments
(2 files)
The action.openPopup() API currently focuses a window if it was not focused before.
This can result in surprising behavior, and is not supported by Chrome https://issuetracker.google.com/issues/345640558
We should reject with a new error message instead. Chrome's error message is as follows, for reference:
Cannot show popup for an inactive window. To show the popup for this window, first call `chrome.windows.update` with `focused` set to "true."
We'd want our error message to be more generic, because the recommendation of using focused:true would not work on Android (when implemented - bug 1817809). There, the extension has to make the relevant tab active instead (e.g. browser.tabs.update(tabId, { active: true }).
Removing this feature could also enable us to re-enable the tests on Linux (bug 1798334), where it is currently skipped for causing intermittent failures. If we don't need to depend on a particular sequence of window state changes, and only need to check for errors, the test becomes more reliable.
Updated•3 months ago
|
| Assignee | ||
Comment 1•3 months ago
|
||
Note: Safari's behavior is to show the minimized/background window, like Firefox does now (https://github.com/w3c/webextensions/issues/160#issuecomment-1127042759).
Since Chrome has shipped their behavior for so long without notable complaints, I expect that we'd be able to not automatically focus the window.
On the other hand, action.openPopup() can currently already be called with user interaction (e.g. shortcut), and that could potentially rely on the window getting focused. So I'd put the old behavior behind a pref, to allow us to re-enable support for automatic focusing of windows if needed.
| Assignee | ||
Updated•3 months ago
|
| Assignee | ||
Comment 2•3 months ago
|
||
The tests are for browserAction.openPopup, but the implementation is
identical between action.openPopup and browserAction.openPopup.
| Assignee | ||
Comment 4•2 months ago
|
||
As explained in bug 2015895, extension popups are not closed on Android
when an extension unloads. That is a pre-existing bug, and will cause
test failures when we start rejecting action.openPopup() calls when
another extension popup is present.
As a work-around, this patch explicitly closes popups that were opened
during the test. These can be removed when bug 2015895 is fixed.
Comment 7•2 months ago
|
||
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 9•2 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/99c5190c043a
https://hg.mozilla.org/mozilla-central/rev/b8e546c3acf5
| Assignee | ||
Updated•2 months ago
|
| Assignee | ||
Comment 10•1 month ago
|
||
dev-doc-needed:
I wanted to mention this in the action.openPopup() documentation, but https://github.com/mdn/content/pull/43370 was merged without review from me, so I'm requesting documentation here.
Before 149, action.openPopup could be passed a windowId to open the popup for a different window, and that window would be focused before the popup was opened. This behavior was removed in 149: the windowId (if passed) can only match the active window's ID. If the window is not focused, browser.windows.update(windowId, { focused: true }) needs to be called first.
This behavior is also what Chrome has implemented, so the primary article for action.openPopup can call out this behavior.
Description
•