Make ExtensionControlledPopup wait for the window to be focused + activated before trying to show the popup

RESOLVED FIXED in Firefox 64

Status

enhancement
RESOLVED FIXED
7 months ago
6 months ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

(Blocks 1 bug)

unspecified
mozilla64
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 months ago
+++ 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

7 months ago
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED

Comment 2

7 months 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
Backed out changeset 7bd7b375eb2e (Bug 1497921) per developer's request CLOSED TREE
Flags: needinfo?(gijskruitbosch+bugs)

Comment 4

7 months ago
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

7 months 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)

Comment 6

7 months ago
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 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/757e7b60b9ff
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64

Comment 8

6 months ago
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 months ago
I don't think so, but let's doublecheck with :aswan.
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(aswan)
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 months ago
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.