Closed Bug 800193 Opened 11 years ago Closed 11 years ago

Make the nsILoadContext.usePrivateBrowsing property read-only

Categories

(Firefox :: Private Browsing, defect)

Product:
Firefox
Component:
Private Browsing
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 19

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

Details

Attachments

(1 file, 1 obsolete file)

Patch (v2)
10.36 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review 
Josh, now that we have everything we need for testing private windows, do you think it still makes sense to support setting the usePrivateBrowsing property from script?
It makes it impossible to support an addon implementing per-tab mode, doesn't it?
(In reply to comment #1)
> It makes it impossible to support an addon implementing per-tab mode, doesn't
> it?

It does, and I think we have good reason to do so.  Those add-ons will be partially broken today (because we cache the privacy bit in some cases) and it would be extremely hard if not impossible to keep everything working in a way which doesn't break all such add-ons, except if we end up with an API which puts a tab's docshell in PB mode as soon as it's created.  Thinking more about this, mobile might need this since all they have is a single window, so we can revisit the decision of not supporting such add-ons in the future, but they won't need to be able to set this flag anyway.
So, looking closer into this, we have a while bunch of call sites in tests which set this property in order to test stuff.  We also have a couple of call sites in the front-end which call this which will eventually go away.

So, perhaps making this property read-only is not really practical after all.  Josh, what do you think about just printing a warning to the error console saying that this method is internal and is not meant to be used by folks, etc.?
How would you implement the warning, precisely? The docshell tree stuff uses the setter to propagate the value downwards.
Attached patch Patch (v1) (obsolete) — — Splinter Review 
Like this.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #676905 - Flags: review?(bzbarsky)
Comment on attachment 676905 [details] [diff] [review]
Patch (v1)

That doesn't look like it actually addresses my comment, does it? Won't we always see this warning when creating a new private window, tab in a private window, a content page attaches an iframe, etc.?
(In reply to Josh Matthews [:jdm] from comment #6)
> Comment on attachment 676905 [details] [diff] [review]
> Patch (v1)
> 
> That doesn't look like it actually addresses my comment, does it? Won't we
> always see this warning when creating a new private window, tab in a private
> window, a content page attaches an iframe, etc.?

Ah, hmm, yeah you're right...  How about if I only do this when there is a JS caller?
What does a JS caller mean, precisely? That sounds like it could still trigger if JS invokes opening a new private window.
(In reply to comment #8)
> What does a JS caller mean, precisely? That sounds like it could still trigger
> if JS invokes opening a new private window.

Yeah..  you're right.  I can't seem to think of any other solutions... :/
We can always use a noscript non-warning method for our internal docshell uses, right?
(In reply to Boris Zbarsky (:bz) from comment #10)
> We can always use a noscript non-warning method for our internal docshell
> uses, right?

Yes, that's true.  I'll give that a shot next.
Attached patch Patch (v2) — — Splinter Review 

        Attachment #676905 -
        Attachment is obsolete: true

        Attachment #676905 -
        Flags: review?(bzbarsky)

        Attachment #677740 -
        Flags: review?(bzbarsky)

    
    

    
  
Comment on attachment 677740 [details] [diff] [review]
Patch (v2)

r=me

        Attachment #677740 -
        Flags: review?(bzbarsky) → review+

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/b940cf551019
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19

    
    

    
  
This bug breaks "Private Tab" functionality... I really hope there IS workardound as having private tabs is one of the fetures I have been waiting for a LOOONG time :(

    
    

    
  
(In reply to comment #16)
> This bug breaks "Private Tab" functionality... I really hope there IS
> workardound as having private tabs is one of the fetures I have been waiting
> for a LOOONG time :(

Which private tab functionality?

    
    

    
  
This addon allows creation of a private tab: https://addons.mozilla.org/en-US/seamonkey/addon/private-tab/

          You need to log in
          before you can comment on or make changes to this bug.