1536454 - Record additional event telemetry on user interaction with permission prompts

Status: VERIFIED FIXED

(Firefox :: Site Identity, enhancement, P1)

Description

[Johann Hofmann [:johannh]]

For limited field (shield) studies we would like to have precise event telemetry about different metric when a user interacts with permission prompts.

This could be used to build heuristics to automatically reject permission prompts in the future.

Comment 1

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe622e495ab53cc6a2c4580832938addba7c51ed

Comment 2

[Johann Hofmann [:johannh]]

Attachment: Bug 1536454 - Part 1 - Add userHadInteractedWithDocument and documentDOMContentLoadedTimestamp attributes to nsIContentPermissionPrompt. r=Ehsan

Comment 3

[Johann Hofmann [:johannh]]

Attachment: Bug 1536454 - Part 2 - Add modificationTime attribute to nsIPermission.idl. r=Ehsan

Comment 4

[Johann Hofmann [:johannh]]

Attachment: Bug 1536454 - Part 3 - Make POPUP_NOTIFICATION_STATS probe collect data on notification removal instead of dismissal. r=MattN

Almost none of the prompts that we are currently showing still get dismissed, which messes up our
measurements in this probe. Most of them are persistent now, which means that we should record when
they get removed instead of dismissed to receive meaningful data. This patch does that.

Comment 5

[Johann Hofmann [:johannh]]

Attachment: Bug 1536454 - Part 4 - Add event telemetry for permission prompt studies. r=Ehsan

Comment 6

[Johann Hofmann [:johannh]]

