Closed Bug 1002318 Opened 10 years ago Closed 10 years ago

Add UI Telemetry for sanitizing private data

Categories

(Firefox for Android Graveyard :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(firefox31 verified, firefox32 verified)

VERIFIED FIXED
Firefox 32
Tracking Status
firefox31 --- verified
firefox32 --- verified

People

(Reporter: mfinkle, Assigned: mfinkle)

References

Details

Attachments

(2 files)

Simple patch to add an Event for sanitizing private data.

Notes:
1. The event is fired after the sanitizing has completed. If we want to capture just opening the dialog, we can do that too, but is there value?
2. I added a "dialog" Method thinking that there might be other entry points for clearing data. Maybe through add-ons? I could be eaisly swayed to remove the "dialog" Method.

Are there other entry points we should care about?

Chenxia for the Event and Brian for the sanitize coverage.
Attachment #8413523 - Flags: review?(liuche)
Attachment #8413523 - Flags: review?(bnicholson)
This code was not removed when it could have been in bug 771036
Assignee: nobody → mark.finkle
Attachment #8413525 - Flags: review?(bnicholson)
Comment on attachment 8413523 [details] [diff] [review]
uitelemetry-sanitize v0.1

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

(In reply to Mark Finkle (:mfinkle) from comment #0)
> Created attachment 8413523 [details] [diff] [review]
> uitelemetry-sanitize v0.1
> 
> Simple patch to add an Event for sanitizing private data.
> 
> Notes:
> 1. The event is fired after the sanitizing has completed.

Note that it's fired after the the Java-side sanitization, but not necessarily before the Gecko sanitization. If you want to wait until everything is complete, the telemetry event should be fired here: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/preferences/GeckoPreferences.java#304.

Not sure if there's much value capturing right when the dialog is opened, but there may be value in waiting until after we get Sanitize:Finished since we also have a success boolean we could pass along.
Attachment #8413523 - Flags: review?(bnicholson) → review+
Attachment #8413525 - Flags: review?(bnicholson) → review+
Comment on attachment 8413523 [details] [diff] [review]
uitelemetry-sanitize v0.1

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

I'm kind of okay with having the sending of the Java message trigger the UI telemetry event, because technically, this is UI Telemetry, where we care about what the user is doing/tapping, but there's might be value in waiting for the Sanitize:Finished message from Gecko.

::: mobile/android/base/TelemetryContract.java
@@ +33,5 @@
>          // Sharing content.
>          public static final String SHARE = "share.1";
> +
> +        // Sanitizing private data.
> +        public static final String SANITIZE = "sanitize.1";

My first impression is that sanitize is what we do when we remove user-specific data from logs, not clearing private data. But I guess "Sanitize" is what we use for the Gecko message, so that's probably fine.

@@ +48,5 @@
>          // Action triggered from a button.
>          public static final String BUTTON = "button";
> +
> +        // Action triggered from a dialog.
> +        public static final String DIALOG = "dialog";

We could also reuse "dialog" when we launch contextmenus too.
Attachment #8413523 - Flags: review?(liuche) → review+
(In reply to Brian Nicholson (:bnicholson) from comment #2)
> Note that it's fired after the the Java-side sanitization

Actually, the Java-side sanitization is triggered by Gecko [1], so the Java-side sanitization may not have happened yet either. But if all we want to track is whether the user clicked the button without caring about the sanitization itself, it doesn't really matter.

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/modules/Sanitizer.jsm#141
remote:   https://hg.mozilla.org/integration/fx-team/rev/6fcc6dae5409
remote:   https://hg.mozilla.org/integration/fx-team/rev/ce243803cab7

Chenxia's point about UI Telemetry being used to watch what the user taps, not what functionality really does, hit home to to me. I moved the Event to occur before any sanitizing actually completed, to the place where we know the dialog had a "positive" button press. So as soon as the user decided to start the sanitization.
https://hg.mozilla.org/mozilla-central/rev/6fcc6dae5409
https://hg.mozilla.org/mozilla-central/rev/ce243803cab7
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Comment on attachment 8413523 [details] [diff] [review]
uitelemetry-sanitize v0.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: We miss out on getting some "before changes" telemetry
Testing completed (on m-c, etc.): Working and being analyzed
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none
Attachment #8413523 - Flags: approval-mozilla-aurora?
Comment on attachment 8413525 [details] [diff] [review]
uitelemetry-sanitize-other v0.1

See above
Attachment #8413525 - Flags: approval-mozilla-aurora?
Attachment #8413525 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8413523 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
The following telemetry probe is logged when clearing private data:
SendUIEvent: action = sanitize.1 method = dialog timestamp = 3040083 extras = null


Verified as fixed in builds:
- 32.0a1 (2014-05-25);
- 31.0a2 (2014-05-25);

Device: Alcatel One Touch 8008D (Android 4.1.2).
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.