Even when bug 1103241 is complete, very few people will change the defaults in privacy preferences. From http://monica-at-mozilla.blogspot.com/2013/02/writing-for-98.html, most preferences are changed <1% of the time, unless there is a massive marketing campaign promoting that preference, such as the case with DNT. We should invite Nightly users to enable tracking protection through a doorhanger. This can be similar to the doorhanger that e10s used to recruit users and run for only short periods of time and be targeted by existing preferences.

Screenshot of doorhanger

Screenshot of activation notice

Comment on attachment 8550011 [details] doorhanger.png The proposed flow for Nightly, based on E10S doorhanger: 1) If the user has DNT enabled and doesn't have tracking protection enabled, show them this doorhanger 2) If they say no, never show them the doorhanger again 3) If they dismiss without selecting yes or no, show them the doorhanger up to 5 times total 4) If they say yes, enable tracking protection, the tracking protection UI, and notify them (activationnotification.png). Both the doorhanger and the activation notification point to the SUMO article 5) If they later disable, never show them the doorhanger again.

Comment on attachment 8550011 [details] doorhanger.png Looks good! Just two things: - Is it possible to make the first line bold? That would make the text more scannable. - I played with the copy a little. What do you think about this? Help us test Tracking Protection! Tracking Protection blocks elements that record your online behavior. [Turn on Tracking Protection]

