Closed
Bug 1175606
Opened 9 years ago
Closed 9 years ago
Private Browsing Mode should enable Tracking Protection by default
Categories
(Firefox :: General, defect, P1)
Firefox
General
Tracking
()
People
(Reporter: past, Assigned: past)
References
Details
(Whiteboard: [fxprivacy] [campaign])
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
When the user opens a new window in PBM, it should have TP enabled by default.
Flags: firefox-backlog?
Updated•9 years ago
|
Rank: 6
Flags: firefox-backlog? → firefox-backlog+
Priority: -- → P1
Assignee | ||
Comment 1•9 years ago
|
||
I think this is all there is to it.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → past
Status: NEW → ASSIGNED
Updated•9 years ago
|
Points: --- → 1
Flags: qe-verify+
Updated•9 years ago
|
Iteration: --- → 42.1 - Jul 13
Updated•9 years ago
|
Assignee: past → nobody
Status: ASSIGNED → NEW
Iteration: 42.1 - Jul 13 → ---
Updated•9 years ago
|
QA Contact: mwobensmith
Comment 2•9 years ago
|
||
When we flip the default value of the preference and enable the feature for all users of Nightly and other channels, we'll have to show the new design of the "about:privatebrowsing" page implemented in bug 1177152.
The new version is temporarily controlled by the "privacy.trackingprotection.ui.enabled" preference, so that we can test it manually but is disabled by default in Nightly. We'll just have to remove the dependence on the preference (and keep the "ui.enabled" preference to its value of false).
Points: 1 → 2
Updated•9 years ago
|
Whiteboard: [fxprivacy] → [fxprivacy] [blocked on feature readiness]
Updated•9 years ago
|
Whiteboard: [fxprivacy] [blocked on feature readiness] → [fxprivacy] [campaign] [blocked on feature readiness]
Comment 3•9 years ago
|
||
Javaun confirmed this is ready for selection in IT 42.3
Whiteboard: [fxprivacy] [campaign] [blocked on feature readiness] → [fxprivacy] [campaign]
Updated•9 years ago
|
Assignee: nobody → past
Status: NEW → ASSIGNED
Iteration: --- → 42.3 - Aug 10
Assignee | ||
Comment 4•9 years ago
|
||
Updated the patch according to comment 2. The trackingprotection mochitests pass locally.
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4058d217004a
Attachment #8624199 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8640112 -
Flags: review?(ttaubert)
Updated•9 years ago
|
Attachment #8640112 -
Flags: review?(ttaubert) → review+
Comment 5•9 years ago
|
||
I suggest we wait a few days before landing this in Nightly, in order to test and implement the revised about:privatebrowsing design first, which is significantly different from the current one.
Bug 1188455 has references to the updated mockups.
Comment 6•9 years ago
|
||
Comment on attachment 8640112 [details] [diff] [review]
Patch v2
Flagging the patch as a reminder.
Attachment #8640112 -
Flags: checkin-
Comment 7•9 years ago
|
||
Comment on attachment 8640112 [details] [diff] [review]
Patch v2
I don't get the reasoning why we wouldn't land it in Nightly? It's a technical audience that could help us get some more test coverage. This shouldn't block on any about:privatebrowsing design changes, we have no A/B tests or telemetry on Nightly to figure out if that makes a difference anyway.
Attachment #8640112 -
Flags: checkin-
Comment 8•9 years ago
|
||
The reasoning is that we would show a version of about:privatebrowsing we're not going to use, for just three days or so. I can see how our technical audience may be surprised. On the other hand, if we land this sooner we gain three more days of testing on the actual Tracking Protection feature, so there is an advantage as well.
Comment 9•9 years ago
|
||
Would love to get some input from Aislinn but she's OOO until the end of the week. Maybe we can just discuss this in the standup in a few minutes. The new design is indeed very different but it does just as well promote TP on about:privatebrowsing. I would expect at least 90% of the Nightly audience to understand what TP is immediately... But let's ask other people too.
Comment 10•9 years ago
|
||
Comment on attachment 8640112 [details] [diff] [review]
Patch v2
Talked it through, let's hold off until bug 1188455 and bug 1184312 are fixed.
Attachment #8640112 -
Flags: checkin-
Assignee | ||
Comment 11•9 years ago
|
||
Fixes a test that I missed before locally, but was caught on try. Still not planning to land yet, per the discussion above.
Attachment #8640112 -
Attachment is obsolete: true
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8641093 [details] [diff] [review]
Patch v3
New try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eab7e5a52c1c
Attachment #8641093 -
Flags: review?(ttaubert)
Updated•9 years ago
|
Attachment #8641093 -
Flags: review?(ttaubert) → review+
Assignee | ||
Comment 13•9 years ago
|
||
This is ready to land, but the tree is closed so let's see if the sheriffs can land it when they reopen it.
Keywords: checkin-needed
Comment 14•9 years ago
|
||
This should actually be rebased on top of bug 1190427.
Keywords: checkin-needed
This broke every talos suite with failures like https://treeherder.mozilla.org/logviewer.html#?job_id=4137077&repo=fx-team
Backed out in https://hg.mozilla.org/integration/fx-team/rev/152eafb9dc89
Flags: needinfo?(past)
Comment 18•9 years ago
|
||
Maybe we could override privacy.trackingprotection.pbmode.enabled in the talos prefs
Comment 19•9 years ago
|
||
I'll put together a patch for talos
Comment 20•9 years ago
|
||
Reminder to include again the removal of the code that hides the Tracking Protection section from about:privatebrowsing in the next version of the patch.
Comment 21•9 years ago
|
||
Attachment #8645150 -
Attachment is obsolete: true
Flags: needinfo?(past)
Comment 22•9 years ago
|
||
I've got a try push with the talos fix in place here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2a29513c92b. And a normal try push with v5 here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=96e2cd19a5de
Comment 23•9 years ago
|
||
Comment 24•9 years ago
|
||
Backed out in https://hg.mozilla.org/integration/fx-team/rev/c56e7866445c since the talos bump had to be backed out.
Comment 25•9 years ago
|
||
Third time's a charm:
https://hg.mozilla.org/integration/fx-team/rev/dc44b9a3b542
Comment 27•9 years ago
|
||
Not sure merge into m-c removed this patch.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1192611 for breaking win7 Start Menu
I may have messed up regression window testing but testing points to:
20150807135006 799742085c18 as Good
20150807143017 152eafb9dc89 as Bad
Comment 28•9 years ago
|
||
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #27)
> Not sure merge into m-c removed this patch.
>
> See https://bugzilla.mozilla.org/show_bug.cgi?id=1192611 for breaking win7
> Start Menu
>
> I may have messed up regression window testing but testing points to:
>
> 20150807135006 799742085c18 as Good
> 20150807143017 152eafb9dc89 as Bad
Commented in 1192611 for further discussion, but both of the previous landings have been backed out before hitting m-c. And in fact 152eafb9dc89 was the backout revision
Comment 29•9 years ago
|
||
(In reply to Pulsebot from comment #26)
> https://hg.mozilla.org/integration/fx-team/rev/c0a2c18218d1
Note, this follow up commit was to fix an issue with broken crashtests in linux 32 debug [0]. I needed to disable TP separately for reftests to fix that issue (which, sidenote, apparently have *many* places that default prefs can be set [1]). Picked layout/tools/reftest/runreftest.py because that's where safebrowsing was being disabled as well. It seems to have worked since all the retriggers after this look good [2].
[0]: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=dc44b9a3b542
[1]: https://developer.mozilla.org/en-US/docs/Mozilla/QA/Automated_testing#Need_to_set_preferences_for_test-suites
[2]: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=c0a2c18218d1
Comment 30•9 years ago
|
||
I don't know what happened or how it cleared. I'm totally confused at this point.
See my comment here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1192611#c5
Sorry for noise.
Comment 31•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/dc44b9a3b542
https://hg.mozilla.org/mozilla-central/rev/c0a2c18218d1
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Comment 32•9 years ago
|
||
Just tested the m-c build with the patch landing in cset:
https://hg.mozilla.org/mozilla-central/rev/a431fb8b9a40
The missing Nightly Icon in the Win7 Start Menu 'Most Frequent used list' is again missing as described in bug 1192611
Please see my comments in that bug about this bug needing a 'clobber' to work correctly.
https://bugzilla.mozilla.org/show_bug.cgi?id=1192611#c6
I guess tomorrow's Nightly will maybe work because Nightly's are 'clobber builds'.
Comment 33•9 years ago
|
||
Testing in today's Nightly build the Nightly Icon appears now in the Most Frequent list from the Win7 Start Orb.
Appears to me that whenever this patch is backed out or landed that it needs a clobber to fully activate/deactivate the changes in this patch.
As to why its affecting the Start Menu Frequently used list is beyond my understanding.
Comment 34•9 years ago
|
||
Sanity test on Mac/Win/Linux to confirm enabled by default in private browsing looks good. That's all I will verify in this bug.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•