Closed
Bug 1399014
Opened 7 years ago
Closed 7 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•7 years ago
|
||
Hi Joe Please help decide the experiment we want.
Flags: needinfo?(jcheng)
Assignee | ||
Comment 2•7 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•7 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•7 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•7 years ago
|
Assignee: nobody → cnevinchen
Comment 9•7 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•7 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•7 years ago
|
Attachment #8907162 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Comment 13•7 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•7 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•7 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•7 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•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0d59c91f1ad4
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(cnevinchen)
Summary: Experiment with tracking protection → Allow 'enable tracking protection in normal browsing' for release and beta channel.
Updated•7 years ago
|
Flags: needinfo?(max)
Assignee | ||
Comment 19•7 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•7 years ago
|
Whiteboard: [FNC][SPT57.3][INT]
Updated•7 years ago
|
Flags: needinfo?(jcheng)
Updated•6 years ago
|
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
QA Contact: ioana.chiorean
Updated•3 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
•