1800184 - Quick suggest impression telemetry is recorded for results with large `rowIndex` values

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Drew Willcoxon :adw]

Rebecca noticed that the telemetry data shows a significant number of quick suggest impressions with large rowIndex values. In the data she was looking at, there were 10,000's of impressions with indexes between 10 and 20, and >10% of clients with impressions had an index >10. These are significant numbers that can't be explained by users with custom values for the maxRichResults hidden preference.

UrlbarProviderQuickSuggest records an impression for each result it adds, even if the result isn't actually visible in the view at the time the engagement happens. The only reason the result wouldn't be visible is if the query is canceled before it finishes and before the remove-stale-rows timer fires.

So the only thing I can think of is that users are triggering quick suggest results with their search strings but then hitting enter very quickly before the query finishes. UrlbarProviderQuickSuggest doesn't finish its startQuery() until it receives a response from Merino, and it will wait up to 200ms, so maybe this isn't as far-fetched as it seems. (That's true only when Merino is enabled, as it was until very recently for Beta and Nightly users.)

Regardless of the cause, it's definitely incorrect for UrlbarProviderQuickSuggest to record an impression when its result is hidden, so we should fix that anyway.

Comment 1

[Drew Willcoxon :adw]

Attachment: Bug 1800184 - Don't record quick suggest impression telemetry when the result is hidden.

This adds result.isVisible. Like rowIndex, it's updated by the view.
UrlbarProviderQuickSuggest will skip impression telemetry updates (and
impression stats updates too) when it's false.

Please see bug 1800184 for background.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8f96cc8676e
Don't record quick suggest impression telemetry when the result is hidden. r=dao

Comment 3

[Drew Willcoxon :adw]

I found some cases this fix doesn't cover and filed bug 1800810 for them. That should fix everything, including index values of zero that are also present in the data.

Comment 4

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/d8f96cc8676e

Comment 5

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/d8f96cc8676e

Comment 6

[Drew Willcoxon :adw]

Marking this qe-verify- because it's superseded by bug 1800810. I'll set qe-verify+ in that bug to make sure this gets covered.

Comment 7

[Drew Willcoxon :adw]

Comment on attachment 9303001 [details]
Bug 1800184 - Don't record quick suggest impression telemetry when the result is hidden.

Beta/Release Uplift Approval Request

• User impact if declined: This is an important fix to Firefox Suggest telemetry that we'd like sooner than later.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: Bug 1800810 should be uplifted after this one
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Not a user-visible change, has a test.
• String changes made/needed:
• Is Android affected?: No

Comment 8

[Dianna Smith [:diannaS]]

Comment on attachment 9303001 [details]
Bug 1800184 - Don't record quick suggest impression telemetry when the result is hidden.

Approved for 108.0b3

Comment 9

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3cd02db08e29