Closed
Bug 1497921
Opened 6 years ago
Closed 6 years ago
Make ExtensionControlledPopup wait for the window to be focused + activated before trying to show the popup
Categories
(WebExtensions :: Frontend, enhancement)
WebExtensions
Frontend
Tracking
(firefox64 fixed)
RESOLVED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox64 | --- | fixed |
People
(Reporter: Gijs, Assigned: Gijs)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
+++ This bug was initially created as a clone of Bug #1491998 +++
14:50:58 <Gijs> so, in other news... I've found a non-test issue with webextensions in the about-blank-reduction quest
14:51:14 <Gijs> specifically, we open the homepage-controlled-by-an-addon notification popup when the new window opens
14:51:20 <Gijs> but we don't wait for it to be ready for that to happen
14:51:27 <Gijs> so the openPopup call can fail to open the popup
14:51:57 <Gijs> what's the right place to make that code wait? Can we just do it inside ExtensionControlledPopup, or are there cases where we dont' want to wait for windows to have been focused + activated before attempting to show the popup?
16:22:23 <@aswan> Gijs: [...] As for ExtensionControlledPopup, i think it would be fine for it to always wait for the window to be focused
ExtensionControlledPopup needs to wait for the window to be focused + activated.
This is breaking browser_ext_chrome_settings_overrides_home.js once we get rid of some about:blank loads.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•6 years ago
|
||
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7bd7b375eb2e
make sure the window in which we try to show the ExtensionControlledPopup can show popups, r=aswan
Comment 3•6 years ago
|
||
Backed out changeset 7bd7b375eb2e (Bug 1497921) per developer's request CLOSED TREE
Flags: needinfo?(gijskruitbosch+bugs)
Backout by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b9222b391f89
Backed out changeset 7bd7b375eb2e per developer's request CLOSED TREE
Assignee | ||
Comment 5•6 years ago
|
||
The problem seems to be that in the non-remote case, we can focus a child window on mac/windows, and that throws things off. Fix pending on try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=029539954653cd459a6dadb35c3a32ef838eb51e
Flags: needinfo?(gijskruitbosch+bugs)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/757e7b60b9ff
make sure the window in which we try to show the ExtensionControlledPopup can show popups, r=aswan
Comment 7•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Is manual testing required on this bug? If yes, please provide some STR and the proper extension(if required) or set the “qe-verify -“ flag.
Thanks!
Flags: needinfo?(gijskruitbosch+bugs)
Assignee | ||
Comment 9•6 years ago
|
||
I don't think so, but let's doublecheck with :aswan.
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(aswan)
Comment 10•6 years ago
|
||
Yeah, I don't think there's a need for manual testing here, automated tests cover the "normal" case of a window opening in the foreground so we can feel confident that that didn't regress due to this patch.
Flags: needinfo?(aswan)
Assignee | ||
Updated•6 years ago
|
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•