Bug 1717603 Comment 11 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 :Gijs (he/him) from comment #10)
> > 
> > I don't really understand. What currently takes care of adding this script to *new* windows, that are created after startup?
> 
> Actually, it looks like this uses a window mediator listener, see https://searchfox.org/mozilla-central/rev/70a94bffbb73d2b0ba751fb3905428fdbcd4d402/toolkit/components/formautofill/FormAutofillParent.jsm#97 .
> 

Yes, as you commented. We use window mediator listener for newly created window, and we also [iterate though all the windows to inject script to the existing window](https://searchfox.org/mozilla-central/rev/70a94bffbb73d2b0ba751fb3905428fdbcd4d402/toolkit/components/formautofill/FormAutofillParent.jsm#94-96). The problem of this issue is we do have an existing window but the window doesn't have a DOM window yet (so the window mediator `onOpenWindow` is not triggred in this case).

> The simplest way of doing the tracking is via browser/modules/BrowserWindowTracker.jsm , which has guarantees that a window always ends up being tracked. But that's a browser module, and for some reason form autofill lives in toolkit? I don't really know why that is, or if we use this on android or any other non-desktop-firefox consumers.

Yes, it was moved to toolkit to support geckoview in Bug 1691821
> 
> If we can't use BrowserWindowTracker, I would make the enumerator code use `""` as the windowtype (so you get all windows) and then checking if the window is loaded, and if not, add an event listener for that. See https://searchfox.org/mozilla-central/rev/70a94bffbb73d2b0ba751fb3905428fdbcd4d402/toolkit/components/extensions/parent/ext-tabs-base.js#1484-1493 for how the extensions code itself does this.

I also tried this approach before asking the question, but somehow I can't get any window when using `""` as the windowtype (maybe I didn't use it correctly). I didn't dig deeper to see what caused the problem.  Since you also suggested this, I'll try this approach again, thanks!

> If this is implemented as an extension, perhaps a simpler option is to use the webextension window APIs instead?

