Bug 1912040 - Remove all instances of onLegacyEngagement(). r=adw,urlbar-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D219068
This commit is contained in:
Karandeep
2024-08-14 12:38:26 +00:00
parent b12edbb492
commit d1dbab6e8d
10 changed files with 8 additions and 112 deletions

View File

@@ -604,7 +604,7 @@ export class UrlbarController {
/** /**
* Triggers a "dismiss" engagement for the selected result if one is selected * Triggers a "dismiss" engagement for the selected result if one is selected
* and it's not the heuristic. Providers that can respond to dismissals of * and it's not the heuristic. Providers that can respond to dismissals of
* their results should implement `onLegacyEngagement()`, handle the * their results should implement `onEngagement()`, handle the
* dismissal, and call `controller.removeResult()`. * dismissal, and call `controller.removeResult()`.
* *
* @param {Event} event * @param {Event} event

View File

@@ -107,7 +107,6 @@ class ProvidersManager {
onImpression: new Set(), onImpression: new Set(),
onAbandonment: new Set(), onAbandonment: new Set(),
onSearchSessionEnd: new Set(), onSearchSessionEnd: new Set(),
onLegacyEngagement: new Set(),
}; };
for (let [symbol, module] of Object.entries(localProviderModules)) { for (let [symbol, module] of Object.entries(localProviderModules)) {
let { [symbol]: provider } = ChromeUtils.importESModule(module); let { [symbol]: provider } = ChromeUtils.importESModule(module);
@@ -446,14 +445,6 @@ class ProvidersManager {
details details
); );
} }
this.#notifyLegacyEngagement(
this.providersByNotificationType.onLegacyEngagement,
state,
queryContext,
details,
controller
);
} }
#notifyEngagement(engagementProviders, queryContext, controller, details) { #notifyEngagement(engagementProviders, queryContext, controller, details) {
@@ -517,24 +508,6 @@ class ProvidersManager {
} }
} }
#notifyLegacyEngagement(
legacyEngagementProviders,
state,
queryContext,
details,
controller
) {
for (const provider of legacyEngagementProviders) {
provider.tryMethod(
"onLegacyEngagement",
state,
queryContext,
details,
controller
);
}
}
#globalAction = null; #globalAction = null;
async pickGlobalAction(queryContext, controller) { async pickGlobalAction(queryContext, controller) {

View File

@@ -2541,55 +2541,6 @@ export class UrlbarProvider {
* onSearchSessionEnd(_queryContext, _controller) {} * onSearchSessionEnd(_queryContext, _controller) {}
*/ */
/**
* Called when the user starts and ends an engagement with the urlbar. This is
* called for all providers who have implemented this method.
*
* @param {string} _state
* The state of the engagement, one of the following strings:
*
* engagement
* The user picked a result in the urlbar or used paste-and-go.
* abandonment
* The urlbar was blurred (i.e., lost focus).
* @param {UrlbarQueryContext} _queryContext
* The engagement's query context.
* @param {object} _details
* This object is non-empty only when `state` is "engagement" or
* "abandonment", and it describes the search string and engaged result.
*
* For "engagement", it has the following properties:
*
* {UrlbarResult} result
* The engaged result. If a result itself was picked, this will be it.
* If an element related to a result was picked (like a button or menu
* command), this will be that result. This property will be present if
* and only if `state` == "engagement", so it can be used to quickly
* tell when the user engaged with a result.
* {Element} element
* The picked DOM element.
* {boolean} isSessionOngoing
* True if the search session remains ongoing or false if the engagement
* ended it. Typically picking a result ends the session but not always.
* Picking a button or menu command may not end the session; dismissals
* do not, for example.
* {string} searchString
* The search string for the engagement's query.
* {number} selIndex
* The index of the picked result.
* {string} selType
* The type of the selected result. See TelemetryEvent.record() in
* UrlbarController.sys.mjs.
* {string} provider
* The name of the provider that produced the picked result.
*
* For "abandonment", only `searchString` is defined.
* @param {UrlbarController} _controller
* The associated controller.
*
* onLegacyEngagement(_state, _queryContext, _details, _controller) {}
*/
/** /**
* Called before a result from the provider is selected. See `onSelection` * Called before a result from the provider is selected. See `onSelection`
* for details on what that means. * for details on what that means.

View File

@@ -153,7 +153,7 @@ appropriate to your use case.
If any elements created in the view for your results can be picked with the If any elements created in the view for your results can be picked with the
keyboard or mouse, then be sure to implement your provider's keyboard or mouse, then be sure to implement your provider's
``onLegacyEngagement`` method. ``onEngagement`` method.
For help on implementing providers in general, see the address bar's For help on implementing providers in general, see the address bar's
`Architecture Overview`__. `Architecture Overview`__.

View File

@@ -1513,10 +1513,6 @@ class TestProvider extends UrlbarProvider {
* {@link UrlbarView.#selectElement} method is called. * {@link UrlbarView.#selectElement} method is called.
* @param {Function} [options.onEngagement] * @param {Function} [options.onEngagement]
* If given, a function that will be called when engagement. * If given, a function that will be called when engagement.
* @param {Function} [options.onLegacyEngagement]
* If given, a function that will be called when engagement.
* onLegacyEngagement() is implemented for those who rely on the
* older implementation of onEngagement()
* @param {Function} [options.onAbandonment] * @param {Function} [options.onAbandonment]
* If given, a function that will be called when abandonment. * If given, a function that will be called when abandonment.
* @param {Function} [options.onImpression] * @param {Function} [options.onImpression]
@@ -1540,7 +1536,6 @@ class TestProvider extends UrlbarProvider {
onAbandonment = null, onAbandonment = null,
onImpression = null, onImpression = null,
onSearchSessionEnd = null, onSearchSessionEnd = null,
onLegacyEngagement = null,
delayResultsPromise = null, delayResultsPromise = null,
} = {}) { } = {}) {
if (delayResultsPromise && addTimeout) { if (delayResultsPromise && addTimeout) {
@@ -1579,10 +1574,6 @@ class TestProvider extends UrlbarProvider {
if (onSearchSessionEnd) { if (onSearchSessionEnd) {
this.onSearchSessionEnd = onSearchSessionEnd.bind(this); this.onSearchSessionEnd = onSearchSessionEnd.bind(this);
} }
if (onLegacyEngagement) {
this.onLegacyEngagement = onLegacyEngagement.bind(this);
}
} }
get name() { get name() {

View File

@@ -247,7 +247,7 @@ add_task(async function firefoxSuggest() {
Assert.greater( Assert.greater(
onEngagementCallCount, onEngagementCallCount,
0, 0,
"onLegacyEngagement() should have been called" "onEngagement() should have been called"
); );
Assert.equal( Assert.equal(
UrlbarTestUtils.getResultCount(window), UrlbarTestUtils.getResultCount(window),

View File

@@ -11,7 +11,7 @@ add_setup(async function () {
await initExposureTest(); await initExposureTest();
}); });
add_task(async function exposureSponsoredOnLegacyEngagement() { add_task(async function exposureSponsoredOnEngagement() {
await doExposureTest({ await doExposureTest({
prefs: [ prefs: [
["browser.urlbar.exposureResults", suggestResultType("adm_sponsored")], ["browser.urlbar.exposureResults", suggestResultType("adm_sponsored")],

View File

@@ -10,7 +10,6 @@ const TEST_URL = "https://example.com/";
add_task(async function () { add_task(async function () {
await setup(); await setup();
let legacyEngagementDeferred = Promise.withResolvers();
let onEngagementDeferred = Promise.withResolvers(); let onEngagementDeferred = Promise.withResolvers();
const provider = new UrlbarTestUtils.TestProvider({ const provider = new UrlbarTestUtils.TestProvider({
results: [ results: [
@@ -27,14 +26,6 @@ add_task(async function () {
), ),
], ],
priority: 999, priority: 999,
onLegacyEngagement: () => {
info("Blur the address bar during the onLegacyEngagement notification");
gURLBar.blur();
// Run at the next tick to be sure spurious events would have happened.
TestUtils.waitForTick().then(() => {
legacyEngagementDeferred.resolve();
});
},
onEngagement: () => { onEngagement: () => {
info("Blur the address bar during the onEngagement notification"); info("Blur the address bar during the onEngagement notification");
gURLBar.blur(); gURLBar.blur();
@@ -46,7 +37,6 @@ add_task(async function () {
}); });
UrlbarProvidersManager.registerProvider(provider); UrlbarProvidersManager.registerProvider(provider);
// This should cover at least engagement and abandonment. // This should cover at least engagement and abandonment.
let legacyEngagementSpy = sinon.spy(provider, "onLegacyEngagement");
let engagementSpy = sinon.spy(provider, "onEngagement"); let engagementSpy = sinon.spy(provider, "onEngagement");
let beforeRecordCall = false, let beforeRecordCall = false,
@@ -69,22 +59,12 @@ add_task(async function () {
await openPopup("example"); await openPopup("example");
await selectRowByURL(TEST_URL); await selectRowByURL(TEST_URL);
EventUtils.synthesizeKey("VK_RETURN"); EventUtils.synthesizeKey("VK_RETURN");
await Promise.all([legacyEngagementDeferred, onEngagementDeferred]); await Promise.all([onEngagementDeferred]);
assertEngagementTelemetry([{ engagement_type: "enter" }]); assertEngagementTelemetry([{ engagement_type: "enter" }]);
assertAbandonmentTelemetry([]); assertAbandonmentTelemetry([]);
Assert.ok(recordReentered, "`record()` was re-entered"); Assert.ok(recordReentered, "`record()` was re-entered");
Assert.equal(
legacyEngagementSpy.callCount,
1,
"`onLegacyEngagement` was invoked once"
);
Assert.equal(
legacyEngagementSpy.args[0][0],
"engagement",
"`engagement` notified"
);
Assert.equal(engagementSpy.callCount, 1, "`onEngagement` was invoked once"); Assert.equal(engagementSpy.callCount, 1, "`onEngagement` was invoked once");
}); });
}); });

View File

@@ -3884,6 +3884,9 @@ async function checkSearch({ name, searchString, expectedResults }) {
removeResult() {}, removeResult() {},
}, },
}); });
// If this test is ever re-enabled, this line will need to be updated for the
// new engagement API (onEngagement())
UrlbarProviderQuickSuggest.onLegacyEngagement( UrlbarProviderQuickSuggest.onLegacyEngagement(
"engagement", "engagement",
context, context,

View File

@@ -28,7 +28,6 @@ add_setup(async function () {
onAbandonment: () => {}, onAbandonment: () => {},
onImpression: () => {}, onImpression: () => {},
onSearchSessionEnd: () => {}, onSearchSessionEnd: () => {},
onLegacyEngagement: () => {},
}); });
secondProvider = new UrlbarTestUtils.TestProvider({ secondProvider = new UrlbarTestUtils.TestProvider({
@@ -278,7 +277,6 @@ add_task(async function testProviderPresenceInMap() {
"onAbandonment", "onAbandonment",
"onImpression", "onImpression",
"onSearchSessionEnd", "onSearchSessionEnd",
"onLegacyEngagement",
]; ];
for (const method of notificationMethods) { for (const method of notificationMethods) {