Closed Bug 801630 Opened 12 years ago Closed 12 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

16 Branch
x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 19

People

(Reporter: mfinkle, Assigned: mfinkle)

References

Details

Attachments

(2 files)

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.
(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.
(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.
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)
Attached patch patch 2 - cleanup (deleted) — Splinter Review
This patch does some simple cleanup in the JS file.
Attachment #674005 - Flags: review?(bnicholson)
Attachment #674005 - Flags: review?(bnicholson) → review+
Attachment #674004 - Flags: review?(ehsan) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: