825338 - Need a way to determine if per window private browsing is available

Status: RESOLVED WONTFIX

(Firefox :: Private Browsing, defect)

Description

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

I can't find an explicit way for add-ons to determine if per window private browsing is enabled or not.  The perfect place for such a flag would be resource://gre/modules/PrivateBrowsingUtils.jsm I would think.  Is there some other way to do this that I have missed?

Comment 1

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

I used to check that `chromeWindow.gPrivateBrowsingUI.privateWindow` existed, but it no longer seems to have been removed.

Comment 2

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

* `chromeWindow.gPrivateBrowsingUI.privateWindow` seems to have been removed.

Comment 3

[Josh Matthews [:jdm]]

Let me reply to your question with another one: how do you want to use this information?

Comment 4

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/message/reply/question/etc.)]

Maybe for addons that will be disabled when you will enter window with private browsing.

Comment 5

[(no longer active)]

With bug 826037, one possible way would be |("privateBrowsingEnabled" in Components.interfaces.nsIPrivateBrowsingService)|, but I suggest trying to avoid these types of check as much as possible.

Comment 6

[(no longer active)]

Or |("nsIPrivateBrowsingService" in Components.interfaces && "privateBrowsingEnabled" in Components.interfaces.nsIPrivateBrowsingService)| if you want forward-compatible code!

Comment 7

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

