Closed
Bug 787738
Opened 9 years ago
Closed 9 years ago
Telemetry for Geolocation Prompt UI
Categories
(Firefox :: Security, defect)
Tracking
()
RESOLVED
FIXED
Firefox 18
Tracking | Status | |
---|---|---|
firefox17 | --- | fixed |
People
(Reporter: devd, Unassigned)
Details
(Whiteboard: [qa-])
Attachments
(1 file, 3 obsolete files)
4.88 KB,
patch
|
devd
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
We would like to measure the efficacy of the geo-location prompt via the Security UI Telemetry (Bug 767676)
Reporter | ||
Comment 1•9 years ago
|
||
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•9 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•9 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•9 years ago
|
Attachment #657621 -
Flags: review?(felipc)
Comment 4•9 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 5•9 years ago
|
||
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•9 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•9 years ago
|
||
Reporter | ||
Updated•9 years ago
|
Attachment #657621 -
Attachment is obsolete: true
Reporter | ||
Comment 8•9 years ago
|
||
Reporter | ||
Updated•9 years ago
|
Attachment #659415 -
Attachment is obsolete: true
Updated•9 years ago
|
Attachment #659432 -
Flags: review+
Comment 9•9 years ago
|
||
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•9 years ago
|
||
Reporter | ||
Updated•9 years ago
|
Attachment #659432 -
Attachment is obsolete: true
Reporter | ||
Comment 11•9 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•9 years ago
|
||
Dev pushed this to try : https://tbpl.mozilla.org/?tree=Try&rev=bc2f144e83b3 - looks good.
Comment 13•9 years ago
|
||
Pushed to inbound : https://hg.mozilla.org/integration/mozilla-inbound/rev/ec2b20c68ba2
Comment 14•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ec2b20c68ba2
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
Comment 15•9 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?
Updated•9 years ago
|
Attachment #659493 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 16•9 years ago
|
||
Since the UUID change has Telemetry in the name we are assuming there shouldn't be much fallout from taking this.
Updated•9 years ago
|
status-firefox17:
--- → fixed
Reporter | ||
Comment 19•9 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.
Comment 20•9 years ago
|
||
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.
Description
•