891008 - Simplify SDK panel architecture

Status: RESOLVED INCOMPLETE

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

Description

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

Right now SDK panels are implemented via:
https://developer.mozilla.org/en-US/docs/XUL/panel 

In a XUL panel we create an iframe and load panel document into it.

This architecture has a few weak spots:

1. Implementation vise XUL Panel is attached to a browser window, although
   API vise it's not, it's more of a independent view that can be displayed
   over specific windows. In order to make all this work we keep moving XUL
   panel element across different browser windows.

2. Panel document state is persistent across show's and hide's but then moving
   XUL panel across documents will destroy document in iframe. To overcome this
   we use swapFrameLoaders trick:
   https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIFrameLoaderOwner#swapFrameLoaders%28%29

3. In order to be able to persist document and use swapFrameLoaders we have another
   hidden frame where panel document is contained when panel is hidden, which is
   also when it's being moved accross windows.

4. swapFrameLoaders works only with XUL iframes, but then it's not trivial to create
   xul hidden iframes, specially in cross platform way.

5. To much document joggling & moving parts to make this work also moving parts are
   source for bugs.

6. Things like "private mode panels" become almost impossible since it adds yet
   another ball to joggle crossing a limit.

Comment 1

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

I've submitted this bug so we could revisit our current architecture and see if we maybe fix platform to avoid series of workarounds or come up with something entirely new to avoid a lot of complexity.

Comment 2

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

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #1)
> I've submitted this bug so we could revisit our current architecture and see
> if we maybe fix platform to avoid series of workarounds or come up with
> something entirely new to avoid a lot of complexity.

It's very unclear to me what's being asked for in this bug.  The flaws with the current Panel are well described however.  Irakli can you make a JEP for your proposal please?

Comment 3

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

Erik I don't actually know exactly what should we do that's why I don't have a JEP around yet, but I wanted to submit a bug and maybe get it into roadmap as tech dept so we can look into better solutions.

Comment 4

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

Neil I heard you know the most about XUL panels so maybe you can help us out with some useful input. Currently our main problem is that panels has to be associated with a specific browser window, while what we need is an application global panel that we can display over specific windows. Hacks we use currently to make it work are described in the bug description, but hopefully we can get rid of those. With that here are my questions:

1. Is there any way we could implement panels that won't be bound to a specific browser window ? Maybe as an XPCOM component or something alike.

2. If above is no, could we use hidden window to achieve that ? I did some testing on this and it seems to work on OSX, but has issues on other platforms caused by the fact that hidden window hold non XUL documents. Maybe there's some way to work around that.

3. Could panels be created in an invisible docShells & used for creating panels that we could overlay windows ? In my tests this does not works now, presumably
because document holding panel is not a top level window. But maybe we can change that ?

Gabor I think you'll end up looking into this, so I'm cc-ing you on this too.

Comment 5

[Neil Deakin]

You shouldn't use a panel and instead just create a different window and open it without any window chrome on it.

Comment 6

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

Is there actually a way to make a window with an arrow pointers & any chrome ? I'm afraid that attempt to using just a window will cause a new set of problems & it won't necessary be easier to fix them.

Comment 7

[Gabor Krizsanits (INACTIVE)]

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #4)

> This architecture has a few weak spots:

This looks terrible. I'm happy to help doing something remotely sane instead of all these hacks.

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #6)
> I'm afraid that attempt to using just a window will cause a new set of
> problems & it won't necessary be easier to fix them.

Panels as far as I can tell are really not designed for what we're using them for. I'd be glad to just get away from them and find a cleaner solution, instead of adding new hacks to it, it's already scary what we do with them.

Invisible docshell is in an experimental state, no one has a clue what kind of random issues/crashes will it introduce if we try to throw it in here.

First I would really really like to get away from this swapFrameLoaders magic. So I would be glad to give a try to what Neil suggest and and use a chromeless window and see what kind of issues we might have to face with.

> Is there actually a way to make a window with an arrow pointers & any chrome

I'm not sure I get it...

> ? I'm afraid that attempt to using just a window will cause a new set of
> problems & it won't necessary be easier to fix them.

Maybe... personally, even if those issues are somewhat harder to fix, I would not mind if it results a proper solution that does what we need instead of hacking panels further. We might also end up with a panel of our own, I need to learn more about this problem to tell.

Comment 8

[Gabor Krizsanits (INACTIVE)]

