Move quick-suggest ping submission from UrlbarProviderQuickSuggest to AdmWikipedia
Categories
(Firefox :: Address Bar, task, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox136 | --- | fixed |
People
(Reporter: adw, Assigned: adw)
References
Details
(Whiteboard: [sng])
Attachments
(1 file, 1 obsolete file)
In bug 1940808 we'll need to manually manage the lifetime of the quick-suggest ping by enabling it when Suggest is enabled and disabling it when Suggest is disabled -- or, only when AMP and Wikipedia suggestions are enabled/disabled, since they are all the ping is used for.
In preparation for that, I'd like to refactor how that ping is submitted. Right now UrlbarProviderQuickSuggest handles it, just like it has from the very start, but that's not the best place for it anymore. I'd like to move it to AdmWikipedia since that's the module responsible for AMP and Wikipedia suggestions. That way, the entire lifetime of the ping can be handled in the one place that makes the most sense, everything from enabling/disabling it to submitting it.
|
||
Updated•1 month ago
|
|
Assignee | |
Comment 1•1 month ago
|
||
This makes a number of changes. The main point is to fix the bug, and I did some
refactoring to accomplish that. Summary of changes:
Move submission of the quick-suggest ping from UrlbarProviderQuickSuggest to
AdmWikipedia.
Add SuggestProvider.onImpression() and onEngagement().
UrlbarProviderQuickSuggest forwards its own impression and engagement method
calls to the methods on the appropriate Suggest features. This is how
AdmWikipedia knows when to submit the ping.
Add a details param to UrlbarProvider.onImpression(), just like
onEngagement() already has. This is necessary because
AdmWikipedia.onImpression() needs to know whether the result was clicked since
that info is part of the impression ping.
Replace all SuggestFeature.handleCommand() implementations with
onEngagement().
Remove UrlbarProviderQuickSuggest.#dismissResult(). The only suggestion types
that relied on it were AMP and Wikipedia. I replaced it with dismissal handling
in AdmWikipedia.onEngagement().
Improve the related tests in a few ways: (1) Make the ping check more rigorous
(in assertQuickSuggestPing() in head.js), (2) simplify expected ping objects
(no more type and payload properties, just the properties that are expected
in the ping), (3) make browser_telemetry_gleanEmptyStrings.js more rigorous,
(4) factor out a helper function to trigger commands in xpcshell tests.
|
||
Updated•1 month ago
|
|
|
Assignee | |
Comment 2•1 month ago
|
||
This makes a number of changes. The main point is to fix the bug, and I did some
refactoring to accomplish that. Summary of changes:
Move submission of the quick-suggest ping from UrlbarProviderQuickSuggest to
AdmWikipedia.
Add SuggestProvider.onImpression() and onEngagement().
UrlbarProviderQuickSuggest forwards its own impression and engagement method
calls to the methods on the appropriate Suggest features. This is how
AdmWikipedia knows when to submit the ping.
Add a details param to UrlbarProvider.onImpression(), just like
onEngagement() already has. This is necessary because
AdmWikipedia.onImpression() needs to know whether the result was clicked since
that info is part of the impression ping.
Replace all SuggestFeature.handleCommand() implementations with
onEngagement().
Remove UrlbarProviderQuickSuggest.#dismissResult(). The only suggestion types
that relied on it were AMP and Wikipedia. I replaced it with dismissal handling
in AdmWikipedia.onEngagement().
Improve the related tests in a few ways: (1) Make the ping check more rigorous
(in assertQuickSuggestPing() in head.js), (2) simplify expected ping objects
(no more type and payload properties, just the properties that are expected
in the ping), (3) make browser_telemetry_gleanEmptyStrings.js more rigorous,
(4) factor out a helper function to trigger commands in xpcshell tests.
|
||
Comment 4•1 month ago
|
||
| bugherder | ||
Description
•