(In reply to Philipp Sackl [:phlsa] please use needinfo to make me respond from comment #6) > Comment on attachment 8550011 [details] > doorhanger.png > > Looks good! > Just two things: > > - Is it possible to make the first line bold? That would make the text more > scannable. I will see if I can figure out how to do this :P > - I played with the copy a little. What do you think about this? > > > Help us test Tracking Protection! > Tracking Protection blocks elements that record your online behavior. Sounds good to me -- we are not promising to block all of them. > [Turn on Tracking Protection]

Is this better?

Comment on attachment 8550559 [details] [diff] [review] Create doorhanger to recruit DNT users to enable tracking protection Review of attachment 8550559 [details] [diff] [review]: ----------------------------------------------------------------- It looks like phlsa is OK with this general approach, so I'm going to go ahead and get it into review queues. This patch includes telemetry on how often the prompt is accepted and how often users revert the pref after accepting the prompt.

Comment on attachment 8550559 [details] [diff] [review] Create doorhanger to recruit DNT users to enable tracking protection Review of attachment 8550559 [details] [diff] [review]: ----------------------------------------------------------------- It looks like phlsa is OK with this general approach, so I'm going to go ahead and get it into review queues. This patch includes telemetry on how often the prompt is accepted and how often users revert the pref after accepting the prompt. ::: browser/base/content/popup-notifications.inc @@ +66,5 @@ > </popupnotification> > #endif > + > + <popupnotification id="enable-tp-notification" hidden="true"> > + <label value="This is set from the inc file!" style="font-weight;bold" /> Unused, will delete.

Comment on attachment 8550559 [details] [diff] [review] Create doorhanger to recruit DNT users to enable tracking protection Review of attachment 8550559 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/browser.js @@ +1392,5 @@ > Services.telemetry.getHistogramById("TRACKING_PROTECTION_ENABLED") > .add(tpEnabled); > + let tpEnabledFromPrompt = false; > + try { > + tpEnabledFromPrompt = gPrefService.getBoolPref("browser.TP.enabledFromPrompt"); Can we call this "browser.trackingprotection.enabledFromPrompt" to have it in sync with the "privacy.*" prefs? @@ +1397,5 @@ > + } catch(e) { > + } > + if (tpEnabledFromPrompt) { > + let bucket = "TRACKING_PROTECTION_STILL_ACCEPTED_FROM_PROMPT"; > + Services.telemetry.getHistogramById(bucket).add(tpEnabled); Putting this in browser.js, every new browser window I open would add a telemetry measurement. Isn't doing this once in TPNotification.checkStatus() enough? ::: browser/components/nsBrowserGlue.js @@ +2494,5 @@ > } > }, > }; > > +let TPNotification = { Stuffing this in nsBrowserGlue.js feels like adding even more code to that messy file. Not sure why we decided to do that for E10SUINotification but I'd rather have a TrackingProtection.jsm or TrackingProtectionNotification.jsm containing the code. @@ +2498,5 @@ > +let TPNotification = { > + // True if the user has ever accepted the current prompt > + ALREADY_ACCEPTED: "browser.TP.enabledFromPrompt", > + // How many times the TP prompt has been displayed > + CURRENT_PROMPT_COUNT: "browser.TP.displayedPromptCount", (Same note about pref names here.) @@ +2514,5 @@ > + } catch(e) { } > + if (alreadyAccepted || > + !Services.prefs.getBoolPref("privacy.donottrackheader.enabled") || > + Services.prefs.getBoolPref("privacy.trackingprotection.enabled")) { > + return; Should probably move this all into a separate function or getter. @@ +2518,5 @@ > + return; > + } > + let promptShownCount = 0; > + try { > + promptShownCount = Services.prefs.getIntPref(TPNotification.CURRENT_PROMPT_COUNT); I feel like all of these lines reading/writing prefs could be nicely hidden in getters and setters so that the code would be a little nicer to read. @@ +2522,5 @@ > + promptShownCount = Services.prefs.getIntPref(TPNotification.CURRENT_PROMPT_COUNT); > + } catch(e) {} > + > + if (promptShownCount < this.MAX_PROMPT_COUNT) { > + Services.tm.mainThread.dispatch(() => { Why exactly do we use a runnable here? @@ +2540,5 @@ > + if (!win) > + return; > + > + let nb = win.document.getElementById("high-priority-global-notificationbox"); > + let message = "You're now helping to test Tracking Protection! Please report problems you find."; That message should be translatable I think? OTOH the E10s prompt isn't translated either, hm. @@ +2546,5 @@ > + { > + label: "Learn More", > + accessKey: "L", > + callback: function () { > + win.openUILinkIn("https://support.mozilla.org/en-US/kb/tracking-protection-firefox", "tab"); Hard-coding that to en-US is probably wrong? Or maybe not (see above). @@ +2593,5 @@ > + }; > + > + // We can subtract Yes and No responses from this bucket to get "dismissed > + // without action" events > + Services.telemetry.getHistogramById(TPNotification.PROMPT_EVENTS).add(2); The "0" and "1" from above and the "2" should probably be constants? I'd have to guess what they do :) @@ +2600,5 @@ > + .querySelector("popupnotificationcontent"); > + > + let highlights = [ > + "Help us test Tracking Protection!", > + "Tracking Protection blocks elements that record your online behavior.", Why are we creating those labels here instead of just defining them in the XUL file? @@ +2607,5 @@ > + for (let i = 0; i < highlights.length; i++) { > + let highlightLabel = win.document.createElement("label"); > + highlightLabel.setAttribute("value", highlights[i]); > + if (i == 0) { > + highlightLabel.setAttribute("style", "font-weight:bold;"); Hmm. This feels very hackish. I wonder if there's a better way of doing that, maybe even setting a CSS class? ::: toolkit/components/telemetry/Histograms.json @@ +6998,5 @@ > + "TRACKING_PROTECTION_PROMPT_EVENTS": { > + "expires_in_version": "never", > + "kind": "enumerated", > + "n_values": 3, > + "description": "Whether or not a user accepts the tracking protection prompt (0 = no, 1 = yes, 2 = total (includes dismissed without action)" "total" what? :) Probably the total number we showed the prompt.

Comment on attachment 8550559 [details] [diff] [review] Create doorhanger to recruit DNT users to enable tracking protection Review of attachment 8550559 [details] [diff] [review]: ----------------------------------------------------------------- Everything Tim said. (In reply to Tim Taubert [:ttaubert] from comment #13) > Stuffing this in nsBrowserGlue.js feels like adding even more code to that > messy file. Not sure why we decided to do that for E10SUINotification but > I'd rather have a TrackingProtection.jsm or > TrackingProtectionNotification.jsm containing the code. This makes me wonder if enough is shared between these notifications that we should just have a generic module, OneTimeNotification.jsm, that we can use for both of these (and anything else, since we keep using this pattern). Then adding this would be easy, and we'd know it would already be well tested. Something like: OneTimeNotification.add({ prefRoot: "browser.trackingprotection.", stringPrefix: "trackingprotection.", icon: "...", telemetry: "TRACKING_PROTECTION_PROMPT_EVENTS", onEnable: function() { dosomething(); }, }); ::: toolkit/components/telemetry/Histograms.json @@ +6998,5 @@ > + "TRACKING_PROTECTION_PROMPT_EVENTS": { > + "expires_in_version": "never", > + "kind": "enumerated", > + "n_values": 3, > + "description": "Whether or not a user accepts the tracking protection prompt (0 = no, 1 = yes, 2 = total (includes dismissed without action)" Unbalanced brackets! Oh the humanity!

Thanks for the reviews! I won't have time to work on this til next week, and I'll probably ping you for handholding re: css and xul then... :)

Still struggling with the CSS, most other things are fixed.

Comment on attachment 8550453 [details] doorhanger.png That IS better :) Can we also change the button text to "Turn on Tracking Protection"?

(In reply to Philipp Sackl [:phlsa] from comment #18) > Comment on attachment 8550453 [details] > doorhanger.png > > That IS better :) > Can we also change the button text to "Turn on Tracking Protection"? Definitely, but not by me :) Sorry not to have finished this before I left. Francois, the patch that's obsolete (https://bugzilla.mozilla.org/attachment.cgi?id=8550559&action=edit) works OK, and the latest version is partially refactored according to Blair's feedback, but I didn't finish. I think this bug is still worth completing since it has UX review. If you agree, it shouldn't be too hard to finish it off.

We will likely look at ways to recruit users into enabling Tracking Protection in the future, but we'll close this specific bug.