890635 - Need some way to support private panels

Status: RESOLVED WONTFIX

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

Description

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

Take an extension like https://github.com/erikvold/page-portals-jetpack for example, I cannot support private browsing with the current Panel API.

Comment 1

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

Erik do you wanna bring this up at weekly meeting or better write up a JEP with a proposed API changes to panel to address this ?

Comment 2

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

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #1)
> Erik do you wanna bring this up at weekly meeting or better write up a JEP
> with a proposed API changes to panel to address this ?

I had a plan that we agreed to a while ago that we threw out of the window already, I'll leave this to you now, I have another project to work on.

Comment 3

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

Here are the options that I can see:

* Panels have two views, one for private windows, another for non-private windows 
** Note: this was the original idea I had that we all agreed to before throwing it out of the window for some reason that I never understood.
** Note: this would now be a breaking change for developers that use panels in private windows, because we didn't take the opportunity to implement this when it would not have been (the introduction of the private-browsing permission flag).


* Panels have multiple views, one per window, or one per show
** Note: this would be a breaking change

* Somehow create a panel has one view which can somehow mutate between private and non-private states
** Note: this seems very hard

* Create a new PrivatePanel class, or add a `private` constructor property to the Panel class which has a single persistent private view.
** This means if a developer cannot reuse a panel instance across private/non-private windows and will have to create two instances and do the juggling manually.

* any other ideas?

Give me your votes and I'll write a JEP..

Comment 4

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

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #3)
> Here are the options that I can see:
>
> * Panels have multiple views, one per window, or one per show
> ** Note: this would be a breaking change

Oh right, an alternate way to do this one would be to create a new class, like `Portal`

Comment 5

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #3)
> Here are the options that I can see:
> 
> * Panels have two views, one for private windows, another for non-private
> windows 
> ** Note: this was the original idea I had that we all agreed to before
> throwing it out of the window for some reason that I never understood.
> ** Note: this would now be a breaking change for developers that use panels
> in private windows, because we didn't take the opportunity to implement this
> when it would not have been (the introduction of the private-browsing
> permission flag).

I wouldn't worry too much about that group, there aren't very many of them this early.

> * any other ideas?

I think what your real problem is, is that remote content is loaded into a panel in a private window incorrectly. It seems like the simplest way to provide that is your first proposal?

Comment 6

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

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #3)
> Here are the options that I can see:
> 
> * Panels have two views, one for private windows, another for non-private
> windows 
> ** Note: this was the original idea I had that we all agreed to before
> throwing it out of the window for some reason that I never understood.
> ** Note: this would now be a breaking change for developers that use panels
> in private windows, because we didn't take the opportunity to implement this
> when it would not have been (the introduction of the private-browsing
> permission flag).
> 

-1 on this, since there are to much magic going on under the hood. The reason it was thrown away is because it was making too many assumptions how panels are / going to be used. You can't switch from 1 to 1 model to 1 to n model and expect that everything is going to work fine. Also current panels aren't bound to any specific window and have persistent state, adding custom behavior to private windows does not seems to compose with this design.

I'm open to revisiting Panel design in fact I've submitted new bug 891008 to do that, but I'm really opposed to changing model in the existing API. If we wanna change 1 to 1 model with persistant state we should create new API similar to how new buttons and toolbars where added, trying to shoehorn on the current implementation is not a good option IMO.


> 
> * Panels have multiple views, one per window, or one per show
> ** Note: this would be a breaking change
> 

If we do that it should be a new API.

> * Somehow create a panel has one view which can somehow mutate between
> private and non-private states
> ** Note: this seems very hard
>

Too magical!!

> 
> * Create a new PrivatePanel class, or add a `private` constructor property
> to the Panel class which has a single persistent private view.
> ** This means if a developer cannot reuse a panel instance across
> private/non-private windows and will have to create two instances and do the
> juggling manually.
> 
> * any other ideas?
>

This is the one that makes most sense to me. Very likely users will have panels that are like ones we have now, not attached to any window and behave same regardless of window that is being focused. Or they may wanna create panels like you do with portals where you want private browsing mode panels. I think that case where you would want panel with same content in private or not mode depending on focused window will be quite exotic and it's ok to require little more work to cover these cases. Also we could probably some helper functions to simplify that case too.

Also since these panels will be `PrivatePanel`s they can be non persistent without causing any backwards compatibility issues.

> 
> Give me your votes and I'll write a JEP..

Comment 7

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

Creating a new API with 1 instance for many views makes the most sense to me fwiw.  Where the views are private or not private depending on the context.

Comment 8

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

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #7)
> Creating a new API with 1 instance for many views makes the most sense to me
> fwiw.  Where the views are private or not private depending on the context.

That implies we can decide when it should and should not be private. I don't think we can make such decision since that really depends on what add-on does. As we pointed out few times during private browsing discussions there are add-ons like 1password that don't change their behavior regardless of weather window is private or not.

As a firefox user I would also rather prefer to decide weather I want specific add-on to be aware of private windows or not instead of hoping that they do right thing.

Comment 9

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

(In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from comment #8)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #7)
> > Creating a new API with 1 instance for many views makes the most sense to me
> > fwiw.  Where the views are private or not private depending on the context.
> 
> That implies we can decide when it should and should not be private.
> I don't
> think we can make such decision since that really depends on what add-on
> does. As we pointed out few times during private browsing discussions there
> are add-ons like 1password that don't change their behavior regardless of
> weather window is private or not.


I did not intend to make such a implication nor do I think it.

> As a firefox user I would also rather prefer to decide weather I want
> specific add-on to be aware of private windows or not instead of hoping that
> they do right thing.

I want this too, I've been pushing for this since the start of this discussion..

I don't want to repeat myself any longer, so I'm going to make these my final words on the subject:

The SDK does not provide a means for which a Firefox user can use an add-on like Page Portals without allowing the add-on to violate their privacy..  At this point I don't care what the module owners decide that they want to implement to support this use case, just make it something and don't wait a year please.

Comment 10

[Dave Townsend [:mossop]]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #9)
> (In reply to Irakli Gozilalishvili [:irakli] [:gozala] [@gozala] from
> comment #8)
> > (In reply to Erik Vold [:erikvold] [:ztatic] from comment #7)
> > > Creating a new API with 1 instance for many views makes the most sense to me
> > > fwiw.  Where the views are private or not private depending on the context.
> > 
> > That implies we can decide when it should and should not be private.
> > I don't
> > think we can make such decision since that really depends on what add-on
> > does. As we pointed out few times during private browsing discussions there
> > are add-ons like 1password that don't change their behavior regardless of
> > weather window is private or not.
> 
> 
> I did not intend to make such a implication nor do I think it.
> 
> > As a firefox user I would also rather prefer to decide weather I want
> > specific add-on to be aware of private windows or not instead of hoping that
> > they do right thing.
> 
> I want this too, I've been pushing for this since the start of this
> discussion..
> 
> I don't want to repeat myself any longer, so I'm going to make these my
> final words on the subject:
> 
> The SDK does not provide a means for which a Firefox user can use an add-on
> like Page Portals without allowing the add-on to violate their privacy..  At
> this point I don't care what the module owners decide that they want to
> implement to support this use case, just make it something and don't wait a
> year please.

We discussed this in triage and decided that this simply isn't a common enough need for it to be worth us spending the time supporting right now.