commit eaa1cae055f3f1c4cc8308dee31e8217b4ccd20e parent 7fc0395c52083d57b85aa4c0e6bacb8dc12f28b7 Author: Daisuke Akatsuka <daisuke@birchill.co.jp> Date: Tue, 28 Oct 2025 05:49:55 +0000 Bug 1995887: Use geolocation in GeolocationUtils if user does not provide the weather area r=adw Differential Revision: https://phabricator.services.mozilla.com/D269936 Diffstat:
12 files changed, 269 insertions(+), 3 deletions(-)
diff --git a/browser/components/urlbar/moz.build b/browser/components/urlbar/moz.build @@ -86,6 +86,7 @@ MOZ_SRC_FILES += [ ] TESTING_JS_MODULES += [ + "tests/quicksuggest/GeolocationTestUtils.sys.mjs", "tests/quicksuggest/MerinoTestUtils.sys.mjs", "tests/quicksuggest/QuickSuggestTestUtils.sys.mjs", "tests/quicksuggest/RemoteSettingsServer.sys.mjs", diff --git a/browser/components/urlbar/private/WeatherSuggestions.sys.mjs b/browser/components/urlbar/private/WeatherSuggestions.sys.mjs @@ -501,10 +501,14 @@ export class WeatherSuggestions extends SuggestProvider { async #fetchMerinoSuggestion(cityGeoname) { if (!this.#merino) { this.#merino = new lazy.MerinoClient(this.constructor.name, { + allowOhttp: true, cachePeriodMs: MERINO_WEATHER_CACHE_PERIOD_MS, }); } + let merino = this.#merino; + let fetchInstance = (this.#fetchInstance = {}); + // Set up location params to pass to Merino. We need to null-check each // suggestion property because `MerinoClient` will stringify null values. let otherParams = { source: "urlbar" }; @@ -524,10 +528,30 @@ export class WeatherSuggestions extends SuggestProvider { if (adminCodes) { otherParams.region = adminCodes; } + } else { + let geolocation = await lazy.GeolocationUtils.geolocation(); + + if ( + !geolocation || + fetchInstance != this.#fetchInstance || + merino != this.#merino + ) { + return null; + } + + if (geolocation.country_code) { + otherParams.country = geolocation.country_code; + } + let region = geolocation.region_code || geolocation.region; + if (region) { + otherParams.region = region; + } + let city = geolocation.city || geolocation.region; + if (city) { + otherParams.city = city; + } } - let merino = this.#merino; - let fetchInstance = (this.#fetchInstance = {}); let merinoSuggestions = await merino.fetch({ query: "", otherParams, diff --git a/browser/components/urlbar/tests/engagementTelemetry/browser/browser_glean_telemetry_engagement_selected_result.js b/browser/components/urlbar/tests/engagementTelemetry/browser/browser_glean_telemetry_engagement_selected_result.js @@ -487,9 +487,11 @@ add_task(async function selected_result_weather() { const cleanupQuickSuggest = await ensureQuickSuggestInit(); await MerinoTestUtils.initWeather(); + const cleanupGeolocation = GeolocationTestUtils.stubGeolocation(); - let provider = "UrlbarProviderQuickSuggest"; await doTest(async () => { + let provider = "UrlbarProviderQuickSuggest"; + await openPopup("weather"); await selectRowByProvider(provider); await doEnter(); @@ -504,6 +506,7 @@ add_task(async function selected_result_weather() { ]); }); + await cleanupGeolocation(); await cleanupQuickSuggest(); await SpecialPowers.popPrefEnv(); }); diff --git a/browser/components/urlbar/tests/engagementTelemetry/browser/head.js b/browser/components/urlbar/tests/engagementTelemetry/browser/head.js @@ -31,6 +31,14 @@ ChromeUtils.defineLazyGetter(this, "MerinoTestUtils", () => { return module; }); +ChromeUtils.defineLazyGetter(this, "GeolocationTestUtils", () => { + const { GeolocationTestUtils: module } = ChromeUtils.importESModule( + "resource://testing-common/GeolocationTestUtils.sys.mjs" + ); + module.init(this); + return module; +}); + ChromeUtils.defineESModuleGetters(lazy, { UrlbarTestUtils: "resource://testing-common/UrlbarTestUtils.sys.mjs", sinon: "resource://testing-common/Sinon.sys.mjs", diff --git a/browser/components/urlbar/tests/quicksuggest/GeolocationTestUtils.sys.mjs b/browser/components/urlbar/tests/quicksuggest/GeolocationTestUtils.sys.mjs @@ -0,0 +1,68 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs"; + +const lazy = XPCOMUtils.declareLazy({ + GeolocationUtils: + "moz-src:///browser/components/urlbar/private/GeolocationUtils.sys.mjs", + sinon: "resource://testing-common/Sinon.sys.mjs", +}); + +/** + * + */ +class _GeolocationTestUtils { + /** + * Initializes the utils. + * + * @param {object} scope + * The global JS scope where tests are being run. This allows the instance + * to access test helpers like `Assert` that are available in the scope. + */ + init(scope) { + if (!scope) { + throw new Error( + "GeolocationTestUtils.init() must be called with a scope" + ); + } + + this.#scope = scope; + } + + /** + * Setup stub for GeolocationUtils.geolocation() using given geolocation. + * If the geolocation parameter is null, set "San Francisco" geolocation. + * NOTE: This returns function to restore the dummy function. + * + * @param {object} [geolocation] + * @param {string} [geolocation.country_code] + * @param {string} [geolocation.city] + * @param {string} [geolocation.region_code] + * @returns {Function} function to restore the stub. + */ + stubGeolocation(geolocation) { + if (!geolocation) { + geolocation = { + country_code: "US", + city: "San Francisco", + region_code: "CA", + }; + } + + let sandbox = lazy.sinon.createSandbox(); + sandbox.stub(lazy.GeolocationUtils, "geolocation").resolves(geolocation); + + let cleanup = () => { + sandbox.restore(); + }; + + this.#scope.registerCleanupFunction?.(cleanup); + + return cleanup; + } + + #scope; +} + +export let GeolocationTestUtils = new _GeolocationTestUtils(); diff --git a/browser/components/urlbar/tests/quicksuggest/browser/browser_weather.js b/browser/components/urlbar/tests/quicksuggest/browser/browser_weather.js @@ -23,6 +23,7 @@ add_setup(async function () { ], }); await MerinoTestUtils.initWeather(); + GeolocationTestUtils.stubGeolocation(); }); // Does a search, clicks the "Show less frequently" result menu command, and diff --git a/browser/components/urlbar/tests/quicksuggest/browser/head.js b/browser/components/urlbar/tests/quicksuggest/browser/head.js @@ -36,6 +36,14 @@ ChromeUtils.defineLazyGetter(this, "MerinoTestUtils", () => { return module; }); +ChromeUtils.defineLazyGetter(this, "GeolocationTestUtils", () => { + const { GeolocationTestUtils: module } = ChromeUtils.importESModule( + "resource://testing-common/GeolocationTestUtils.sys.mjs" + ); + module.init(this); + return module; +}); + ChromeUtils.defineLazyGetter(this, "PlacesFrecencyRecalculator", () => { return Cc["@mozilla.org/places/frecency-recalculator;1"].getService( Ci.nsIObserver diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js b/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js @@ -71,6 +71,8 @@ add_task(async function disableAndEnable() { }); async function doBasicDisableAndEnableTest(pref) { + let cleanup = GeolocationTestUtils.stubGeolocation(); + // Disable the feature. It should be immediately uninitialized. UrlbarPrefs.set(pref, false); assertDisabled({ @@ -100,6 +102,8 @@ async function doBasicDisableAndEnableTest(pref) { context, matches: [QuickSuggestTestUtils.weatherResult()], }); + + await cleanup(); } // Tests a Merino fetch that doesn't return a suggestion. @@ -122,6 +126,8 @@ add_task(async function noSuggestion() { // When the Merino response doesn't include a `region_code` for the geolocated // version of the suggestion, the suggestion title should only contain a city. add_task(async function geolocationSuggestionNoRegion() { + let cleanup = GeolocationTestUtils.stubGeolocation(); + let { suggestions } = MerinoTestUtils.server.response.body; let s = { ...MerinoTestUtils.WEATHER_SUGGESTION }; delete s.region_code; @@ -146,12 +152,15 @@ add_task(async function geolocationSuggestionNoRegion() { }); MerinoTestUtils.server.response.body.suggestions = suggestions; + await cleanup(); }); // When the query matches both the weather suggestion and a previous visit to // the suggestion's URL, the suggestion should be shown and the history visit // should not be shown. add_task(async function urlAlreadyInHistory() { + let cleanup = GeolocationTestUtils.stubGeolocation(); + // A visit to the weather suggestion's exact URL. let suggestionVisit = { uri: MerinoTestUtils.WEATHER_SUGGESTION.url, @@ -199,6 +208,7 @@ add_task(async function urlAlreadyInHistory() { }); await PlacesUtils.history.clear(); + await cleanup(); }); // Locale task for when this test runs on an en-US OS. @@ -303,6 +313,8 @@ async function doLocaleTest({ shouldRunTask, osUnit, unitsByLocale }) { // enabled rather than being set according to region/locale. skipSuggestReset: true, callback: async () => { + let cleanup = GeolocationTestUtils.stubGeolocation(); + info("Checking locale: " + locale); await check_results({ context: createContext("weather", { @@ -326,6 +338,8 @@ async function doLocaleTest({ shouldRunTask, osUnit, unitsByLocale }) { ], }); Services.prefs.clearUserPref("intl.regional_prefs.use_os_locales"); + + await cleanup(); }, }); } @@ -475,6 +489,8 @@ async function doRegionTest({ homeRegion, locale, query, expectedTitleL10n }) { // Tests dismissal. add_task(async function dismissal() { + let cleanup = GeolocationTestUtils.stubGeolocation(); + await doDismissAllTest({ result: QuickSuggestTestUtils.weatherResult(), command: "dismiss", @@ -486,11 +502,14 @@ add_task(async function dismissal() { }, ], }); + + await cleanup(); }); // When a Nimbus experiment is installed, it should override the remote settings // weather record. add_task(async function nimbusOverride() { + let cleanup = GeolocationTestUtils.stubGeolocation(); let defaultResult = QuickSuggestTestUtils.weatherResult(); // Verify a search works as expected with the default remote settings weather @@ -528,6 +547,8 @@ add_task(async function nimbusOverride() { }), matches: [defaultResult], }); + + await cleanup(); }); // Tests queries that include a city without a region and where Merino does not @@ -807,6 +828,8 @@ add_task(async function cityRegionQueries() { // Tests weather queries that don't include a city. add_task(async function noCityQuery() { + let cleanup = GeolocationTestUtils.stubGeolocation(); + await doCityTest({ desc: "No city in query, so only one call to Merino should be made and Merino does the geolocation internally", query: "weather", @@ -823,6 +846,8 @@ add_task(async function noCityQuery() { }, }, }); + + await cleanup(); }); async function doCityTest({ diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_weather_geolocation.js b/browser/components/urlbar/tests/quicksuggest/unit/test_weather_geolocation.js @@ -0,0 +1,110 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +// Tests the weather suggestion with geolocation. + +"use strict"; + +add_setup(async () => { + await MerinoTestUtils.server.start(); + await QuickSuggestTestUtils.ensureQuickSuggestInit({ + remoteSettingsRecords: [QuickSuggestTestUtils.weatherRecord()], + prefs: [ + ["suggest.quicksuggest.sponsored", true], + ["weather.featureGate", true], + ], + }); +}); + +const TEST_DATA = [ + { + geolocation: { + country_code: "US", + region_code: "CA", + city: "San Francisco", + }, + expected: { + country: "US", + region: "CA", + city: "San Francisco", + }, + }, + { + geolocation: { + region_code: "CA", + city: "San Francisco", + }, + expected: { + region: "CA", + city: "San Francisco", + }, + }, + { + geolocation: { + country_code: "US", + region_code: "CA", + }, + expected: { + country: "US", + region: "CA", + }, + }, + { + geolocation: { + country_code: "US", + city: "San Francisco", + }, + expected: { + country: "US", + city: "San Francisco", + }, + }, + { + // Use region as city if no city. + geolocation: { + country_code: "JP", + region: "Kanagawa", + region_code: "14", + }, + expected: { + country: "JP", + region: "14", + city: "Kanagawa", + }, + }, +]; + +add_task(async function () { + // No need actual suggestions as we check only the http requests in Merino + // server. + MerinoTestUtils.server.response.body.suggestions = []; + + let controller = UrlbarTestUtils.newMockController(); + + for (let [index, { geolocation, expected }] of TEST_DATA.entries()) { + info(`Test for ${JSON.stringify(geolocation)}`); + let cleanup = GeolocationTestUtils.stubGeolocation(geolocation); + + await controller.startQuery( + createContext("weather", { + providers: [UrlbarProviderQuickSuggest.name], + isPrivate: false, + }) + ); + + MerinoTestUtils.server.checkAndClearRequests([ + { + params: { + [MerinoTestUtils.SEARCH_PARAMS.QUERY]: "", + [MerinoTestUtils.SEARCH_PARAMS.PROVIDERS]: "accuweather", + [MerinoTestUtils.SEARCH_PARAMS.SEQUENCE_NUMBER]: index, + source: "urlbar", + ...expected, + }, + }, + ]); + + await cleanup(); + } +}); diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_weather_keywords.js b/browser/components/urlbar/tests/quicksuggest/unit/test_weather_keywords.js @@ -391,6 +391,8 @@ async function doKeywordsTest({ info("Doing keywords test: " + desc); info(JSON.stringify({ nimbusValues, settingsData, minKeywordLength })); + let cleanup = GeolocationTestUtils.stubGeolocation(); + let nimbusCleanup; if (nimbusValues) { nimbusCleanup = await UrlbarTestUtils.initNimbusFeature(nimbusValues); @@ -433,6 +435,8 @@ async function doKeywordsTest({ ]); UrlbarPrefs.clear("weather.minKeywordLength"); + + await cleanup(); } // Tests the "Show less frequently" command when no show-less-frequently cap is @@ -561,6 +565,8 @@ async function doShowLessFrequentlyTest({ info("Doing increment test: " + desc); info(JSON.stringify({ weather, configuration, nimbusValues })); + let cleanup = GeolocationTestUtils.stubGeolocation(); + let nimbusCleanup; if (nimbusValues) { nimbusCleanup = await UrlbarTestUtils.initNimbusFeature(nimbusValues); @@ -648,4 +654,6 @@ async function doShowLessFrequentlyTest({ ]); UrlbarPrefs.clear("weather.minKeywordLength"); UrlbarPrefs.clear("weather.showLessFrequentlyCount"); + + await cleanup(); } diff --git a/browser/components/urlbar/tests/quicksuggest/unit/xpcshell.toml b/browser/components/urlbar/tests/quicksuggest/unit/xpcshell.toml @@ -76,4 +76,6 @@ requesttimeoutfactor = 2 # Slow on Linux and Windows ["test_weather.js"] +["test_weather_geolocation.js"] + ["test_weather_keywords.js"] diff --git a/browser/components/urlbar/tests/unit/head.js b/browser/components/urlbar/tests/unit/head.js @@ -57,6 +57,14 @@ ChromeUtils.defineLazyGetter(this, "UrlbarTestUtils", () => { return module; }); +ChromeUtils.defineLazyGetter(this, "GeolocationTestUtils", () => { + const { GeolocationTestUtils: module } = ChromeUtils.importESModule( + "resource://testing-common/GeolocationTestUtils.sys.mjs" + ); + module.init(this); + return module; +}); + ChromeUtils.defineLazyGetter(this, "PlacesFrecencyRecalculator", () => { return Cc["@mozilla.org/places/frecency-recalculator;1"].getService( Ci.nsIObserver