Closed
Bug 679784
Opened 13 years ago
Closed 13 years ago
nsIContentPrefService should handle private browsing mode
Categories
(Toolkit :: Preferences, defect)
Tracking
()
RESOLVED
FIXED
mozilla9
People
(Reporter: arno, Assigned: arno)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 2 obsolete files)
38.40 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
Hi, Currently, nsHTMLInputElement uses content preferences service (bug #536567). But, it also manages private browsing mode: when in private browsing mode, it stores the preferences in memory. So, it has first to observe NS_PRIVATE_BROWSING_SWITCH_TOPIC, then it must have two code paths: one for private mode, the other for normal mode. The private mode path also duplicates a part of nsIContentPrefService.setPref (by getting the group's uri). Then, the same thing will have to be done for the spellChecker in #678842 So, it would be nice that content pref service manages private browsing mode. Here is how this is handled in nsHTMLInputElement.cpp - when setting a pref in private mode, store the association uri/pref in an in-memory hashtable. - when getting a pref in private mode, get it from in-memory hashtable. - when leaving the private mode, clear the memory storage. This allow content preferences service to run correctly during a private browsing session, and assures that no data is kept afterwards. This would also improve full zoom content pref handling because currently, no zoom value is restored inside a private browsing session. Also, it would make future uses of content pref services easier, and make sure an extension using content pref service does not leak browsing informations accidentally.
Assignee | ||
Comment 1•13 years ago
|
||
Comment 2•13 years ago
|
||
Comment on attachment 554705 [details] [diff] [review] patch proposal Review of attachment 554705 [details] [diff] [review]: ----------------------------------------------------------------- Looks really good! ::: toolkit/components/contentprefs/nsContentPrefService.js @@ +602,4 @@ > return this._selectPrefsByName(aName); > }, > > + // or boolean to indicate if we are in private browsing mode Nit: s/or // ::: toolkit/components/contentprefs/tests/unit/test_bug248970.js @@ +91,5 @@ > do_check_eq(cp.getPref(uri1, pref_name), zoomA_new); > // exit private browsing mode > pb.privateBrowsingEnabled = false; > + // make sure Zoom-A change has not persisted > + do_check_eq(cp.getPref(uri1, pref_name), zoomA); Can you also make sure that the Zoom-B value is not persisted here?
Attachment #554705 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 3•13 years ago
|
||
updated patch to fix mentioned issues. By the way, I forgot to mention that previous patch has built and passed tests successfully on try server: http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=3c2bfc6c7739
Attachment #554705 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Attachment #554966 -
Flags: review?(ehsan)
Comment 4•13 years ago
|
||
Comment on attachment 554966 [details] [diff] [review] patch v1.1 This is the wrong patch I believe! :)
Attachment #554966 -
Flags: review?(ehsan)
Assignee | ||
Comment 5•13 years ago
|
||
I think this one in correct
Attachment #554966 -
Attachment is obsolete: true
Attachment #557338 -
Flags: review?(ehsan)
Updated•13 years ago
|
Attachment #557338 -
Flags: review?(ehsan) → review+
Comment 7•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/3b04343f2382
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 8•13 years ago
|
||
arno documented this: https://developer.mozilla.org/en/Using_content_preferences#private-browsing It's also listed on Firefox 9 for developers.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•