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

Status: RESOLVED FIXED

(WebExtensions :: Frontend, enhancement)

Description

[:Gijs (he/him)]

+++ 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.

Comment 1

[:Gijs (he/him)]

Attachment: Bug 1497921 - make sure the window in which we try to show the ExtensionControlledPopup can show popups, r?aswan

Comment 2

[Pulsebot]

Pushed by [e-mail]:
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

[Stefan Hindli [:stefan_hindli]]

Backed out changeset 7bd7b375eb2e (Bug 1497921) per developer's request CLOSED TREE

Comment 4

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9222b391f89
Backed out changeset 7bd7b375eb2e per developer's request CLOSED TREE

Comment 5

[:Gijs (he/him)]

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
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

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/757e7b60b9ff

Comment 8

[CosminB]

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!

Comment 9

[:Gijs (he/him)]

I don't think so, but let's doublecheck with :aswan.

Comment 10

[Andrew Swan [: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.