(In reply to :Ehsan Akhgari from comment #6)
> Or |("nsIPrivateBrowsingService" in Components.interfaces &&
> "privateBrowsingEnabled" in
> Components.interfaces.nsIPrivateBrowsingService)| if you want
> forward-compatible code!

is this still valid for FF 20 after bug 826037 ?

Comment 8

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

(In reply to :Ehsan Akhgari from comment #5)
> With bug 826037, one possible way would be |("privateBrowsingEnabled" in
> Components.interfaces.nsIPrivateBrowsingService)|, but I suggest trying to
> avoid these types of check as much as possible.

What would you suggest doing instead?  the SDK will need to support apps that use global pb, pwpb, ptpb, and nothing.

Comment 9

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

(In reply to Josh Matthews [:jdm] from comment #3)
> Let me reply to your question with another one: how do you want to use this
> information?

The SDK needs to support apps that use global pb, pwpb, ptpb, and nothing, so we need to do some feature detection afaict, let me know if you have a better approach in mind.

Comment 10

[(no longer active)]

(In reply to comment #9)
> (In reply to Josh Matthews [:jdm] from comment #3)
> > Let me reply to your question with another one: how do you want to use this
> > information?
> 
> The SDK needs to support apps that use global pb, pwpb, ptpb, and nothing, so
> we need to do some feature detection afaict, let me know if you have a better
> approach in mind.

Hmm, yeah maybe the SDK is special in that sense.  Note that there is no per-tab PB, we either have the old global PB service, or the new per-window PB service or neither (in case of other Mozilla-based apps.)

The correct check now would be something like: ("nsIPrivateBrowsingService" in Components.interfaces && "privateBrowsingEnabled" in Components.interfaces.nsIPrivateBrowsingService).  This will be future-proof as well, for when the service is completely removed.

Comment 11

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

(In reply to :Ehsan Akhgari from comment #10)
> (In reply to comment #9)
> > (In reply to Josh Matthews [:jdm] from comment #3)
> > > Let me reply to your question with another one: how do you want to use this
> > > information?
> > 
> > The SDK needs to support apps that use global pb, pwpb, ptpb, and nothing, so
> > we need to do some feature detection afaict, let me know if you have a better
> > approach in mind.
> 
> Hmm, yeah maybe the SDK is special in that sense.  Note that there is no
> per-tab PB, we either have the old global PB service, or the new per-window
> PB service or neither (in case of other Mozilla-based apps.)

I think mobile is going to support ptpb, there are a bunch of bugs about it at least, see bug 815897 dependencies.

> The correct check now would be something like: ("nsIPrivateBrowsingService"
> in Components.interfaces && "privateBrowsingEnabled" in
> Components.interfaces.nsIPrivateBrowsingService).  This will be future-proof
> as well, for when the service is completely removed.

Will this work in FF20 with bug 826037 ?

Comment 12

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

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #11)
> Will this work in FF20 with bug 826037 ?

Ah never mind, I updated my Nightly and see that this does work for FF20.

Comment 13

[(no longer active)]

(In reply to comment #11)
> (In reply to :Ehsan Akhgari from comment #10)
> > (In reply to comment #9)
> > > (In reply to Josh Matthews [:jdm] from comment #3)
> > > > Let me reply to your question with another one: how do you want to use this
> > > > information?
> > > 
> > > The SDK needs to support apps that use global pb, pwpb, ptpb, and nothing, so
> > > we need to do some feature detection afaict, let me know if you have a better
> > > approach in mind.
> > 
> > Hmm, yeah maybe the SDK is special in that sense.  Note that there is no
> > per-tab PB, we either have the old global PB service, or the new per-window
> > PB service or neither (in case of other Mozilla-based apps.)
> 
> I think mobile is going to support ptpb, there are a bunch of bugs about it at
> least, see bug 815897 dependencies.

Ah, I see.  Yeah I was only talking about desktop.  Makes sense.

Comment 14

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

Hmm so I think that ("nsIPrivateBrowsingService" in Components.interfaces && "privateBrowsingEnabled" in Components.interfaces.nsIPrivateBrowsingService) is always false.  So I'd need to check that nsIPrivateBrowsingService exists then create an instance of it and check that "privateBrowsingEnabled" is available.

This only tells me that the global service is not available tho, and not that per window private browsing is available and active.

Comment 15

[(no longer active)]

(In reply to comment #14)
> This only tells me that the global service is not available tho, and not that
> per window private browsing is available and active.

Ah, hmm yeah I confused myself.  Sorry.  You're right.

Comment 16

[(no longer active)]

This doesn't need to block bug 463027.  We already have a way to determine this.  I'm not sure what you want to happen in this bug.

Comment 17

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

(In reply to :Ehsan Akhgari from comment #16)
> This doesn't need to block bug 463027.  We already have a way to determine
> this.  I'm not sure what you want to happen in this bug.

I need a way to determine if pwpb is active at runtime without version checking.

Comment 18

[(no longer active)]

(In reply to comment #17)
> (In reply to :Ehsan Akhgari from comment #16)
> > This doesn't need to block bug 463027.  We already have a way to determine
> > this.  I'm not sure what you want to happen in this bug.
> 
> I need a way to determine if pwpb is active at runtime without version
> checking.

Can't you just instantiate the service and see if it has a privateBrowsingEnabled property on it (after QIing to nsIPrivateBrowsingService, of course), and put it all in a try catch block to make sure that it's future proof?  Something like:

function isGlobalPBAvailable() {
  var globalPB = false;
  try {
    var pbSvc = Components.classes["@mozilla.org/privatebrowsing;1"].
                getService(Components.interfaces.nsIPrivateBrowsingService);
    globalPB = ("privateBrowsingEnabled" in pbSvc);
  } catch (e) {}
  return globalPB;
}

(I tested the snippet this time.  :-)

Comment 19

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

(In reply to :Ehsan Akhgari from comment #18)
> (In reply to comment #17)
> > (In reply to :Ehsan Akhgari from comment #16)
> > > This doesn't need to block bug 463027.  We already have a way to determine
> > > this.  I'm not sure what you want to happen in this bug.
> > 
> > I need a way to determine if pwpb is active at runtime without version
> > checking.
> 
> Can't you just instantiate the service and see if it has a
> privateBrowsingEnabled property on it (after QIing to
> nsIPrivateBrowsingService, of course), and put it all in a try catch block
> to make sure that it's future proof?  Something like:
> 
> function isGlobalPBAvailable() {
>   var globalPB = false;
>   try {
>     var pbSvc = Components.classes["@mozilla.org/privatebrowsing;1"].
>                 getService(Components.interfaces.nsIPrivateBrowsingService);
>     globalPB = ("privateBrowsingEnabled" in pbSvc);
>   } catch (e) {}
>   return globalPB;
> }
> 
> (I tested the snippet this time.  :-)

My problem is that detecting the absence of the global pb isn't the same as feature detecting pwpb.  I'd rather have the latter because it'll be more reliable and doesn't require app/version checking.

Comment 20

[(no longer active)]

I see.  The only way that I can think we can enable you to do that is to add a new property to PrivateBrowsingUtils which returns true when building with MOZ_PER_WINDOW_PRIVATE_BROWSING and false otherwise, and then uplift that patch to Aurora and Beta.  Let's see what Josh thinks.

Comment 21

[(no longer active)]

Attachment: Patch (v1)

Josh, if you don't agree with this, please r-.

Comment 22

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

(In reply to :Ehsan Akhgari from comment #20)
> I see.  The only way that I can think we can enable you to do that is to add
> a new property to PrivateBrowsingUtils which returns true when building with
> MOZ_PER_WINDOW_PRIVATE_BROWSING and false otherwise, and then uplift that
> patch to Aurora and Beta.  Let's see what Josh thinks.

Wait pwpb is going to be active for Fx20 which is Aurora atm right? so we'd only need to uplift to Aurora afaict.

Comment 23

[Josh Matthews [:jdm]]

I suspect this doesn't actually fulfill Erik's desires, since this isn't a property that can be customized by applications like Thunderbird.

Comment 24

[(no longer active)]

Hmm, I think we might be taking this too far.  Erik, please consider a white-listing approach instead, that is, make sure that the add-on sdk supports Firefox desktop and mobile as far as private browsing is concerned, and consider all other environments as not supported.  Whenever we have more applications supporting private browsing at all, we can reconsider this.

Comment 25

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

(In reply to :Ehsan Akhgari from comment #18)
> (In reply to comment #17)
> > (In reply to :Ehsan Akhgari from comment #16)
> > > This doesn't need to block bug 463027.  We already have a way to determine
> > > this.  I'm not sure what you want to happen in this bug.
> > 
> > I need a way to determine if pwpb is active at runtime without version
> > checking.
> 
> Can't you just instantiate the service and see if it has a
> privateBrowsingEnabled property on it (after QIing to
> nsIPrivateBrowsingService, of course), and put it all in a try catch block
> to make sure that it's future proof?  Something like:
> 
> function isGlobalPBAvailable() {
>   var globalPB = false;
>   try {
>     var pbSvc = Components.classes["@mozilla.org/privatebrowsing;1"].
>                 getService(Components.interfaces.nsIPrivateBrowsingService);
>     globalPB = ("privateBrowsingEnabled" in pbSvc);
>   } catch (e) {}
>   return globalPB;
> }
> 
> (I tested the snippet this time.  :-)

This returns true on Nightly.

Comment 26

[Josh Matthews [:jdm]]

Not for me on Linux from 1/15.

Comment 27

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

(In reply to Josh Matthews [:jdm] from comment #26)
> Not for me on Linux from 1/15.

Weird I must have been trying release somehow.. sorry, it seems to work for me now too.