This is still lacking basic tests (we could test an endless combination of things but I'd like to have at least some basic assertions), and I will file the data review request after we're done with our conversation with legal on approving this study.

Thanks!

Comment 7

[Johann Hofmann [:johannh]]

Attachment: request.md

Comment 8

[Chris H-C :chutten]

Comment on attachment 9058548 [details]
request.md

DATA COLLECTION REVIEW RESPONSE:

    Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes. This collection is Telemetry so is documented in its definitions file [Events.yaml](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/Events.yaml) and the [Probe Dictionary](https://telemetry.mozilla.org/probe-dictionary/).

    Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.
Additionally, this collection is gated to a study, so users can disable studies to disable this collection.

    If the request is for permanent data collection, is there someone who will monitor the data over time?

No. This collection will expire in Firefox 72.
In addition, this collection is gated by a preference that will only be enabled as part of a study with limited duration.

    Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 3, Web Activity.
The "Web Activity" being collected is a "Was the site that prompted this on a list of known sites?". This has been deemed to be an acceptable mitigation to permit its opt-out collection on release.

    Is the data collection request for default-on or default-off?

Default on for all channels.

    Does the instrumentation include the addition of any new identifiers?

No.

    Is the data collection covered by the existing Firefox privacy notice?

Yes.

    Does there need to be a check-in in the future to determine whether to renew the data?

Yes. :johannh is responsible for renewing or removing the collection before it expires in Firefox 72.

---
Result: datareview+

Comment 9

[Johann Hofmann [:johannh]]

Attachment: Bug 1536454 - Part 5 - Add tests for permission prompt event telemetry. r=Ehsan

Comment 10

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9020aa4e744a371aaae270e5cf02be34772f262

Comment 11

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a3be8ff74910e365afaaed6737e5e96b7fa73e0

Comment 12

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d4de4f3587d7809c9b3f5f2a6c3b5a740447751

Comment 13

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c9e87664cb02808f9299e3fd7f5da3712b3b4ed

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/565279f28841
Part 1 - Add userHadInteractedWithDocument and documentDOMContentLoadedTimestamp attributes to nsIContentPermissionPrompt. r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/c4beb7256d16
Part 2 - Add modificationTime attribute to nsIPermission.idl. r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/8c6ec5c528ff
Part 3 - Make POPUP_NOTIFICATION_STATS probe collect data on notification removal instead of dismissal. r=MattN
https://hg.mozilla.org/integration/autoland/rev/19b40bd2e67f
Part 4 - Add event telemetry for permission prompt studies. r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/4b317b418c14
Part 5 - Add tests for permission prompt event telemetry. r=Ehsan

Comment 15

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/565279f28841
https://hg.mozilla.org/mozilla-central/rev/c4beb7256d16
https://hg.mozilla.org/mozilla-central/rev/8c6ec5c528ff
https://hg.mozilla.org/mozilla-central/rev/19b40bd2e67f
https://hg.mozilla.org/mozilla-central/rev/4b317b418c14

Comment 16

[Johann Hofmann [:johannh]]

Comment on attachment 9057302 [details]
Bug 1536454 - Part 1 - Add userHadInteractedWithDocument and documentDOMContentLoadedTimestamp attributes to nsIContentPermissionPrompt. r=Ehsan

Beta/Release Uplift Approval Request

• User impact if declined: None, this is a preffed-off telemetry collection that will only be used for a study we are planning to run in Release 67 (we already announced this to rel-man), see https://blog.nightly.mozilla.org/2019/04/01/reducing-notification-permission-prompt-spam-in-firefox/
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: We hope to get separate testing for the Telemetry itself (besides the automated tests), but it would be nice to get a sanity check that permission prompts are still working (see https://permission.site/) on Beta after uplift.
• List of other uplifts needed: To avoid merge pains we would need bug 1508961 uplifted, too. :/
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The main risk with this patch is it's size, and the fact that it depends on bug 1508961. Otherwise, this is entirely preffed-off, has extensive test coverage and should have no user facing consequences even if enabled.
• String changes made/needed: None

Comment 17

[Johann Hofmann [:johannh]]

List of other uplifts needed: To avoid merge pains we would need bug 1508961 uplifted, too. :/

I just realized we need bug 1524619 as well.

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Johann, do we have new telemetry data coming in from Nightly based on commits in comment 15? I am always a bit wary of uplifting telemetry related changes in Beta until we know we are seeing some of the data from Nightly users.

Comment 19

[Johann Hofmann [:johannh]]

(In reply to Ritu Kothari (:ritu) from comment #18)

Hi Johann, do we have new telemetry data coming in from Nightly based on commits in comment 15? I am always a bit wary of uplifting telemetry related changes in Beta until we know we are seeing some of the data from Nightly users.

Hi Ritu, that's a very valid concern. We initially turned this off in Nightly by default, because this was only intended for the experiment. But seeing that we made a last minute decision to remove the C3 data collection after all, it should be fine to enable in Nightly. I'll file a bug for it.

However, that still means we won't have Telemetry data coming in for another couple of days. I would still like to get this onto Beta as early as possible. My main concern is about breaking things by merging all these patches into Beta, so I'd prefer to be optimistic about the telemetry here. It has good test coverage and I just tried it myself on Nightly and things show up correctly in about:telemetry.

Maybe instead of waiting on the data, the QA team can try to verify this on Nightly first?

STR are:

• Enable permissions.eventTelemetry.enabled in about:config
• Go to https://permission.site, click on "Notifications" or "Geo"
• Visit about:telemetry -> Events
• You should see an entry for security.ui.permissionprompt with the show method and the correct type (geo or notifications)
• Respond to the prompt or leave the site
• Refresh about:telemetry
• You should see an entry for security.ui.permissionprompt with the method corresponding to what you did (allow, deny, leave)

Might also be nice to test this:

• Enable permissions.eventTelemetry.enabled in about:config
• Disable dom.webnotifications.requireuserinteraction
• Go to https://www.cnet.com/, notice a prompt is shown
• You should see an entry for security.ui.permissionprompt with the show method and the correct type (geo or notifications)
• Respond to the prompt or leave the site
• Refresh about:telemetry
• You should see an entry for security.ui.permissionprompt with the method corresponding to what you did (allow, deny, leave)

Brindusa, are you the right person to tackle this? :)

Thank you!

Comment 20

[Rares Doghi, Desktop QA]

Hi, I have retested this issue on our latest Version of Nightly and I couldn't find any issues, everything works according to plan.

Event>Telemetry shows the Correct Leave, Allow, Deny or Never messages according to my selection from the https://www.cnet.com/ website prompt.

Comment 21

[Rares Doghi, Desktop QA]

Forgot to mention that this issue was tested on all Operating Systems using Johann's steps from Comment 19. Thanks.

Comment 22

[Johann Hofmann [:johannh]]

Awesome, thanks for the quick confirmation ⚡️

Comment 23

[Pascal Chevrel:pascalc]

Comment on attachment 9057302 [details]
Bug 1536454 - Part 1 - Add userHadInteractedWithDocument and documentDOMContentLoadedTimestamp attributes to nsIContentPermissionPrompt. r=Ehsan

P1, preffed-off by default and QA verified on nightly. uplift approved for 67 beta14. I reset the qe-verify+ keyword as I would like this uplift to be tested manually again by QA once it is on beta. Thanks.

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/f71a18e81a69
https://hg.mozilla.org/releases/mozilla-beta/rev/fbe79e8981f7
https://hg.mozilla.org/releases/mozilla-beta/rev/fe7f1569e8d6
https://hg.mozilla.org/releases/mozilla-beta/rev/759d4a9a8970
https://hg.mozilla.org/releases/mozilla-beta/rev/156dcfca7e31

Comment 25

[Rares Doghi, Desktop QA]

Hi, I have retested this issue on all Operating systems with our latest Beta 67.0b14 and I can confirm this issue Verified as Fixed according to the steps from Comment 19.

I will mark this issue accordingly.