diff --git a/browser/components/urlbar/UrlbarInput.sys.mjs b/browser/components/urlbar/UrlbarInput.sys.mjs index 3caac3b56988..f385e56bae08 100644 --- a/browser/components/urlbar/UrlbarInput.sys.mjs +++ b/browser/components/urlbar/UrlbarInput.sys.mjs @@ -1238,19 +1238,13 @@ export class UrlbarInput { return; } case lazy.UrlbarUtils.RESULT_TYPE.DYNAMIC: { - if (url) { - break; - } - url = result.payload.url; - // Keep the searchMode for telemetry since handleRevert sets it to null. - const searchMode = this.searchMode; - // Do not revert the Urlbar if we're going to navigate. We want the URL - // populated so we can navigate to it. - if (!url || !result.payload.shouldNavigate) { + if (!url) { + // If we're not loading a URL, the engagement is done. First revert + // and then record the engagement since providers expect the urlbar to + // be reverted when they're notified of the engagement, but before + // reverting, copy the search mode since it's nulled on revert. + const { searchMode } = this; this.handleRevert(); - } - // If we won't be navigating, this is the end of the engagement. - if (!url || !result.payload.shouldNavigate) { this.controller.engagementEvent.record(event, { result, element, diff --git a/browser/components/urlbar/UrlbarMuxerUnifiedComplete.sys.mjs b/browser/components/urlbar/UrlbarMuxerUnifiedComplete.sys.mjs index da87aa1ad002..2a4e932fac69 100644 --- a/browser/components/urlbar/UrlbarMuxerUnifiedComplete.sys.mjs +++ b/browser/components/urlbar/UrlbarMuxerUnifiedComplete.sys.mjs @@ -816,10 +816,10 @@ class MuxerUnifiedComplete extends UrlbarMuxer { return false; } - // For tab-to-search results, result.payload.url is the engine's domain - // with the public suffix already stripped, for example "www.mozilla.". + // `searchUrlDomainWithoutSuffix` is the engine's domain with the public + // suffix already stripped, for example "www.mozilla.". let [engineDomain] = UrlbarUtils.stripPrefixAndTrim( - result.payload.url, + result.payload.searchUrlDomainWithoutSuffix, { stripWww: true, } diff --git a/browser/components/urlbar/UrlbarProviderTabToSearch.sys.mjs b/browser/components/urlbar/UrlbarProviderTabToSearch.sys.mjs index 6891814606b9..4f43f873ab0f 100644 --- a/browser/components/urlbar/UrlbarProviderTabToSearch.sys.mjs +++ b/browser/components/urlbar/UrlbarProviderTabToSearch.sys.mjs @@ -404,16 +404,12 @@ class ProviderTabToSearch extends UrlbarProvider { } function makeOnboardingResult(engine, satisfiesAutofillThreshold = false) { - let [url] = UrlbarUtils.stripPrefixAndTrim(engine.searchUrlDomain, { - stripWww: true, - }); - url = url.substr(0, url.length - engine.searchUrlPublicSuffix.length); let result = new lazy.UrlbarResult( UrlbarUtils.RESULT_TYPE.DYNAMIC, UrlbarUtils.RESULT_SOURCE.SEARCH, { engine: engine.name, - url, + searchUrlDomainWithoutSuffix: searchUrlDomainWithoutSuffix(engine), providesSearchMode: true, icon: UrlbarUtils.ICON.SEARCH_GLASS, dynamicType: DYNAMIC_RESULT_TYPE, @@ -426,17 +422,13 @@ function makeOnboardingResult(engine, satisfiesAutofillThreshold = false) { } function makeResult(context, engine, satisfiesAutofillThreshold = false) { - let [url] = UrlbarUtils.stripPrefixAndTrim(engine.searchUrlDomain, { - stripWww: true, - }); - url = url.substr(0, url.length - engine.searchUrlPublicSuffix.length); let result = new lazy.UrlbarResult( UrlbarUtils.RESULT_TYPE.SEARCH, UrlbarUtils.RESULT_SOURCE.SEARCH, ...lazy.UrlbarResult.payloadAndSimpleHighlights(context.tokens, { engine: engine.name, isGeneralPurposeEngine: engine.isGeneralPurposeEngine, - url, + searchUrlDomainWithoutSuffix: searchUrlDomainWithoutSuffix(engine), providesSearchMode: true, icon: UrlbarUtils.ICON.SEARCH_GLASS, query: "", @@ -447,5 +439,12 @@ function makeResult(context, engine, satisfiesAutofillThreshold = false) { return result; } +function searchUrlDomainWithoutSuffix(engine) { + let [value] = UrlbarUtils.stripPrefixAndTrim(engine.searchUrlDomain, { + stripWww: true, + }); + return value.substr(0, value.length - engine.searchUrlPublicSuffix.length); +} + export var UrlbarProviderTabToSearch = new ProviderTabToSearch(); initializeDynamicResult(); diff --git a/browser/components/urlbar/UrlbarUtils.sys.mjs b/browser/components/urlbar/UrlbarUtils.sys.mjs index 2f3757c6c870..64e9a307de7f 100644 --- a/browser/components/urlbar/UrlbarUtils.sys.mjs +++ b/browser/components/urlbar/UrlbarUtils.sys.mjs @@ -570,38 +570,34 @@ export var UrlbarUtils = { }, /** - * Extracts an url from a result, if possible. + * Extracts the URL from a result. * - * @param {UrlbarResult} result The result to extract from. - * @returns {object} a {url, postData} object, or null if a url can't be built - * from this result. + * @param {UrlbarResult} result + * The result to extract from. + * @returns {object} + * An object: `{ url, postData }` + * `url` will be null if the result doesn't have a URL. `postData` will be + * null if the result doesn't have post data. */ getUrlFromResult(result) { - switch (result.type) { - case UrlbarUtils.RESULT_TYPE.URL: - case UrlbarUtils.RESULT_TYPE.REMOTE_TAB: - case UrlbarUtils.RESULT_TYPE.TAB_SWITCH: - return { url: result.payload.url, postData: null }; - case UrlbarUtils.RESULT_TYPE.KEYWORD: - return { - url: result.payload.url, - postData: result.payload.postData - ? this.getPostDataStream(result.payload.postData) - : null, - }; - case UrlbarUtils.RESULT_TYPE.SEARCH: { - if (result.payload.engine) { - const engine = Services.search.getEngineByName(result.payload.engine); - let [url, postData] = this.getSearchQueryUrl( - engine, - result.payload.suggestion || result.payload.query - ); - return { url, postData }; - } - break; - } + if ( + result.type == UrlbarUtils.RESULT_TYPE.SEARCH && + result.payload.engine + ) { + const engine = Services.search.getEngineByName(result.payload.engine); + let [url, postData] = this.getSearchQueryUrl( + engine, + result.payload.suggestion || result.payload.query + ); + return { url, postData }; } - return { url: null, postData: null }; + + return { + url: result.payload.url ?? null, + postData: result.payload.postData + ? this.getPostDataStream(result.payload.postData) + : null, + }; }, /** @@ -1722,6 +1718,9 @@ UrlbarUtils.RESULT_PAYLOAD_SCHEMA = { satisfiesAutofillThreshold: { type: "boolean", }, + searchUrlDomainWithoutSuffix: { + type: "string", + }, suggestion: { type: "string", }, @@ -2090,12 +2089,6 @@ UrlbarUtils.RESULT_PAYLOAD_SCHEMA = { dynamicType: { type: "string", }, - // If `shouldNavigate` is `true` and the payload contains a `url` - // property, when the result is selected the browser will navigate to the - // `url`. - shouldNavigate: { - type: "boolean", - }, }, }, [UrlbarUtils.RESULT_TYPE.RESTRICT]: { diff --git a/browser/components/urlbar/docs/dynamic-result-types.rst b/browser/components/urlbar/docs/dynamic-result-types.rst index 2c81c1656fe1..daced925679b 100644 --- a/browser/components/urlbar/docs/dynamic-result-types.rst +++ b/browser/components/urlbar/docs/dynamic-result-types.rst @@ -614,10 +614,9 @@ useful in this case. URL Navigation ~~~~~~~~~~~~~~ -If a result's payload includes a string ``url`` property and a boolean -``shouldNavigate: true`` property, then picking the result will navigate to the -URL. The ``onLegacyEngagement`` method of the result's provider will still be called -before navigation. +If a result's payload includes a string ``url`` property, picking the result +will navigate to the URL. The ``onEngagement`` method of the result's provider +will be called before navigation. Text Highlighting ~~~~~~~~~~~~~~~~~ diff --git a/browser/components/urlbar/private/FakespotSuggestions.sys.mjs b/browser/components/urlbar/private/FakespotSuggestions.sys.mjs index 97fb13c3b8a3..1c5787fdd09a 100644 --- a/browser/components/urlbar/private/FakespotSuggestions.sys.mjs +++ b/browser/components/urlbar/private/FakespotSuggestions.sys.mjs @@ -130,7 +130,6 @@ export class FakespotSuggestions extends BaseFeature { totalReviews: Number(suggestion.totalReviews), fakespotGrade: suggestion.fakespotGrade, fakespotProvider: this.#parseProvider(suggestion), - shouldNavigate: true, dynamicType: "fakespot", }; diff --git a/browser/components/urlbar/private/Weather.sys.mjs b/browser/components/urlbar/private/Weather.sys.mjs index 51ff5b2f3f43..51bf360c8d48 100644 --- a/browser/components/urlbar/private/Weather.sys.mjs +++ b/browser/components/urlbar/private/Weather.sys.mjs @@ -361,7 +361,6 @@ export class Weather extends BaseFeature { forecast: suggestion.forecast.summary, high: suggestion.forecast.high[unit], low: suggestion.forecast.low[unit], - shouldNavigate: true, } ), { diff --git a/browser/components/urlbar/tests/browser/browser_dynamicResults.js b/browser/components/urlbar/tests/browser/browser_dynamicResults.js index 78bd25671080..31a04ec818fb 100644 --- a/browser/components/urlbar/tests/browser/browser_dynamicResults.js +++ b/browser/components/urlbar/tests/browser/browser_dynamicResults.js @@ -460,7 +460,7 @@ add_task(async function pick() { // Tests picking elements in a dynamic result. add_task(async function shouldNavigate() { /** - * A dummy provider that providers results with a `shouldNavigate` property. + * A dummy provider that providers results with a `url` property. */ class TestShouldNavigateProvider extends TestProvider { /** @@ -470,7 +470,6 @@ add_task(async function shouldNavigate() { async startQuery(context, addCallback) { for (let result of this.results) { result.payload.searchString = context.searchString; - result.payload.shouldNavigate = true; result.payload.url = DUMMY_PAGE; addCallback(this, result); } diff --git a/browser/components/urlbar/tests/quicksuggest/unit/head.js b/browser/components/urlbar/tests/quicksuggest/unit/head.js index 6c45ec5be5ac..6d1152a150c6 100644 --- a/browser/components/urlbar/tests/quicksuggest/unit/head.js +++ b/browser/components/urlbar/tests/quicksuggest/unit/head.js @@ -425,7 +425,6 @@ function makeWeatherResult({ forecast: MerinoTestUtils.WEATHER_SUGGESTION.forecast.summary, high: MerinoTestUtils.WEATHER_SUGGESTION.forecast.high[temperatureUnit], low: MerinoTestUtils.WEATHER_SUGGESTION.forecast.low[temperatureUnit], - shouldNavigate: true, }, }; diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest.js index 5bff7f7d303f..9c6cc59fb647 100644 --- a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest.js +++ b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest.js @@ -1526,7 +1526,9 @@ add_tasks_with_rust(async function tabToSearch() { makeSearchResult(context, { engineName: engine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost(engine.searchUrlDomain), + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( + engine.searchUrlDomain + ), providesSearchMode: true, query: "", providerName: "TabToSearch", diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_fakespot.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_fakespot.js index 6a6db8f84ba7..445d77fc721c 100644 --- a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_fakespot.js +++ b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_fakespot.js @@ -856,7 +856,6 @@ function makeExpectedResult({ totalReviews, fakespotGrade, fakespotProvider, - shouldNavigate: true, dynamicType: "fakespot", icon: null, }, diff --git a/browser/components/urlbar/tests/unit/head.js b/browser/components/urlbar/tests/unit/head.js index 71a478a2a7a6..91c392129819 100644 --- a/browser/components/urlbar/tests/unit/head.js +++ b/browser/components/urlbar/tests/unit/head.js @@ -711,6 +711,9 @@ function makeRemoteTabResult( * If the window to test is a private window. * @param {boolean} [options.isPrivateEngine] * If the engine is a private engine. + * @param {string} [options.searchUrlDomainWithoutSuffix] + * For tab-to-search results, the search engine domain without the public + * suffix. * @param {number} [options.type] * The type of the search result. Defaults to UrlbarUtils.RESULT_TYPE.SEARCH. * @param {number} [options.source] @@ -739,6 +742,7 @@ function makeSearchResult( providerName, inPrivateWindow, isPrivateEngine, + searchUrlDomainWithoutSuffix, heuristic = false, trending = false, isRichSuggestion = false, @@ -786,10 +790,11 @@ function makeSearchResult( payload.url = uri; } if (providerName == "TabToSearch") { - payload.satisfiesAutofillThreshold = satisfiesAutofillThreshold; - if (payload.url.startsWith("www.")) { - payload.url = payload.url.substring(4); + if (searchUrlDomainWithoutSuffix.startsWith("www.")) { + searchUrlDomainWithoutSuffix = searchUrlDomainWithoutSuffix.substring(4); } + payload.searchUrlDomainWithoutSuffix = searchUrlDomainWithoutSuffix; + payload.satisfiesAutofillThreshold = satisfiesAutofillThreshold; payload.isGeneralPurposeEngine = false; } diff --git a/browser/components/urlbar/tests/unit/test_autofill_origins_alt_frecency.js b/browser/components/urlbar/tests/unit/test_autofill_origins_alt_frecency.js index 41ff69acf28b..6e6c92dad3cb 100644 --- a/browser/components/urlbar/tests/unit/test_autofill_origins_alt_frecency.js +++ b/browser/components/urlbar/tests/unit/test_autofill_origins_alt_frecency.js @@ -106,7 +106,9 @@ add_task( makeSearchResult(context, { engineName: engine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost(engine.searchUrlDomain), + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( + engine.searchUrlDomain + ), providesSearchMode: true, query: "", providerName: "TabToSearch", diff --git a/browser/components/urlbar/tests/unit/test_providerTabToSearch.js b/browser/components/urlbar/tests/unit/test_providerTabToSearch.js index 32bf4506546f..15960615eba4 100644 --- a/browser/components/urlbar/tests/unit/test_providerTabToSearch.js +++ b/browser/components/urlbar/tests/unit/test_providerTabToSearch.js @@ -49,7 +49,9 @@ add_task(async function basic() { makeSearchResult(context, { engineName: testEngine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost(testEngine.searchUrlDomain), + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( + testEngine.searchUrlDomain + ), providesSearchMode: true, query: "", providerName: "TabToSearch", @@ -94,7 +96,9 @@ add_task(async function noAutofill() { makeSearchResult(context, { engineName: testEngine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost(testEngine.searchUrlDomain), + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( + testEngine.searchUrlDomain + ), providesSearchMode: true, query: "", providerName: "TabToSearch", @@ -145,7 +149,9 @@ add_task(async function ignoreWww() { makeSearchResult(context, { engineName: testEngine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost(testEngine.searchUrlDomain), + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( + testEngine.searchUrlDomain + ), providesSearchMode: true, query: "", providerName: "TabToSearch", @@ -179,7 +185,7 @@ add_task(async function ignoreWww() { makeSearchResult(context, { engineName: wwwTestEngine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost( + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( wwwTestEngine.searchUrlDomain ), providesSearchMode: true, @@ -207,7 +213,7 @@ add_task(async function ignoreWww() { makeSearchResult(context, { engineName: wwwTestEngine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost( + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( wwwTestEngine.searchUrlDomain ), providesSearchMode: true, @@ -266,7 +272,7 @@ add_task(async function conflictingEngines() { makeSearchResult(context, { engineName: fooTestEngine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost( + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( fooTestEngine.searchUrlDomain ), providesSearchMode: true, @@ -298,7 +304,7 @@ add_task(async function conflictingEngines() { makeSearchResult(context, { engineName: fooBarTestEngine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost( + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( fooBarTestEngine.searchUrlDomain ), providesSearchMode: true, @@ -361,7 +367,9 @@ add_task(async function multipleEnginesForHostname() { makeSearchResult(context, { engineName: testEngine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost(testEngine.searchUrlDomain), + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( + testEngine.searchUrlDomain + ), providesSearchMode: true, query: "", providerName: "TabToSearch", @@ -396,7 +404,9 @@ add_task(async function test_casing() { makeSearchResult(context, { engineName: testEngine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost(testEngine.searchUrlDomain), + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( + testEngine.searchUrlDomain + ), providesSearchMode: true, query: "", providerName: "TabToSearch", @@ -430,7 +440,9 @@ add_task(async function test_publicSuffix() { makeSearchResult(context, { engineName: engine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost(engine.searchUrlDomain), + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( + engine.searchUrlDomain + ), providesSearchMode: true, query: "", providerName: "TabToSearch", @@ -505,7 +517,9 @@ add_task(async function test_disabledEngine() { makeSearchResult(context, { engineName: engine.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: UrlbarUtils.stripPublicSuffixFromHost(engine.searchUrlDomain), + searchUrlDomainWithoutSuffix: UrlbarUtils.stripPublicSuffixFromHost( + engine.searchUrlDomain + ), providesSearchMode: true, query: "", providerName: "TabToSearch", diff --git a/browser/components/urlbar/tests/unit/test_providerTabToSearch_partialHost.js b/browser/components/urlbar/tests/unit/test_providerTabToSearch_partialHost.js index 22af945da792..151d7c7204b3 100644 --- a/browser/components/urlbar/tests/unit/test_providerTabToSearch_partialHost.js +++ b/browser/components/urlbar/tests/unit/test_providerTabToSearch_partialHost.js @@ -71,7 +71,7 @@ add_task(async function test() { makeSearchResult(context, { engineName: "TestEngine", engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: "en.example.", + searchUrlDomainWithoutSuffix: "en.example.", providesSearchMode: true, query: "", providerName: "TabToSearch", @@ -114,7 +114,7 @@ add_task(async function test() { makeSearchResult(context, { engineName: engine2.name, engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: "www.it.mochi.", + searchUrlDomainWithoutSuffix: "www.it.mochi.", providesSearchMode: true, query: "", providerName: "TabToSearch", @@ -162,7 +162,7 @@ add_task(async function test() { makeSearchResult(context, { engineName: "TestEngine3", engineIconUri: UrlbarUtils.ICON.SEARCH_GLASS, - uri: "search.foo.", + searchUrlDomainWithoutSuffix: "search.foo.", providesSearchMode: true, query: "", providerName: "TabToSearch",