Telemetry for Geolocation Prompt UI

RESOLVED FIXED in Firefox 17

Status

()

Firefox
Security
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: devd, Unassigned)

Tracking

unspecified
Firefox 18
x86
Linux
Points:
---

Firefox Tracking Flags

(firefox17 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

5 years ago
We would like to measure the efficacy of the geo-location prompt via the Security UI Telemetry (Bug 767676)
(Reporter)

Comment 1

5 years ago
Created attachment 657621 [details] [diff] [review]
Telemetry for Geolocation Prompt UI

Proposed Patch. Two cases are not covered because the popup notification API does not provide enough data to differentiate these cases. Does anyone have suggestions for how to fix this?
(Reporter)

Comment 2

5 years ago
The lack of the two cases shouldn't impede this patch imho. We can still measure how many people didn't respond to the prompt. What we will miss is: out of those who didn't respond, how many clicked 'X' and closed vs. how many just clicked outside the prompt to make it go away.

IMO, that doesn't matter for the security question. Anyone else have thoughts on this ?

Comment 3

5 years ago
(In reply to Devdatta Akhawe from comment #2)
> The lack of the two cases shouldn't impede this patch imho. We can still
> measure how many people didn't respond to the prompt. What we will miss is:
> out of those who didn't respond, how many clicked 'X' and closed vs. how
> many just clicked outside the prompt to make it go away.
> 
> IMO, that doesn't matter for the security question. Anyone else have
> thoughts on this ?

Yeah, I don't see any reason we'd really care about these differentiating those cases for our purposes - they seem the same to me.
(Reporter)

Updated

5 years ago
Attachment #657621 - Flags: review?(felipc)

Comment 4

5 years ago
dveditz suggested that if this finishes up soon we should try to get it uplifted to ff17/Aurora so it can ship with the rest of the UI telemetry probes.
Comment on attachment 657621 [details] [diff] [review]
Telemetry for Geolocation Prompt UI

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

::: security/manager/boot/public/nsISecurityUITelemetry.idl
@@ +90,5 @@
> +const uint32_t WARNING_GEOLOCATION_REQUEST = 46;
> +const uint32_t WARNING_GEOLOCATION_REQUEST_SHARE_LOCATION = 47;
> +const uint32_t WARNING_GEOLOCATION_REQUEST_ALWAYS_SHARE = 48;
> +const uint32_t WARNING_GEOLOCATION_REQUEST_NEVER_SHARE = 49;
> +//TODO: It would be nice to measure the two cases below too.

Don't add the TODO comment and unused values
Attachment #657621 - Flags: review?(felipc) → review+
(Reporter)

Comment 6

5 years ago
Thanks! How about I remove the unused values, but leave a comment saying just "Space for 2 constants since it would be nice to know how many people click outside the button"? I basically want to mark off space in the IDL for the constants.
(Reporter)

Comment 7

5 years ago
Created attachment 659415 [details] [diff] [review]
Telemetry for Geolocation Prompt UI
(Reporter)

Updated

5 years ago
Attachment #657621 - Attachment is obsolete: true
(Reporter)

Comment 8

5 years ago
Created attachment 659432 [details] [diff] [review]
Telemetry for Geolocation Prompt UI
(Reporter)

Updated

5 years ago
Attachment #659415 - Attachment is obsolete: true
Attachment #659432 - Flags: review+
I forgot one detail: you need to rev the uuid of this file. No need for re-review, just remember to add that before pushing
(Reporter)

Comment 10

5 years ago
Created attachment 659493 [details] [diff] [review]
Telemetry for Geolocation Prompt UI
(Reporter)

Updated

5 years ago
Attachment #659432 - Attachment is obsolete: true
(Reporter)

Comment 11

5 years ago
Comment on attachment 659493 [details] [diff] [review]
Telemetry for Geolocation Prompt UI

New uuid, carrying over the r+ from felipe
Attachment #659493 - Flags: review+

Comment 12

5 years ago
Dev pushed this to try : https://tbpl.mozilla.org/?tree=Try&rev=bc2f144e83b3 - looks good.

Comment 13

5 years ago
Pushed to inbound : https://hg.mozilla.org/integration/mozilla-inbound/rev/ec2b20c68ba2
https://hg.mozilla.org/mozilla-central/rev/ec2b20c68ba2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18

Comment 15

5 years ago
Comment on attachment 659493 [details] [diff] [review]
Telemetry for Geolocation Prompt UI

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: n/a
Testing completed (on m-c, etc.): has been on m-c since Monday
Risk to taking this patch (and alternatives if risky): should be very low - adding a new telemetry probe
String or UUID changes made by this patch: nsISecurityUITelemetry UUID change (interface was introduced in ff17, i believe)

we would like to uplift this probe to Aurora/17 as the rest of the security UI telemetry probes landed in 17 and we can then start collecting a comprehensive set of security UI data in 17.
Attachment #659493 - Flags: approval-mozilla-aurora?
Attachment #659493 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Since the UUID change has Telemetry in the name we are assuming there shouldn't be much fallout from taking this.

Comment 17

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/d648d93f9d76

Updated

5 years ago
status-firefox17: --- → fixed
Is there something QA can do to verify this fix?
Whiteboard: [qa?]
(Reporter)

Comment 19

5 years ago
I am afraid I am not the right person to answer that. One thing though: this was really an additional feature, not a fix. I don't see what QA can do beyond the basic testing I did.
I don't see much value in repeating what you've already tested, Devdatta. For now, flagging this [qa-]. Please change this whiteboard tag to the verifyme keyword if there is any requirement for manual verification from QA.

Thanks
Whiteboard: [qa?] → [qa-]
You need to log in before you can comment on or make changes to this bug.