Closed Bug 1071120 Opened 6 years ago Closed 6 years ago

Telemetry for searching from error pages

Categories

(Firefox for Android :: General, defect)

15 Branch
x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 35
Tracking Status
firefox34 --- fixed
firefox35 --- fixed

People

(Reporter: wesj, Assigned: wesj)

Details

Attachments

(2 files, 1 obsolete file)

We should add some telemetry here.
Attached patch Patch (obsolete) — Splinter Review
This adds some telemetry here, for searches from either hitting the button or hitting enter in the search box. It also adds some for doing any edit to the search text. That's probably more than you want. Happy to rollback, just wanted to get a discussion started :)
Attachment #8495069 - Flags: review?(mark.finkle)
mfinkle ping?
Comment on attachment 8495069 [details] [diff] [review]
Patch

>diff --git a/mobile/android/modules/NetErrorHelper.jsm b/mobile/android/modules/NetErrorHelper.jsm

>+      let typed = false;
>       text.addEventListener("keypress", (event) => {
>         if (event.keyCode === KEY_CODE_ENTER) {
>+          UITelemetry.addEvent("neterror.1", "searchbox", "search");
>           this.doSearch(event.target.value);
>+        } else if (!typed) {
>+          UITelemetry.addEvent("neterror.1", "searchbox", "edit");
>+          typed = true;

Let's skip all of this and only send telemetry in doSearch

Also, you are calling UITelemetry.addEvent incorrectly:

    UITelemetry.addEvent("neterror.1", "button", "search");

should be:
    UITelemetry.addEvent("neterror.1", "button", null, "search");

Can you fix the two "wifi" calls too? And also prep a patch for Fx34 with the "wifi" fixes?
Attachment #8495069 - Flags: review?(mark.finkle) → review-
If we really care about the "button" or "keyboard" Method, then go ahead and add two calls, one in handleClick for "button" and one in the VK_ENTER block for "keyboard"
Attached patch Patch 1/2Splinter Review
Forgot this was r- and landed, but got an irc r+.
Attachment #8495069 - Attachment is obsolete: true
Attachment #8499029 - Flags: review+
Attached patch Patch 2/2Splinter Review
Also landed this fix for the wifi telemetry. Will nom this.
Attachment #8499031 - Flags: review+
Comment on attachment 8499031 [details] [diff] [review]
Patch 2/2

Approval Request Comment
[Feature/regressing bug #]: 1042196
[User impact if declined]: telemetry isn't filed correctly
[Describe test coverage new/current, TBPL]: none.
[Risks and why]: Low risk. Fixing arguments to telmetry methods.
[String/UUID change made/needed]: none.
Attachment #8499031 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/b34aa5d30c65
https://hg.mozilla.org/mozilla-central/rev/d353e7dd0ae8
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Comment on attachment 8499031 [details] [diff] [review]
Patch 2/2

Aurora+
Attachment #8499031 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.