Closed Bug 946015 Opened 8 years ago Closed 4 years ago

Use invisible docshells instead of hidden iframes.

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: irakli, Assigned: irakli)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
Assignee: nobody → rFobic
Summary: Use invisible docshells instead of iframes hidden iframes. → Use invisible docshells instead of hidden iframes.
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.
Flags: needinfo?(gkrizsanits)
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.
(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`?
Flags: needinfo?(gkrizsanits) → needinfo?(rFobic)
(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
Flags: needinfo?(rFobic)
Hey Irakli, are you working on this one? if not could you please unassign yourself.
Flags: needinfo?(rFobic)
(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.
Flags: needinfo?(rFobic)
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.
Flags: needinfo?(rFobic)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.
Flags: needinfo?(rFobic)
Yes, please do. Nobody's going to fix it if the issue is buried in the middle of an add-on sdk bug.
Flags: needinfo?(rFobic)
Flags: needinfo?(rFobic)
Priority: P3 → P1
Priority: P1 → --
https://bugzilla.mozilla.org/show_bug.cgi?id=1399562
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.