Bug 1882776 Comment 6 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Steve P from comment #5)
> Hey Gijs, 
> 
> I wanted to get your input before going too far in the wrong direction. I've separated out `PopupBlockerObserver` to a `sys.mjs` file, added it to the list on `moz.build`, and imported it into `browser.js`. 
> 
> Looking at the Downloads example you gave, it looks like they have a file that uses Integration, and another that calls it. I'm thinking I need the `PopupBlockerObserver.sys.mjs` to import and use Integration, and then instead of importing it like I have it, in `browser.js` I call it like you have in this example? https://searchfox.org/mozilla-central/rev/b3f40fd7c4671537ed29a232e76c962977650045/browser/components/downloads/DownloadsViewableInternally.sys.mjs#70

Unfortunately I don't think this is quite right.

However, having thought about this a bit more, I think I've over-complicated the suggested design. Sorry!

Instead, I think the patch you have, minus the `Integration` parts, can work, and we can simply use the `browser/` module you've created and update this callsite:

https://searchfox.org/mozilla-central/rev/c84d3db8d7d6503b1208a0378db640095e106355/browser/base/content/browser-init.js#184

to use:

```js
    gBrowser.addEventListener("DOMUpdateBlockedPopups", e => gPopupBlockerObserver.handleEvent(e));
```

This would mean that we don't load the module until the event is first fired.

We'd want to include the observer in the `ChromeUtils.defineESModuleGetters` definition [here](https://searchfox.org/mozilla-central/rev/c84d3db8d7d6503b1208a0378db640095e106355/browser/base/content/browser.js#19) rather than importing it directly at the top of browser.js as your PoC patch has done.

Then, we'll want to make sure that the exported variable in the module is `PopupBlockerObserver` (minus the `g`) and update the various references...

Hopefully this helps unblock you for now? Don't hesitate to ask more questions if this wasn't clear.
(In reply to Steve P from comment #5)
> Hey Gijs, 
> 
> I wanted to get your input before going too far in the wrong direction. I've separated out `PopupBlockerObserver` to a `sys.mjs` file, added it to the list on `moz.build`, and imported it into `browser.js`. 
> 
> Looking at the Downloads example you gave, it looks like they have a file that uses Integration, and another that calls it. I'm thinking I need the `PopupBlockerObserver.sys.mjs` to import and use Integration, and then instead of importing it like I have it, in `browser.js` I call it like you have in this example? https://searchfox.org/mozilla-central/rev/b3f40fd7c4671537ed29a232e76c962977650045/browser/components/downloads/DownloadsViewableInternally.sys.mjs#70

Unfortunately I don't think this is quite right.

However, having thought about this a bit more, I think I've over-complicated the suggested design. Sorry!

Instead, I think the patch you have, minus the `Integration` parts, can work, and we can simply use the `browser/` module you've created and update this callsite:

https://searchfox.org/mozilla-central/rev/c84d3db8d7d6503b1208a0378db640095e106355/browser/base/content/browser-init.js#184

to use:

```js
    gBrowser.addEventListener("DOMUpdateBlockedPopups", e => PopupBlockerObserver.handleEvent(e));
```

This would mean that we don't load the module until the event is first fired.

We'd want to include the observer in the `ChromeUtils.defineESModuleGetters` definition [here](https://searchfox.org/mozilla-central/rev/c84d3db8d7d6503b1208a0378db640095e106355/browser/base/content/browser.js#19) rather than importing it directly at the top of browser.js as your PoC patch has done.

Then, we'll want to make sure that the exported variable in the module is `PopupBlockerObserver` (minus the `g`) and update the various references...

Hopefully this helps unblock you for now? Don't hesitate to ask more questions if this wasn't clear.

Back to Bug 1882776 Comment 6