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

RESOLVED FIXED in Firefox 19

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mfinkle, Assigned: mfinkle)

Tracking

16 Branch
Firefox 19
x86_64
Linux
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

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

5 years ago
(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.
(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

5 years ago
(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.
Created attachment 674004 [details] [diff] [review]
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.
Assignee: bnicholson → mark.finkle
Attachment #674004 - Flags: review?(ehsan)
Created attachment 674005 [details] [diff] [review]
patch 2 - cleanup

This patch does some simple cleanup in the JS file.
Attachment #674005 - Flags: review?(bnicholson)
Attachment #674005 - Flags: review?(bnicholson) → review+

Updated

5 years ago
Attachment #674004 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/a1170e62ccd5
https://hg.mozilla.org/mozilla-central/rev/f2686a2cd826
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
You need to log in before you can comment on or make changes to this bug.