Closed
Bug 1496743
Opened 6 years ago
Closed 6 years ago
Stop logging telemetry when pressing enter on the one-off search settings button in the urlbar
Categories
(Firefox :: Address Bar, enhancement)
Firefox
Address Bar
Tracking
()
RESOLVED
FIXED
Firefox 64
Tracking | Status | |
---|---|---|
firefox64 | --- | fixed |
People
(Reporter: standard8, Assigned: standard8)
References
Details
Attachments
(1 file, 1 obsolete file)
In the urlbarBindings in handleCommand, we have this code: ``` // Determine whether to use the selected one-off search button. In // one-off search buttons parlance, "selected" means that the button // has been navigated to via the keyboard. So we want to use it if // the triggering event is not a mouse click -- i.e., it's a Return // key -- or if the one-off was mouse-clicked. let selectedOneOff = this.popup.oneOffSearchButtons.selectedButton; if (selectedOneOff && isMouseEvent && event.originalTarget != selectedOneOff) { selectedOneOff = null; } // Do the command of the selected one-off if it's not an engine. if (selectedOneOff && !selectedOneOff.engine) { selectedOneOff.doCommand(); return; } ``` I've done some digging around and I just can't see how that second part is triggered - the preferences button on the one-offs bar doesn't get to the handleCommand method, and the right-click menu is doesn't trigger it either. Try also shows that mochitests pass without it: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d63853d6ec7819dc559cec59afa0f154f0a98b3b Therefore, I think it can be removed, which will make the code slightly simpler and make it easier to figure out what to do with the new address bar implementation. I'll attach a patch, Drew please let me know if you think I'm wrong.
Assignee | ||
Comment 1•6 years ago
|
||
Comment 2•6 years ago
|
||
The second part is hit when you press the return key while the preferences button is selected.
Comment 3•6 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #0) > Try also shows that mochitests pass without it: > > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=d63853d6ec7819dc559cec59afa0f154f0a98b3b It's possible we don't have a test that specifically synthesizes an enter keypress on the settings button.
Assignee | ||
Comment 4•6 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #3) > (In reply to Mark Banner (:standard8) from comment #0) > > Try also shows that mochitests pass without it: > > > > https://treeherder.mozilla.org/#/ > > jobs?repo=try&revision=d63853d6ec7819dc559cec59afa0f154f0a98b3b > > It's possible we don't have a test that specifically synthesizes an enter > keypress on the settings button. It certainly looks like it. Do you know why we add a telemetry result to "FX_URLBAR_SELECTED_RESULT_METHOD" for category "enter" when this happens? We don't appear to do that for a click. Could there be something else this is handling which would need to be logged as well? The other one-off buttons don't hit this case. If we just need to re-route the command, and not log telemetry, this would make it easier for the newer urlbar design.
Flags: needinfo?(adw)
Comment 5•6 years ago
|
||
The fact that telemetry is recorded when you hit the return key on the settings button is just an accident I think, due to the fact that the return keypress gets routed through handleCommand. I don't think we really need to record that.
Flags: needinfo?(adw)
Assignee | ||
Comment 6•6 years ago
|
||
Ok, lets mutate this into not logging that telemetry, and I'll also add test coverage for the settings button.
Summary: Remove probably unnecessary search command fallback from urlbarBindings.xml handleCommands → Stop logging telemetry when clicking the one-off search settings button
Assignee | ||
Updated•6 years ago
|
Summary: Stop logging telemetry when clicking the one-off search settings button → Stop logging telemetry when pressing enter on the one-off search settings button
Assignee | ||
Updated•6 years ago
|
Summary: Stop logging telemetry when pressing enter on the one-off search settings button → Stop logging telemetry when pressing enter on the one-off search settings button in the urlbar
Updated•6 years ago
|
Attachment #9014813 -
Attachment is obsolete: true
Assignee | ||
Comment 7•6 years ago
|
||
Also add tests for the search settings button in the urlbar.
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/107392a74e8a Stop logging telemetry when pressing enter on the one-off search settings button in the urlbar. r=adw
Comment 9•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/107392a74e8a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in
before you can comment on or make changes to this bug.
Description
•