Closed Bug 1122122 Opened 5 years ago Closed 5 years ago

Add telemetry for when someone clicks the shield doorhanger

Categories

(Core :: DOM: Security, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: mmc, Assigned: mmc)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

On http://people.mozilla.org/~mchew/tracking-protection/, we know that people hardly disable or enable tracking protection per-site (around ~10 events/day for approximately 500-700 Nightly users). Either

1) Tracking protection is working well enough that people don't have to disable it per-site very often

or

2) People don't discover the doorhanger.

These are not mutually exclusive. We should count the number of times users click on the shield (whether because of mixed content or tracking protection) if tracking protection is enabled to see whether we can rule out 2).
Assignee: nobody → mmc
Status: NEW → ASSIGNED
Attachment #8549753 - Flags: review?(ttaubert)
I tested this manually by going to about:telemetry after clicking on the doorhanger.
Attachment #8549753 - Flags: review?(bmcbride)
Comment on attachment 8549753 [details] [diff] [review]
Count number of times the doorhanger is shown if tracking protection is active (

Review of attachment 8549753 [details] [diff] [review]:
-----------------------------------------------------------------

I don't really understand the changes here. Shouldn't we add a new value to the histogram that would cover when the <xul:menupopup> opens? If we want to know how many people click it then we should probably do something with a popupshown listener?

::: browser/base/content/urlbarBindings.xml
@@ +2246,5 @@
>                + "tracking-protection";
>          }
> +        if (Services.prefs.getBoolPref("privacy.trackingprotection.enabled")) {
> +          let histogram = Services.telemetry.getHistogramById("TRACKING_PROTECTION_EVENTS");
> +          histogram.add(0);

So we add this measurement even if no mixed-content or tp message is shown? Is the constructor called once for every browser window? Is it called every time we show the icon in the URL bar?

The documentation in Histograms.json says "0 = not shown" but at this point we're not checking for STATE_BLOCKED_TRACKING_CONTENT, are we?

::: toolkit/components/telemetry/Histograms.json
@@ +6981,5 @@
>    "TRACKING_PROTECTION_SHIELD": {
>      "expires_in_version": "never",
>      "kind": "enumerated",
>      "n_values": 4,
> +    "description": "Tracking protection shield (0 = not shown, 1 = loaded, 2 = blocked, 3 = due to mixed content"

Did we change something or was the documentation wrong here for 1 & 2?
Attachment #8549753 - Flags: review?(ttaubert)
(In reply to Tim Taubert [:ttaubert] from comment #3)
> Comment on attachment 8549753 [details] [diff] [review]
> Count number of times the doorhanger is shown if tracking protection is
> active (
> 
> Review of attachment 8549753 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I don't really understand the changes here. Shouldn't we add a new value to
> the histogram that would cover when the <xul:menupopup> opens? If we want to
> know how many people click it then we should probably do something with a
> popupshown listener?

If that's the correct way to count the number of times the doorhanger is shown I am happy to do that. I just wasn't sure.

> 
> ::: browser/base/content/urlbarBindings.xml
> @@ +2246,5 @@
> >                + "tracking-protection";
> >          }
> > +        if (Services.prefs.getBoolPref("privacy.trackingprotection.enabled")) {
> > +          let histogram = Services.telemetry.getHistogramById("TRACKING_PROTECTION_EVENTS");
> > +          histogram.add(0);
> 
> So we add this measurement even if no mixed-content or tp message is shown?

It seems to be counted every time the doorhanger is shown. At least, that's what I recollect from https://bugzilla.mozilla.org/show_bug.cgi?id=1058133#c6 and manual tests show.

> Is the constructor called once for every browser window? Is it called every
> time we show the icon in the URL bar?

I don't think so, we already have a histogram for the number of times the shield shows due to tracking protection.

> The documentation in Histograms.json says "0 = not shown" but at this point
> we're not checking for STATE_BLOCKED_TRACKING_CONTENT, are we?

This is an attempt to fix "Doorhanger shown = 0" in TRACKING_PROTECTION_EVENTS.
> 
> ::: toolkit/components/telemetry/Histograms.json
> @@ +6981,5 @@
> >    "TRACKING_PROTECTION_SHIELD": {
> >      "expires_in_version": "never",
> >      "kind": "enumerated",
> >      "n_values": 4,
> > +    "description": "Tracking protection shield (0 = not shown, 1 = loaded, 2 = blocked, 3 = due to mixed content"
> 
> Did we change something or was the documentation wrong here for 1 & 2?

The documentation was wrong for 1 & 2, for a different histogram (which just counts the times the icon is shown). I noticed it when doing this bug. Sorry for the confusion!
The XBL binding gets re-applied every time the popup is shown, so is equivalent to popupshown.
Attachment #8549753 - Flags: review?(bmcbride) → review+
https://hg.mozilla.org/mozilla-central/rev/c1cdf1d40452
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.