801630 - exception.js makes use of the global Private Browsing service to make decisions

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Bug 722978 was filed for desktop, but Firefox on Android uses a tab-based private browing pattern. The patch in bug 722978 is for window-based private browsing and won't work for mobile.

Maybe we should pass the browser.contentWindow into the addPermanentException and addTemporaryException methods so we can check the specific content docshell flags.

Comment 1

[(no longer active)]

(In reply to Mark Finkle (:mfinkle) from comment #0)
> Bug 722978 was filed for desktop, but Firefox on Android uses a tab-based
> private browing pattern. The patch in bug 722978 is for window-based private
> browsing and won't work for mobile.
> 
> Maybe we should pass the browser.contentWindow into the
> addPermanentException and addTemporaryException methods so we can check the
> specific content docshell flags.

Is that code used in mobile directly?  My patch in bug 722978 is using the |window| object, which is the global window object of exceptionDialog.xul, and if the window object in mobile is something similar to that, it should work for mobile as well.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Ehsan Akhgari [:ehsan] from comment #1)
> (In reply to Mark Finkle (:mfinkle) from comment #0)
> > Bug 722978 was filed for desktop, but Firefox on Android uses a tab-based
> > private browing pattern. The patch in bug 722978 is for window-based private
> > browsing and won't work for mobile.
> > 
> > Maybe we should pass the browser.contentWindow into the
> > addPermanentException and addTemporaryException methods so we can check the
> > specific content docshell flags.
> 
> Is that code used in mobile directly?  My patch in bug 722978 is using the
> |window| object, which is the global window object of exceptionDialog.xul,
> and if the window object in mobile is something similar to that, it should
> work for mobile as well.

Mobile uses in-content error pages and does _not_ use a dialog for any part of the UI. That's the core reason we forked the code. Since it's all in-content (and per-tab) I think we need to use the browser.contentWindow to check the private browsing flags.

Comment 3

[(no longer active)]

(In reply to comment #2)
> (In reply to Ehsan Akhgari [:ehsan] from comment #1)
> > (In reply to Mark Finkle (:mfinkle) from comment #0)
> > > Bug 722978 was filed for desktop, but Firefox on Android uses a tab-based
> > > private browing pattern. The patch in bug 722978 is for window-based private
> > > browsing and won't work for mobile.
> > > 
> > > Maybe we should pass the browser.contentWindow into the
> > > addPermanentException and addTemporaryException methods so we can check the
> > > specific content docshell flags.
> > 
> > Is that code used in mobile directly?  My patch in bug 722978 is using the
> > |window| object, which is the global window object of exceptionDialog.xul,
> > and if the window object in mobile is something similar to that, it should
> > work for mobile as well.
> 
> Mobile uses in-content error pages and does _not_ use a dialog for any part of
> the UI. That's the core reason we forked the code. Since it's all in-content
> (and per-tab) I think we need to use the browser.contentWindow to check the
> private browsing flags.

Yeah that should work.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch 1 - use docshell to determine private browsing

This patch passes the browser window to the exception tracking code and uses it to determine if the browser is private. It uses PrivateBrowsingUtils.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch 2 - cleanup

This patch does some simple cleanup in the JS file.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a1170e62ccd5
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2686a2cd826

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a1170e62ccd5
https://hg.mozilla.org/mozilla-central/rev/f2686a2cd826