Closed Bug 679784 Opened 9 years ago Closed 9 years ago
IContent Pref Service should handle private browsing mode
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: nobody → arno
Status: NEW → ASSIGNED
Attachment #554705 - Flags: review?(ehsan)
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+
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
Comment on attachment 554966 [details] [diff] [review] patch v1.1 This is the wrong patch I believe! :)
I think this one in correct
Target Milestone: --- → mozilla9
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
arno documented this: https://developer.mozilla.org/en/Using_content_preferences#private-browsing It's also listed on Firefox 9 for developers.
You need to log in before you can comment on or make changes to this bug.