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

RESOLVED FIXED in Firefox 61

Status

()

P1
critical
RESOLVED FIXED
10 months ago
13 days 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

10 months 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

10 months 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

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

Comment 4

10 months 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

10 months 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

10 months ago
Flags: needinfo?(amarchesini)

Comment 8

10 months 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

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7e62b8b3202a
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox62: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
(Reporter)

Comment 10

10 months 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

10 months 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+

Comment 14

10 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/cab78187269b
status-firefox61: --- → fixed
Depends on: 1533134
You need to log in before you can comment on or make changes to this bug.