okay, I'll also give this a try. Thanks!
(In reply to :Gijs (he/him) from comment #10)
> > 
> > I don't really understand. What currently takes care of adding this script to *new* windows, that are created after startup?
> 
> Actually, it looks like this uses a window mediator listener, see https://searchfox.org/mozilla-central/rev/70a94bffbb73d2b0ba751fb3905428fdbcd4d402/toolkit/components/formautofill/FormAutofillParent.jsm#97 .
> 

Yes, as you commented. We use window mediator listener for newly created window, and we also [iterate though all the windows to inject script to the existing window](https://searchfox.org/mozilla-central/rev/70a94bffbb73d2b0ba751fb3905428fdbcd4d402/toolkit/components/formautofill/FormAutofillParent.jsm#94-96). The problem of this issue is we have an existing window but the window doesn't have a DOM window yet (the window mediator `onOpenWindow` is not triggred in this case because it is already opened while registering the listener).

> The simplest way of doing the tracking is via browser/modules/BrowserWindowTracker.jsm , which has guarantees that a window always ends up being tracked. But that's a browser module, and for some reason form autofill lives in toolkit? I don't really know why that is, or if we use this on android or any other non-desktop-firefox consumers.

Yes, it was moved to toolkit to support geckoview in Bug 1691821
> 
> If we can't use BrowserWindowTracker, I would make the enumerator code use `""` as the windowtype (so you get all windows) and then checking if the window is loaded, and if not, add an event listener for that. See https://searchfox.org/mozilla-central/rev/70a94bffbb73d2b0ba751fb3905428fdbcd4d402/toolkit/components/extensions/parent/ext-tabs-base.js#1484-1493 for how the extensions code itself does this.

I also tried this approach before asking the question, but somehow I can't get any window when using `""` as the windowtype (maybe I didn't use it correctly). I didn't dig deeper to see what caused the problem.  Since you also suggested this, I'll try this approach again, thanks!

> If this is implemented as an extension, perhaps a simpler option is to use the webextension window APIs instead?

okay, I'll also give this a try. Thanks!
(In reply to :Gijs (he/him) from comment #10)
> > 
> > I don't really understand. What currently takes care of adding this script to *new* windows, that are created after startup?
> 
> Actually, it looks like this uses a window mediator listener, see https://searchfox.org/mozilla-central/rev/70a94bffbb73d2b0ba751fb3905428fdbcd4d402/toolkit/components/formautofill/FormAutofillParent.jsm#97 .
> 

Yes, as you commented. We use window mediator listener for newly created window, and we also [iterate though all the windows to inject script to the existing window](https://searchfox.org/mozilla-central/rev/70a94bffbb73d2b0ba751fb3905428fdbcd4d402/toolkit/components/formautofill/FormAutofillParent.jsm#94-96). The problem of this issue is we have an existing window but the window doesn't have a DOM window yet (the window mediator `onOpenWindow` is not triggred in this case because the window is already opened while registering the listener).

> The simplest way of doing the tracking is via browser/modules/BrowserWindowTracker.jsm , which has guarantees that a window always ends up being tracked. But that's a browser module, and for some reason form autofill lives in toolkit? I don't really know why that is, or if we use this on android or any other non-desktop-firefox consumers.

Yes, it was moved to toolkit to support geckoview in Bug 1691821
> 
> If we can't use BrowserWindowTracker, I would make the enumerator code use `""` as the windowtype (so you get all windows) and then checking if the window is loaded, and if not, add an event listener for that. See https://searchfox.org/mozilla-central/rev/70a94bffbb73d2b0ba751fb3905428fdbcd4d402/toolkit/components/extensions/parent/ext-tabs-base.js#1484-1493 for how the extensions code itself does this.

I also tried this approach before asking the question, but somehow I can't get any window when using `""` as the windowtype (maybe I didn't use it correctly). I didn't dig deeper to see what caused the problem.  Since you also suggested this, I'll try this approach again, thanks!

> If this is implemented as an extension, perhaps a simpler option is to use the webextension window APIs instead?

okay, I'll also give this a try. Thanks!
(In reply to :Gijs (he/him) from comment #10)
> > 
> > I don't really understand. What currently takes care of adding this script to *new* windows, that are created after startup?
> 
> Actually, it looks like this uses a window mediator listener, see https://searchfox.org/mozilla-central/rev/70a94bffbb73d2b0ba751fb3905428fdbcd4d402/toolkit/components/formautofill/FormAutofillParent.jsm#97 .
> 

Yes, as you commented. We use window mediator listener for newly created window, and we also [iterate though all the windows to inject script to the existing window](https://searchfox.org/mozilla-central/rev/70a94bffbb73d2b0ba751fb3905428fdbcd4d402/toolkit/components/formautofill/FormAutofillParent.jsm#94-96). The problem of this issue is we have an existing window but the window doesn't have a DOM window yet (the window mediator `onOpenWindow` is not triggred in this case because the window is already opened while registering the listener).

> The simplest way of doing the tracking is via browser/modules/BrowserWindowTracker.jsm , which has guarantees that a window always ends up being tracked. But that's a browser module, and for some reason form autofill lives in toolkit? I don't really know why that is, or if we use this on android or any other non-desktop-firefox consumers.

Yes, it was moved to toolkit to support geckoview in Bug 1691821
> 
> If we can't use BrowserWindowTracker, I would make the enumerator code use `""` as the windowtype (so you get all windows) and then checking if the window is loaded, and if not, add an event listener for that. See https://searchfox.org/mozilla-central/rev/70a94bffbb73d2b0ba751fb3905428fdbcd4d402/toolkit/components/extensions/parent/ext-tabs-base.js#1484-1493 for how the extensions code itself does this.

I did try this approach before asking the question, but somehow I can't get any window when using `""` as the windowtype (maybe I didn't use it correctly). I didn't dig deeper to see what caused the problem.  Since you also suggested this, I'll try this approach again, thanks!

> If this is implemented as an extension, perhaps a simpler option is to use the webextension window APIs instead?

okay, I'll also give this a try. Thanks!

Back to Bug 1717603 Comment 11