1108013 - create dropdown doorhanger for recruiting DNT users to turn on tracking protection

Status: RESOLVED WONTFIX

(Firefox :: Protections UI, defect)

Description

[[:mmc] Monica Chew (no longer reading bugmail)]

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.

Comment 1

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Create doorhanger to recruit DNT users to enable tracking protection

Comment 2

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: doorhanger.png

Screenshot of doorhanger

Comment 3

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: activationnotice.png

Screenshot of activation notice

Comment 4

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Create doorhanger to recruit DNT users to enable tracking protection

Comment 5

[[:mmc] Monica Chew (no longer reading bugmail)]

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 6

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

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]

Comment 7

[[:mmc] Monica Chew (no longer reading bugmail)]

https://tbpl.mozilla.org/?tree=Try&rev=45eb725129d2

Comment 8

[[:mmc] Monica Chew (no longer reading bugmail)]

(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]

Comment 9

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: doorhanger.png

Is this better?

Comment 10

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Create doorhanger to recruit DNT users to enable tracking protection

Comment 11

[[:mmc] Monica Chew (no longer reading bugmail)]

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 12

[[:mmc] Monica Chew (no longer reading bugmail)]

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 13

[Tim Taubert [:ttaubert] (inactive)]

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 14

[Blair McBride [:Unfocused] (UNAVAILABLE)]

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!

Comment 15

[[:mmc] Monica Chew (no longer reading bugmail)]

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... :)

Comment 16

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Create doorhanger to recruit DNT users to enable tracking protection

Comment 17

[[:mmc] Monica Chew (no longer reading bugmail)]

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

Comment 18

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Comment on attachment 8550453 [details]
doorhanger.png

That IS better :)
Can we also change the button text to "Turn on Tracking Protection"?

Comment 19

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: Create doorhanger to recruit DNT users to enable tracking protection

Comment 20

[[:mmc] Monica Chew (no longer reading bugmail)]

(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.

Comment 22

[Peter Dolanjski [:pdol]]

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