946015 - Use invisible docshells instead of hidden iframes.

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, defect)

Comment 1

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1317

Pointer to Github pull-request

Comment 2

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Gabor, so I made another attempt to switch to invisible docshells (see attached pull request), but I'm
afraid failed once again as some of our test keep failing :(

The reasons test fail is because we use addon/window as a host for other iframes. To observe document
state changes we set "load" and "DOMContentLoaded" event listeners to the addon/window and all the
nested iframe events propagate to it. Unfortunately this no longer seems to be a case with window from
the invisible docshell though.

This probably won't be a big deal once  Bug 843910 is landed, since those events will be dispatched
on observer service & we'll be able to use that instead (or maybe not, see paragraph below). 

In addition we also observe `document-element-inserted` notifactions for a similar reasons, but it looks
like they are not dispatched for iframes injected into invisible docshell either.

We need to resolve this issues before we'll be able to switch to invisible docshells.

Comment 3

[Gabor Krizsanits (INACTIVE)]

This is an awesome test for invisible docshells! Unfortunately I'm a bit overloaded right now. But once I get some time I will fix these bugs. I will leave the need info flag here so I won't forget about it.

Comment 4

[Gabor Krizsanits (INACTIVE)]

(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #2)
> The reasons test fail is because we use addon/window as a host for other
> iframes. To observe document
> state changes we set "load" and "DOMContentLoaded" event listeners to the
> addon/window and all the
> nested iframe events propagate to it. Unfortunately this no longer seems to
> be a case with window from
> the invisible docshell though.

What is the problem exactly? I tried to give it a try an reproduce it in scratchpad but docShell.chromeEventHandler is null... is that the problem? Or you created an intermittent iframe like in the original version for the hidden window and that didn't work for some reason? Do you have a scratchpad example for this that you expect to work but does not?

> 
> This probably won't be a big deal once  Bug 843910 is landed, since those
> events will be dispatched
> on observer service & we'll be able to use that instead (or maybe not, see
> paragraph below). 

This part seems to work...

> 
> In addition we also observe `document-element-inserted` notifactions for a
> similar reasons, but it looks
> like they are not dispatched for iframes injected into invisible docshell
> either.

So is this the main issue that needs to be fixed now? That you cannot find any equivalent on nsIWebProgressListener for `document-element-inserted`?

Comment 5

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #4)
> (In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment
> #2)
> > The reasons test fail is because we use addon/window as a host for other
> > iframes. To observe document
> > state changes we set "load" and "DOMContentLoaded" event listeners to the
> > addon/window and all the
> > nested iframe events propagate to it. Unfortunately this no longer seems to
> > be a case with window from
> > the invisible docshell though.
> 
> What is the problem exactly? I tried to give it a try an reproduce it in
> scratchpad but docShell.chromeEventHandler is null... is that the problem?
> Or you created an intermittent iframe like in the original version for the
> hidden window and that didn't work for some reason? Do you have a scratchpad
> example for this that you expect to work but does not?
> 
> > 
> > This probably won't be a big deal once  Bug 843910 is landed, since those
> > events will be dispatched
> > on observer service & we'll be able to use that instead (or maybe not, see
> > paragraph below). 
> 
> This part seems to work...
> 
> > 
> > In addition we also observe `document-element-inserted` notifactions for a
> > similar reasons, but it looks
> > like they are not dispatched for iframes injected into invisible docshell
> > either.
> 
> So is this the main issue that needs to be fixed now? That you cannot find
> any equivalent on nsIWebProgressListener for `document-element-inserted`?

So main issue is that `document-element-inserted` and similar document state change
related observer notifications aren't dispatched for invisible docshell documents
and all the nested (loaded in iframe) documents.
nested

Comment 6

[Erik Vold [:erikvold] (please needinfo? me)]

Hey Irakli, are you working on this one? if not could you please unassign yourself.

Comment 7

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #6)
> Hey Irakli, are you working on this one? if not could you please unassign
> yourself.

I can't work on this until platform issue it depends on is resolved.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Can you provide more details on which platform issues you're waiting on? Bug numbers would be great, since all the bug numbers referenced in this bug so far appear to have been fixed already.

Comment 9

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #8)
> Can you provide more details on which platform issues you're waiting on? Bug
> numbers would be great, since all the bug numbers referenced in this bug so
> far appear to have been fixed already.


Comment 5 contains description of the issue:
https://bugzilla.mozilla.org/show_bug.cgi?id=946015#c5

There is no separate bug for it as far as I can tell. We can create one if you like.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Yes, please do. Nobody's going to fix it if the issue is buried in the middle of an add-on sdk bug.

Comment 11

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562