So, I've been reading panel code both on platform and on SDK side. Let's get some minimal examples out of this. 

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #0)
> Right now SDK panels are implemented via:
> https://developer.mozilla.org/en-US/docs/XUL/panel 
> 
> In a XUL panel we create an iframe and load panel document into it.
> 
> This architecture has a few weak spots:
> 
> 1. Implementation vise XUL Panel is attached to a browser window, although
>    API vise it's not, it's more of a independent view that can be displayed
>    over specific windows. In order to make all this work we keep moving XUL
>    panel element across different browser windows.
> 

Now let's start from here. This is the root of all our problems. First of all I'm not sure it's not us that doing that attachment here: http://mxr.mozilla.org/mozilla-central/source/addon-sdk/source/lib/sdk/panel/utils.js?force=1#217

So if the SDK does not attach the panel to any window, what is our problem then exactly? What does not work exactly in a minimal example (without the SDK around)? I think that an unattached panel should be able to be shown on top of any window just fine, and according to my experiments it works that way. Can you provide me an example that you would like to work and does not? I really don't see why do we have to move XUL panel across documents, and I think we should really avoid that.

Comment 9

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

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #8)
> So, I've been reading panel code both on platform and on SDK side. Let's get
> some minimal examples out of this. 
> 
> (In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from
> comment #0)
> > Right now SDK panels are implemented via:
> > https://developer.mozilla.org/en-US/docs/XUL/panel 
> > 
> > In a XUL panel we create an iframe and load panel document into it.
> > 
> > This architecture has a few weak spots:
> > 
> > 1. Implementation vise XUL Panel is attached to a browser window, although
> >    API vise it's not, it's more of a independent view that can be displayed
> >    over specific windows. In order to make all this work we keep moving XUL
> >    panel element across different browser windows.
> > 
> 
> Now let's start from here. This is the root of all our problems. First of
> all I'm not sure it's not us that doing that attachment here:
> http://mxr.mozilla.org/mozilla-central/source/addon-sdk/source/lib/sdk/panel/
> utils.js?force=1#217
> 
> So if the SDK does not attach the panel to any window, what is our problem
> then exactly? What does not work exactly in a minimal example (without the
> SDK around)? I think that an unattached panel should be able to be shown on
> top of any window just fine, and according to my experiments it works that
> way. Can you provide me an example that you would like to work and does not?
> I really don't see why do we have to move XUL panel across documents, and I
> think we should really avoid that.

That's interesting point, I think it was just a general assumption that any node
needs an owner document in order to function properly (at least that's how DOM
APIs tend to work in HTML).

If panel works without an owner document that would indeed solve most of our problems!!

Only immediate questions I have would be:

1. Which document should we use to create that XUL panel ? Right now
   we just use most recently focused one.
2. What's going to happen if window containing document we used to create
   XUL panel with is closed ? Well panel still function ? Will window be
   GC-ed properly ?

If we could have used hidden window's document to create a panel, that may
have reasonable answers for above questions.

Zer0 said he'll give it a try to see if it works

Comment 10

[Gabor Krizsanits (INACTIVE)]

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #9)
> If panel works without an owner document that would indeed solve most of our
> problems!!

As a node it should have an ownerDocument, just it's not attached to it. Like a detached DOM node.

> 1. Which document should we use to create that XUL panel ? Right now
>    we just use most recently focused one.

Yeah... this is the question. I'm not sure about the lifetime of panel after it's ownerDocument is gone, I will try to look into this. So what privilege should panel have? If it's chrome and belongs to the add-on not to the content as I would think, the add-on window we have should be the best. So yeah, let's try that. Without attaching the panel to anything.

> 2. What's going to happen if window containing document we used to create
>    XUL panel with is closed ? Well panel still function ? Will window be
>    GC-ed properly ?

I will look into this. But it can kill the panel, or the panel can keep the document alive, both would be bad. Maybe it just works, I will read some code, also Matteo should try it out if he has some time.
 
> If we could have used hidden window's document to create a panel, that may
> have reasonable answers for above questions.
> 
> Zer0 said he'll give it a try to see if it works

Yeah, let's try this first. Matteo: keep in mind that you should prevent the SDK to attach the panel to any documents.

Comment 11

[Matteo Ferretti [:zer0] [:matteo]]

Attachment: example of creating and display a panel, with minimum use of SDK

Comment 12

[Matteo Ferretti [:zer0] [:matteo]]

So, it seems that panel needs to be appended to some document, otherwise it won't work. It seems also that it works if append directly to the `hiddenWindow`, but not if it's appended in the addon's window, that is an iframe of `hiddenWindow`.

Comment 13

[Firefox Bug Husbandry Bot]

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