1314650 - Fix the "Unknown source for one-off search: urlbar" error when doing one-off searches

Status: RESOLVED FIXED

(Firefox :: Search, defect, P1)

Description

[Alessio Placitelli [:Dexter]]

> Unknown source for one-off search: urlbar  BrowserUsageTelemetry.jsm:276

This is happening due to the code that landed in bug 1303333. The counts from the one-off searches originating from the urlbar or the searchbar are still being counted (checked in about:telemetry).

However, this exception is thrown because the search code calls BrowserUsageTelemetry.recordSearch twice: one time flagging the search as a one-off, the other time marking it as a normal search.

As a consequence, BrowserUsageTelemetry.recordSearch throws.

Comment 1

[Alessio Placitelli [:Dexter]]

Attachment: bug1314650.patch

This simply drops the exception if it comes from the 'urlbar' or 'searchbar', as they're the ones calling the function twice.

Comment 2

[Marco Bonardo [:mak]]

Comment on attachment 8806789 [details] [diff] [review]
bug1314650.patch

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

::: browser/modules/BrowserUsageTelemetry.jsm
@@ +274,5 @@
>      if (isOneOff) {
>        if (!KNOWN_ONEOFF_SOURCES.includes(source)) {
> +        // Silently drop the error if this bogus call
> +        // came from 'urlbar' or 'searchbar'. They're buggy
> +        // and are calling |recordSearch| twice.

If we're calling them buggy, we'd better add here the bug # that is going to solve the problem, otherwise future readers will have to figure that out every time.

Comment 3

[Alessio Placitelli [:Dexter]]

Attachment: bug1314650.patch

I changed the comment to explain the situation rather than claiming the code is buggy.

Comment 4

[Alessio Placitelli [:Dexter]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=60d15f199003

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/43b69e748e78
Fix the "Unknown source for one-off search: urlbar" error when doing one-off searches. r=mak

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/43b69e748e78

Comment 7

[Alessio Placitelli [:Dexter]]

Comment on attachment 8807240 [details] [diff] [review]
bug1314650.patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1303333.
[User impact if declined]: The browser console will show an exception even though no error really occurred.
[Describe test coverage new/current, TreeHerder]: Test coverage is already in place from bug 1303333.
[Risks and why]: No risk, as this basically mutes an exception.
[String/UUID change made/needed]: None.

Comment 8

[Gerry Chang [:gchang]]

Hi Alessio,
I remember that one-off search is enabled in nightly only. Do we need to uplift this to 51 aurora?

Comment 9

[Alessio Placitelli [:Dexter]]

(In reply to Gerry Chang [:gchang] from comment #8)
> Hi Alessio,
> I remember that one-off search is enabled in nightly only. Do we need to
> uplift this to 51 aurora?

Hi Gerry!

Yes please, let's uplift that too so that the code is in sync in case any weird issue arises.

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8807240 [details] [diff] [review]
bug1314650.patch

Fix an error in browser console related to one-off search. Take it in 51 aurora.

Comment 11

[Alessio Placitelli [:Dexter]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b07de805a13

Comment 12

[Alessio Placitelli [:Dexter]]

Aurora changeset: https://hg.mozilla.org/releases/mozilla-aurora/rev/3da399e85ae56c913ca67fb0e7bff5732640d5f1