Closed Bug 1130210 Opened 7 years ago Closed 7 years ago
.search .show One Off Buttons value in telemetry UI
It's really hard to interpret the telemetry UI data around search if the user has reverted to the old style search box. We should add this info to the telemetry UI data packet so we can filter these users out (or look at how their behavior differs from other users).
We want to do something similar to https://dxr.mozilla.org/mozilla-central/source/browser/modules/BrowserUITelemetry.jsm#496 But put the new check near the other search check at https://dxr.mozilla.org/mozilla-central/source/browser/modules/BrowserUITelemetry.jsm#569
Whiteboard: [good first bug] [lang=js]
Hi, I am new to open source and would like to work on this bug as my first bug. How should I proceed? Thanks in advance abhilash
Hello Abhilash! The first step is to get Firefox building. There are reasonably easy instructions at https://developer.mozilla.org/en-US/docs/Simple_Firefox_build but if you run into any problems please feel free to comment here, or email me!
Hello Mr. Blinton. I successfully built firefox yesterday with help from irc people. :)
Excellent! So the next thing to do is copy the line at https://dxr.mozilla.org/mozilla-central/source/browser/modules/BrowserUITelemetry.jsm#497 paste it down at line 569, and change browser.tabs.drawInTitlebar to browser.search.showOneOffButtons. (You'll also need to change result.titleBarEnabled to something different, like result.oneOffSearchEnabled…) Once you're done that, re-build Firefox, and go to "about:telemetry". If you expand the "Simple Measurements" section you should see the new addition beside the word "UITelemetry". Let me know how it goes!
Okay, I did those changes, and now build is running. #Excitement
I did the mentioned changes. However, there was no addition "besides" UITelemetry. There is an addition "activeTicks" in that same coloumn in Simple measurements section.
Okay Mr. Winton I had not enabled telemetry earlier. Now when I enabled it and run nightly, this is what I got beside UITelemetry : http://pastebin.mozilla.org/8616494 Is that what was expected? - Abhilash
A-ha! You see the text: "OneOffSearchEnabled":false in the pastebin? That's what we're looking for! :D Well, I _think_ that's what we're looking for. Can you tell me if you see the one-off search buttons at the bottom of your searchbar autocomplete? (The part that says "Search for test with:" in the picture at https://www.dropbox.com/s/t8wbbmpzucu6pjv/Screenshot%202015-02-08%2009.06.55.png?dl=0 ) If you do see those, then I would expect OneOffSearchEnabled to be true, not false, so we'll need to reverse that. And while you're changing that bit of code, it looks like all the other keys start with a lowercase letter, so we should probably change "OneOffSearchEnabled" to "oneOffSearchEnabled". After those two things are done, you should attach your patch, set its review request flag to ?, and ask :florian for the review. (This is probably one of the more complicated parts of writing code for Firefox, so if you can't figure it out, let me know, and I'll be happy to help. :)
Assignee: nobody → abhilashmhaisne
Status: NEW → ASSIGNED
Yeah! the part "search for test with:" does appear. And yes, I'll change "OneOffSearchEnabled" to "oneOffSearchEnabled".
Comment on attachment 8561025 [details] [diff] [review] Added browser.search.showOneOffbuttons value in telemetry UI Review of attachment 8561025 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks for the patch! :)
Attachment #8561025 - Flags: review?(florian) → review+
Blake, given that I'm planning to start working on bug 1119250 and remove that preference soon, if we want to get useful data out of adding this value in telemetry, we probably need to uplift this patch.
Thank you Blake and Florian.
Whiteboard: [good first bug] [lang=js] → [good first bug] [lang=js][fixed-in-fx-team]
Thank you florian. Cheers!
Comment on attachment 8561025 [details] [diff] [review] Added browser.search.showOneOffbuttons value in telemetry UI Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: no user impact. Developer impact will be less data available about how many of our users turned off the new search UI. This data would be interesting to collect before we remove this preference (bug 1119250). [Describe test coverage new/current, TreeHerder]: currently in m-c. [Risks and why]: Low risk, one-line patch. [String/UUID change made/needed]: none. I don't know if it's too late or not for 36. I think we get telemetry data primarily from our beta users, so it would be useful to have at least in 37 when it reaches beta.
Comment on attachment 8561025 [details] [diff] [review] Added browser.search.showOneOffbuttons value in telemetry UI It is a bit too late for beta indeed but ok for aurora!
Thank you Mr.Ledru and Mr.Queze!
You need to log in before you can comment on or make changes to this bug.