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.
Created attachment 554705 [details] [diff] [review] patch proposal
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+
Created attachment 554966 [details] [diff] [review] patch v1.1 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! :)
Created attachment 557338 [details] [diff] [review] patch v1.1 I think this one in correct
Target Milestone: --- → mozilla9
Status: ASSIGNED → RESOLVED
Last Resolved: 8 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.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.