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)

enhancement
Not set
normal

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
Hi Joe Please help decide the experiment we want.
Flags: needinfo?(jcheng)
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
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)
(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.
Assignee: nobody → cnevinchen
Attachment #8907162 - Flags: review?(max) → 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+
Attachment #8907162 - Attachment is obsolete: true
hg error in cmd: hg pull gecko -r 0b79b57ec3038e13cec4e88c20479ef2049af405: pulling from https://reviewboard-hg.mozilla.org/gecko abort: HTTP Error 504: Gateway Timeout
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
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
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Flags: needinfo?(cnevinchen)
Summary: Experiment with tracking protection → Allow 'enable tracking protection in normal browsing' for release and beta channel.
Flags: needinfo?(max)
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
Whiteboard: [FNC][SPT57.3][INT]
Flags: needinfo?(jcheng)
See Also: → 1409192
Status: RESOLVED → VERIFIED
QA Contact: ioana.chiorean
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: