Wipe the container for privacy.usercontext.about_newtab_segregation.enabled on shutdown

RESOLVED FIXED in Firefox 61

Status

()

defect
P1
critical
RESOLVED FIXED
a year ago
3 months ago

People

(Reporter: tjr, Assigned: baku)

Tracking

(Depends on 1 bug, Blocks 1 bug)

unspecified
Firefox 62
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed, firefox62 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Reporter

Description

a year ago
+++ This bug was initially created as a clone of Bug #1457929 +++

As discussed in Bug 1457929 - we should clear out the container used by privacy.usercontext.about_newtab_segregation.enabled (which I believe is '5') so we don't make a persistent container that tracks the user through thumbnails.

:baku proposed doing it on shutdown (and startup in case of crash.)
Component: Bookmarks & History → Activity Streams: Newtab
Assignee

Comment 1

a year ago
Posted patch container_5.patch (obsolete) — Splinter Review
This should be enough to cleanup the container number 5.
Assignee: nobody → amarchesini
Attachment #8979530 - Flags: review?(jhofmann)
Comment on attachment 8979530 [details] [diff] [review]
container_5.patch

Review of attachment 8979530 [details] [diff] [review]:
-----------------------------------------------------------------

As mentioned on IRC, I think we'll have to find a different solution for this because pending sanitization is currently only set when the user has chosen to clear data on shutdown.
Attachment #8979530 - Flags: review?(jhofmann) → review-
Assignee

Comment 3

a year ago
Posted patch container_5.patch (obsolete) — Splinter Review
Attachment #8979530 - Attachment is obsolete: true
Attachment #8981372 - Flags: review?(jhofmann)
Assignee

Comment 4

a year ago
Attachment #8981372 - Attachment is obsolete: true
Attachment #8981372 - Flags: review?(jhofmann)
Attachment #8981387 - Flags: review?(jhofmann)
Comment on attachment 8981387 [details] [diff] [review]
container_5.patch

Review of attachment 8981387 [details] [diff] [review]:
-----------------------------------------------------------------

This should work, thanks!

::: browser/modules/Sanitizer.jsm
@@ +59,5 @@
>     */
>    PREF_TIMESPAN: "privacy.sanitize.timeSpan",
>  
>    /**
> +   * Pref to newTab segretation

spelling: segregation

You might also want to explain why that pref is here.

::: browser/modules/test/unit/test_Sanitizer_interrupted.js
@@ +11,5 @@
>  
>  add_task(async function() {
>    ChromeUtils.import("resource:///modules/Sanitizer.jsm");
> +
> +  Services.prefs.setBoolPref(Sanitizer.PREF_NEWTAB_SEGREGATION, false);

You need to clear this pref as well.
Attachment #8981387 - Flags: review?(jhofmann) → review+

Comment 6

a year ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/62ed84f80e37
Wipe the container for privacy.usercontext.about_newtab_segregation.enabled on shutdown, r=johannh
Assignee

Updated

a year ago
Flags: needinfo?(amarchesini)

Comment 8

a year ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e62b8b3202a
Wipe the container for privacy.usercontext.about_newtab_segregation.enabled on shutdown, r=johannh

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7e62b8b3202a
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Reporter

Comment 10

a year ago
Comment on attachment 8981387 [details] [diff] [review]
container_5.patch

[Feature/Bug causing the regression]: If Bug 1457929 is taken in beta, we should take this patch too.

[User impact if declined]: None; unless Bug 1457929 is taken in which case we'll have a weird (but not dangerous) situation for a release.

[Is this code covered by automated tests?]: Yes

[Has the fix been verified in Nightly?]: Yes

[Needs manual test from QE? If yes, steps to reproduce]: No

[List of other uplifts needed for the feature/fix]: Only needed if we take Bug 1457929

[Is the change risky?]: Not very.

[Why is the change risky/not risky?]: It adds a background sanitization. I supposed it's possible this could cause problems, but we use existing history-removal APIs for this.
Attachment #8981387 - Flags: approval-mozilla-beta?
Comment on attachment 8981387 [details] [diff] [review]
container_5.patch

Patch to harden the privacy of the privacy.usercontext.about_newtab_segregation.enabled preference. Approved for 61.0b11.
Attachment #8981387 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
This needs a rebased patch for Beta uplift.
Flags: needinfo?(amarchesini)
Assignee

Comment 13

a year ago
Posted patch m-bSplinter Review
Flags: needinfo?(amarchesini)
Attachment #8982344 - Attachment is patch: true
Attachment #8982344 - Flags: approval-mozilla-beta+
Attachment #8981387 - Flags: approval-mozilla-beta+
Depends on: 1533134
You need to log in before you can comment on or make changes to this bug.