Closed
Bug 1399014
Opened 8 years ago
Closed 8 years ago
Allow 'enable tracking protection in normal browsing' for release and beta channel.
Categories
(Firefox for Android Graveyard :: Settings and Preferences, enhancement)
Firefox for Android Graveyard
Settings and Preferences
Tracking
(firefox57 fixed)
VERIFIED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: cnevinchen, Assigned: cnevinchen)
References
Details
(Whiteboard: [FNC][SPT57.3][INT])
Attachments
(1 file, 1 obsolete file)
We want to experiment on tracking protection.
Currently there are two options
1. A/B testing on options:
For Nightly and in-experiment Beta/Release users, we show three options:
A. Enable
B. Enable in Private Browsing
C. Disabled
For non-experiment Beta/Release users, we show two options:
A. Enable in Private Browsing
B. Disabled
2. A/B testing on default selection:
All users have three options:
A. Enable
B. Enable in Private Browsing
C. Disabled
Except
For Nightly and in-experiment Beta/Release users, default is A
For non-experiment Beta/Release users, default is B
Assignee | ||
Comment 1•8 years ago
|
||
Hi Joe
Please help decide the experiment we want.
Flags: needinfo?(jcheng)
Assignee | ||
Comment 2•8 years ago
|
||
To add one thing, we also need to handle the behavior during experiment transition.
A user should expect below behavior:
TP(tracking protection)
PB(private browsing)
Before After in experiment
-------------------------------------------------------------
Enable in PB Enable in PB
Disable Disable
Before leaving Experiment After leaving experiment
-------------------------------------------------------------
Enable Enable in PB
Enable in PB Enable in PB
Disable Disable
Assignee | ||
Comment 3•8 years ago
|
||
tldr; if we want comment 2, it'll take more time. at least (full) 2 days to code, 3 day to test, and 2 days to fix bugs on
1) Leanplum
2) The new switchboard migration logic
Looking at our code, I found that we didn't save experiment state( i.e. we don't know if we are 'entering' or 'leaving' an experiment). So transition in comment 2 without changing our experiment code is hard)
what I can do is :
1) Add two callbacks to Switchboard, one is to keep previous experiment/kinto configuration before we replace the old one[1] and the second is to do the migration logic in postExecution here[2]
2) Since Leanplum(another component) is also doing something after this loadConfig() call, I may need to move its' logic and add stuff there. ( using composite/listeners or inherence )
3) If we use composite/listeners, we need to add some subscribe/unsubscribe code. If we use inherence, these two post loadConfig() logic will be coupled together. Not a very pretty design.
[1] https://searchfox.org/mozilla-central/rev/51eae084534f522a502e4b808484cde8591cb502/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#901
[2] https://searchfox.org/mozilla-central/rev/51eae084534f522a502e4b808484cde8591cb502/mobile/android/base/java/org/mozilla/gecko/switchboard/AsyncConfigLoader.java#44
Flags: needinfo?(max)
Assignee | ||
Comment 4•8 years ago
|
||
(In reply to Nevin Chen [:nechen] from comment #0)
> We want to experiment on tracking protection.
> Currently there are two options
Looking at email thread, we are going for this approach
> 1. A/B testing on options:
pref name : privacy.trackingprotection.state
> For Nightly and in-experiment Beta/Release users, we show three options:
> A. Enable
> B. Enable in Private Browsing
> C. Disabled
pref name : privacy.trackingprotection.pbmode.enabled
> For non-experiment Beta/Release users, we show two options:
> A. Enable in Private Browsing
> B. Disabled
The setting transition in comment 2 need to check both pref before/after above change occurs.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → cnevinchen
Comment 9•8 years ago
|
||
mozreview-review |
Comment on attachment 8907162 [details]
Bug 1399014 - Remove old tracking protection pref UI
https://reviewboard.mozilla.org/r/178844/#review184792
Attachment #8907162 -
Flags: review?(max) → review+
Comment 10•8 years ago
|
||
mozreview-review |
Comment on attachment 8907161 [details]
Bug 1399014 - Allow 'enable tracking protection in normal browsing' for release and beta channel.
https://reviewboard.mozilla.org/r/178842/#review184790
::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoSharedPrefs.java:80
(Diff revision 2)
> "sendReport",
> "includeUrl",
> "allowContact",
> "contactEmail"
> };
> + private static final String MIGRATE_PREFS_TRACKING_PROTECTION = "privacy.trackingprotection.state";
How about make this reference to GeckoPreferences.PREFS_TRACKING_PROTECTION instead of dup the value here?
::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoSharedPrefs.java:81
(Diff revision 2)
> "includeUrl",
> "allowContact",
> "contactEmail"
> };
> + private static final String MIGRATE_PREFS_TRACKING_PROTECTION = "privacy.trackingprotection.state";
> + private static final String MIGRATE_PREFS_TRACKING_PROTECTION_PB = "privacy.trackingprotection.pbmode.enabled";
Add a comment to indicate the key is deprecated, put it here is only for removal.
::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoSharedPrefs.java:259
(Diff revision 2)
>
> Log.d(LOGTAG, "All keys have been migrated");
> }
>
> + private static void migrateTrackingProtection(SharedPreferences appPrefs) {
> + // Nightly and Local builds already usedq privacy.trackingprotection.state so don't need migration.
typo: usedq
::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoSharedPrefs.java:261
(Diff revision 2)
> }
>
> + private static void migrateTrackingProtection(SharedPreferences appPrefs) {
> + // Nightly and Local builds already usedq privacy.trackingprotection.state so don't need migration.
> + if (AppConstants.RELEASE_OR_BETA) {
> + final boolean isTrackingProtectionEnable = appPrefs.getBoolean(MIGRATE_PREFS_TRACKING_PROTECTION_PB, false);
Should the default value be false?
Attachment #8907161 -
Flags: review?(max) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8907162 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Comment 13•8 years ago
|
||
hg error in cmd: hg pull gecko -r 0b79b57ec3038e13cec4e88c20479ef2049af405: pulling from https://reviewboard-hg.mozilla.org/gecko
abort: HTTP Error 504: Gateway Timeout
Comment 14•8 years ago
|
||
Pushed by nechen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1bc5e32dd5f6
Alow 'enable tracking protection in normal browsing' for release and beta channel. r=maliu
![]() |
||
Comment 15•8 years ago
|
||
Backed out for failing android lint:
https://hg.mozilla.org/integration/autoland/rev/7807000c75138d299410a996db5f95eef9775e1d
Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=1bc5e32dd5f69ddaca634f577df765a43b6ec451&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=130991163&repo=autoland
Flags: needinfo?(cnevinchen)
Comment hidden (mozreview-request) |
Comment 17•8 years ago
|
||
Pushed by nechen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0d59c91f1ad4
Allow 'enable tracking protection in normal browsing' for release and beta channel. r=maliu
![]() |
||
Comment 18•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(cnevinchen)
Summary: Experiment with tracking protection → Allow 'enable tracking protection in normal browsing' for release and beta channel.
Updated•8 years ago
|
Flags: needinfo?(max)
Assignee | ||
Comment 19•8 years ago
|
||
Just for the record,
We won't have an experiment about TP. It'll just ride the train.The only change is on Beta and Release
Updated•8 years ago
|
Whiteboard: [FNC][SPT57.3][INT]
Updated•8 years ago
|
Flags: needinfo?(jcheng)
Updated•7 years ago
|
Status: RESOLVED → VERIFIED
Updated•7 years ago
|
QA Contact: ioana.chiorean
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•