commit 482c1522a520cf392c53bacd847a52225b46fd6d
parent b1ef8e49dd8db810cf5aef0f17fb8dee7e6587b9
Author: Daisuke Akatsuka <daisuke@birchill.co.jp>
Date: Sat, 1 Nov 2025 01:42:23 +0000
Bug 1995894: Use geolocation in GeolocationUtils when detecting the local location r=home-newtab-reviewers,adw,reemhamz
Differential Revision: https://phabricator.services.mozilla.com/D270443
Diffstat:
8 files changed, 386 insertions(+), 102 deletions(-)
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,7 +487,9 @@ add_task(async function selected_result_weather() {
const cleanupQuickSuggest = await ensureQuickSuggestInit();
await MerinoTestUtils.initWeather();
- const cleanupGeolocation = GeolocationTestUtils.stubGeolocation();
+ const cleanupGeolocation = GeolocationTestUtils.stubGeolocation(
+ GeolocationTestUtils.SAN_FRANCISCO
+ );
await doTest(async () => {
let provider = "UrlbarProviderQuickSuggest";
diff --git a/browser/components/urlbar/tests/quicksuggest/GeolocationTestUtils.sys.mjs b/browser/components/urlbar/tests/quicksuggest/GeolocationTestUtils.sys.mjs
@@ -1,18 +1,37 @@
/* 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 = {};
-const lazy = XPCOMUtils.declareLazy({
- GeolocationUtils:
- "moz-src:///browser/components/urlbar/private/GeolocationUtils.sys.mjs",
+ChromeUtils.defineESModuleGetters(lazy, {
sinon: "resource://testing-common/Sinon.sys.mjs",
});
+ChromeUtils.defineLazyGetter(lazy, "GeolocationUtils", () => {
+ try {
+ return ChromeUtils.importESModule(
+ "moz-src:///browser/components/urlbar/private/GeolocationUtils.sys.mjs"
+ ).GeolocationUtils;
+ } catch {
+ // Fallback to URI format prior to FF 144.
+ return ChromeUtils.importESModule(
+ "resource:///modules/urlbar/private/GeolocationUtils.sys.mjs"
+ ).GeolocationUtils;
+ }
+});
+
/**
*
*/
class _GeolocationTestUtils {
+ get SAN_FRANCISCO() {
+ return {
+ country_code: "US",
+ city: "San Francisco",
+ region_code: "CA",
+ };
+ }
+
/**
* Initializes the utils.
*
@@ -32,24 +51,14 @@ class _GeolocationTestUtils {
/**
* 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 {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);
diff --git a/browser/components/urlbar/tests/quicksuggest/browser/browser_weather.js b/browser/components/urlbar/tests/quicksuggest/browser/browser_weather.js
@@ -23,7 +23,7 @@ add_setup(async function () {
],
});
await MerinoTestUtils.initWeather();
- GeolocationTestUtils.stubGeolocation();
+ GeolocationTestUtils.stubGeolocation(GeolocationTestUtils.SAN_FRANCISCO);
});
// Does a search, clicks the "Show less frequently" result menu command, and
diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js b/browser/components/urlbar/tests/quicksuggest/unit/test_weather.js
@@ -71,7 +71,9 @@ add_task(async function disableAndEnable() {
});
async function doBasicDisableAndEnableTest(pref) {
- let cleanup = GeolocationTestUtils.stubGeolocation();
+ let cleanup = GeolocationTestUtils.stubGeolocation(
+ GeolocationTestUtils.SAN_FRANCISCO
+ );
// Disable the feature. It should be immediately uninitialized.
UrlbarPrefs.set(pref, false);
@@ -126,7 +128,9 @@ 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 cleanup = GeolocationTestUtils.stubGeolocation(
+ GeolocationTestUtils.SAN_FRANCISCO
+ );
let { suggestions } = MerinoTestUtils.server.response.body;
let s = { ...MerinoTestUtils.WEATHER_SUGGESTION };
@@ -159,7 +163,9 @@ add_task(async function geolocationSuggestionNoRegion() {
// 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();
+ let cleanup = GeolocationTestUtils.stubGeolocation(
+ GeolocationTestUtils.SAN_FRANCISCO
+ );
// A visit to the weather suggestion's exact URL.
let suggestionVisit = {
@@ -313,7 +319,9 @@ async function doLocaleTest({ shouldRunTask, osUnit, unitsByLocale }) {
// enabled rather than being set according to region/locale.
skipSuggestReset: true,
callback: async () => {
- let cleanup = GeolocationTestUtils.stubGeolocation();
+ let cleanup = GeolocationTestUtils.stubGeolocation(
+ GeolocationTestUtils.SAN_FRANCISCO
+ );
info("Checking locale: " + locale);
await check_results({
@@ -489,7 +497,9 @@ async function doRegionTest({ homeRegion, locale, query, expectedTitleL10n }) {
// Tests dismissal.
add_task(async function dismissal() {
- let cleanup = GeolocationTestUtils.stubGeolocation();
+ let cleanup = GeolocationTestUtils.stubGeolocation(
+ GeolocationTestUtils.SAN_FRANCISCO
+ );
await doDismissAllTest({
result: QuickSuggestTestUtils.weatherResult(),
@@ -509,7 +519,9 @@ add_task(async function dismissal() {
// When a Nimbus experiment is installed, it should override the remote settings
// weather record.
add_task(async function nimbusOverride() {
- let cleanup = GeolocationTestUtils.stubGeolocation();
+ let cleanup = GeolocationTestUtils.stubGeolocation(
+ GeolocationTestUtils.SAN_FRANCISCO
+ );
let defaultResult = QuickSuggestTestUtils.weatherResult();
// Verify a search works as expected with the default remote settings weather
@@ -828,7 +840,9 @@ add_task(async function cityRegionQueries() {
// Tests weather queries that don't include a city.
add_task(async function noCityQuery() {
- let cleanup = GeolocationTestUtils.stubGeolocation();
+ let cleanup = GeolocationTestUtils.stubGeolocation(
+ GeolocationTestUtils.SAN_FRANCISCO
+ );
await doCityTest({
desc: "No city in query, so only one call to Merino should be made and Merino does the geolocation internally",
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,7 +391,9 @@ async function doKeywordsTest({
info("Doing keywords test: " + desc);
info(JSON.stringify({ nimbusValues, settingsData, minKeywordLength }));
- let cleanup = GeolocationTestUtils.stubGeolocation();
+ let cleanup = GeolocationTestUtils.stubGeolocation(
+ GeolocationTestUtils.SAN_FRANCISCO
+ );
let nimbusCleanup;
if (nimbusValues) {
@@ -565,7 +567,9 @@ async function doShowLessFrequentlyTest({
info("Doing increment test: " + desc);
info(JSON.stringify({ weather, configuration, nimbusValues }));
- let cleanup = GeolocationTestUtils.stubGeolocation();
+ let cleanup = GeolocationTestUtils.stubGeolocation(
+ GeolocationTestUtils.SAN_FRANCISCO
+ );
let nimbusCleanup;
if (nimbusValues) {
diff --git a/browser/extensions/newtab/lib/WeatherFeed.sys.mjs b/browser/extensions/newtab/lib/WeatherFeed.sys.mjs
@@ -25,6 +25,19 @@ ChromeUtils.defineLazyGetter(lazy, "MerinoClient", () => {
}
});
+ChromeUtils.defineLazyGetter(lazy, "GeolocationUtils", () => {
+ try {
+ return ChromeUtils.importESModule(
+ "moz-src:///browser/components/urlbar/private/GeolocationUtils.sys.mjs"
+ ).GeolocationUtils;
+ } catch {
+ // Fallback to URI format prior to FF 144.
+ return ChromeUtils.importESModule(
+ "resource:///modules/urlbar/private/GeolocationUtils.sys.mjs"
+ ).GeolocationUtils;
+ }
+});
+
import {
actionTypes as at,
actionCreators as ac,
@@ -93,37 +106,6 @@ export class WeatherFeed {
this.fetchTimer = 0;
}
- /**
- * This thin wrapper around the fetch call makes it easier for us to write
- * automated tests that simulate responses.
- */
- async fetchHelper(retries = 3, queryOverride = null) {
- this.restartFetchTimer();
- const weatherQuery = this.store.getState().Prefs.values[PREF_WEATHER_QUERY];
- let suggestions = [];
- let retry = 0;
- const query = queryOverride ?? weatherQuery ?? "";
- while (retry++ < retries && suggestions.length === 0) {
- try {
- suggestions = await this.merino.fetch({
- query,
- providers: MERINO_PROVIDER,
- timeoutMs: 7000,
- otherParams: {
- request_type: "weather",
- source: "newtab",
- },
- });
- } catch (error) {
- // We don't need to do anything with this right now.
- }
- }
-
- // results from the API or empty array if null
- this.suggestions = suggestions ?? [];
- return this.suggestions;
- }
-
async fetch() {
// Keep a handle on the `MerinoClient` instance that exists at the start of
// this fetch. If fetching stops or this `Weather` instance is uninitialized
@@ -133,7 +115,7 @@ export class WeatherFeed {
this.merino = await this.MerinoClient(MERINO_CLIENT_KEY);
}
- await this.fetchHelper();
+ this.suggestions = await this._fetchHelper();
if (this.suggestions.length) {
const hasLocationData =
@@ -227,41 +209,6 @@ export class WeatherFeed {
}
}
- async fetchLocationByIP() {
- if (!this.merino) {
- this.merino = await this.MerinoClient(MERINO_CLIENT_KEY);
- }
-
- // First we fetch the forecast through user's IP Address
- // which is done by not adding in a query parameter, but keeping the "weather" request_type.
- // This method is mentioned in the AccuWeather docs:
- // https://apidev.accuweather.com/developers/locationsAPIguide#IPAddress
- try {
- const ipLocation = await this.fetchHelper(3, "");
-
- const ipData = ipLocation?.[0];
-
- // Second, we use the city name that came from the IP look up to get the normalized merino response
- // For context, the IP lookup response does not have the complete response data we need
- const locationData = await this.merino.fetch({
- query: ipData.city_name,
- providers: MERINO_PROVIDER,
- timeoutMs: 7000,
- otherParams: {
- request_type: "location",
- source: "newtab",
- },
- });
-
- const response = locationData?.[0]?.locations?.[0];
- return response;
- // return response
- } catch (err) {
- console.error("WeatherFeed failed to look up IP");
- return null;
- }
- }
-
async onPrefChangedAction(action) {
switch (action.data.name) {
case PREF_WEATHER_QUERY:
@@ -334,11 +281,10 @@ export class WeatherFeed {
this.store.dispatch(ac.SetPref("weather.optInAccepted", true));
this.store.dispatch(ac.SetPref("weather.optInDisplayed", false));
- const detectedLocation = await this.fetchLocationByIP();
+ const detectedLocation = await this._fetchNormalizedLocation();
if (detectedLocation) {
// Build the payload exactly like manual search does
-
this.store.dispatch(
ac.BroadcastToContent({
type: at.WEATHER_LOCATION_DATA_UPDATE,
@@ -361,6 +307,95 @@ export class WeatherFeed {
}
}
}
+
+ /**
+ * This thin wrapper around the fetch call makes it easier for us to write
+ * automated tests that simulate responses.
+ */
+ async _fetchHelper(retries = 3, queryOverride = null) {
+ this.restartFetchTimer();
+
+ const weatherQuery = this.store.getState().Prefs.values[PREF_WEATHER_QUERY];
+ const query = queryOverride ?? weatherQuery ?? "";
+ const otherParams = {
+ request_type: "weather",
+ source: "newtab",
+ };
+
+ if (!query) {
+ let geolocation = await lazy.GeolocationUtils.geolocation();
+ if (!geolocation) {
+ return [];
+ }
+
+ 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 suggestions;
+ let retry = 0;
+
+ while (retry++ < retries && !suggestions?.length) {
+ try {
+ suggestions = await this.merino.fetch({
+ query,
+ providers: MERINO_PROVIDER,
+ timeoutMs: 7000,
+ otherParams,
+ });
+ } catch (error) {
+ // We don't need to do anything with this right now.
+ }
+ }
+
+ // results from the API or empty array if null
+ return suggestions ?? [];
+ }
+
+ async _fetchNormalizedLocation() {
+ const geolocation = await lazy.GeolocationUtils.geolocation();
+ if (!geolocation) {
+ return null;
+ }
+
+ // "region" might be able to be city if geolocation.city is null
+ const city = geolocation.city || geolocation.region;
+ if (!city) {
+ return null;
+ }
+
+ if (!this.merino) {
+ this.merino = await this.MerinoClient(MERINO_CLIENT_KEY);
+ }
+
+ try {
+ // We use the given city name look up to get the normalized merino response
+ const locationData = await this.merino.fetch({
+ query: city,
+ providers: MERINO_PROVIDER,
+ timeoutMs: 7000,
+ otherParams: {
+ request_type: "location",
+ source: "newtab",
+ },
+ });
+
+ const response = locationData?.[0]?.locations?.[0];
+ return response;
+ } catch (err) {
+ console.error("WeatherFeed failed to get normalized location");
+ return null;
+ }
+ }
}
/**
@@ -368,7 +403,10 @@ export class WeatherFeed {
* This makes it easier for us to write automated tests that simulate responses.
*/
WeatherFeed.prototype.MerinoClient = (...args) => {
- return new lazy.MerinoClient(...args);
+ return new lazy.MerinoClient({
+ allowOhttp: true,
+ ...args,
+ });
};
WeatherFeed.prototype.PersistentCache = (...args) => {
return new lazy.PersistentCache(...args);
diff --git a/browser/extensions/newtab/test/browser/browser_customize_menu_content.js b/browser/extensions/newtab/test/browser/browser_customize_menu_content.js
@@ -9,11 +9,18 @@ const { WeatherFeed } = ChromeUtils.importESModule(
);
ChromeUtils.defineESModuleGetters(this, {
+ GeolocationTestUtils:
+ "resource://testing-common/GeolocationTestUtils.sys.mjs",
MerinoTestUtils: "resource://testing-common/MerinoTestUtils.sys.mjs",
});
const { WEATHER_SUGGESTION } = MerinoTestUtils;
+add_setup(async function () {
+ GeolocationTestUtils.init(this);
+ GeolocationTestUtils.stubGeolocation(GeolocationTestUtils.SAN_FRANCISCO);
+});
+
test_newtab({
async before({ pushPrefs }) {
await pushPrefs(
diff --git a/browser/extensions/newtab/test/xpcshell/test_WeatherFeed.js b/browser/extensions/newtab/test/xpcshell/test_WeatherFeed.js
@@ -7,12 +7,15 @@ ChromeUtils.defineESModuleGetters(this, {
actionCreators: "resource://newtab/common/Actions.mjs",
actionTypes: "resource://newtab/common/Actions.mjs",
sinon: "resource://testing-common/Sinon.sys.mjs",
+ GeolocationTestUtils:
+ "resource://testing-common/GeolocationTestUtils.sys.mjs",
MerinoTestUtils: "resource://testing-common/MerinoTestUtils.sys.mjs",
WeatherFeed: "resource://newtab/lib/WeatherFeed.sys.mjs",
Region: "resource://gre/modules/Region.sys.mjs",
});
const { WEATHER_SUGGESTION } = MerinoTestUtils;
+GeolocationTestUtils.init(this);
const WEATHER_ENABLED = "browser.newtabpage.activity-stream.showWeather";
const SYS_WEATHER_ENABLED =
@@ -130,8 +133,7 @@ add_task(async function test_onAction_INIT() {
sandbox.stub(feed, "isEnabled").returns(true);
- sandbox.stub(feed, "fetchHelper");
- feed.suggestions = [WEATHER_SUGGESTION];
+ sandbox.stub(feed, "_fetchHelper").resolves([WEATHER_SUGGESTION]);
feed.locationData = locationData;
feed.store = {
dispatch: sinon.spy(),
@@ -188,8 +190,8 @@ add_task(async function test_onAction_opt_in_location_success() {
},
};
- // Stub fetchLocationByIP() to simulate a successful lookup
- sandbox.stub(feed, "fetchLocationByIP").resolves({
+ // Stub _fetchNormalizedLocation() to simulate a successful lookup
+ sandbox.stub(feed, "_fetchNormalizedLocation").resolves({
localized_name: "Testville",
administrative_area: "Paris",
country: "FR",
@@ -252,8 +254,8 @@ add_task(async function test_onAction_opt_in_no_location_found() {
},
};
- // Test that fetchLocationByIP doesn't return a location
- sandbox.stub(feed, "fetchLocationByIP").resolves(null);
+ // Test that _fetchNormalizedLocation doesn't return a location
+ sandbox.stub(feed, "_fetchNormalizedLocation").resolves(null);
await feed.onAction({ type: actionTypes.WEATHER_USER_OPT_IN_LOCATION });
@@ -287,3 +289,211 @@ add_task(async function test_onAction_opt_in_no_location_found() {
sandbox.restore();
});
+
+// Test fetching weather information using GeolocationUtils.geolocation()
+add_task(async function test_fetch_weather_with_geolocation() {
+ const TEST_DATA = [
+ {
+ geolocation: {
+ country_code: "US",
+ region_code: "CA",
+ region: "Califolnia",
+ city: "San Francisco",
+ },
+ expected: {
+ country: "US",
+ region: "CA",
+ city: "San Francisco",
+ },
+ },
+ {
+ geolocation: {
+ country_code: "JP",
+ region_code: "14",
+ region: "Kanagawa",
+ city: "",
+ },
+ expected: {
+ country: "JP",
+ region: "14",
+ city: "Kanagawa",
+ },
+ },
+ {
+ geolocation: {
+ country_code: "TestCountry",
+ region_code: "",
+ region: "TestRegion",
+ city: "TestCity",
+ },
+ expected: {
+ country: "TestCountry",
+ region: "TestRegion",
+ city: "TestCity",
+ },
+ },
+ {
+ geolocation: {
+ country_code: "TestCountry",
+ },
+ expected: {
+ country: "TestCountry",
+ },
+ },
+ {
+ geolocation: {
+ region_code: "TestRegionCode",
+ },
+ expected: {
+ region: "TestRegionCode",
+ },
+ },
+ {
+ geolocation: {
+ region: "TestRegion",
+ },
+ expected: {
+ region: "TestRegion",
+ city: "TestRegion",
+ },
+ },
+ {
+ geolocation: {
+ city: "TestCity",
+ },
+ expected: {
+ city: "TestCity",
+ },
+ },
+ {
+ geolocation: {},
+ expected: {},
+ },
+ {
+ geolocation: null,
+ expected: false,
+ },
+ ];
+
+ for (let { geolocation, expected } of TEST_DATA) {
+ info(`Test for ${JSON.stringify(geolocation)}`);
+
+ let sandbox = sinon.createSandbox();
+ sandbox.stub(WeatherFeed.prototype, "PersistentCache").returns({
+ set: () => {},
+ get: () => {},
+ });
+
+ let feed = new WeatherFeed();
+ sandbox.stub(feed, "isEnabled").returns(true);
+ feed.store = {
+ dispatch: sinon.spy(),
+ getState() {
+ return { Prefs: { values: {} } };
+ },
+ };
+ feed.merino = { fetch: () => {} };
+
+ // Stub merino client
+ let stub = sandbox.stub(feed.merino, "fetch").resolves(["result"]);
+ let cleanupGeolocationStub =
+ GeolocationTestUtils.stubGeolocation(geolocation);
+
+ await feed.onAction({ type: actionTypes.SYSTEM_TICK });
+
+ if (expected) {
+ sinon.assert.calledOnce(stub);
+ sinon.assert.calledWith(stub, {
+ otherParams: { request_type: "weather", source: "newtab", ...expected },
+ providers: ["accuweather"],
+ query: "",
+ timeoutMs: 7000,
+ });
+ } else {
+ sinon.assert.notCalled(stub);
+ }
+
+ await cleanupGeolocationStub();
+ sandbox.restore();
+ }
+});
+
+// Test detecting location using GeolocationUtils.geolocation()
+add_task(async function test_detect_location_with_geolocation() {
+ const TEST_DATA = [
+ {
+ geolocation: {
+ city: "San Francisco",
+ },
+ expected: "San Francisco",
+ },
+ {
+ geolocation: {
+ city: "",
+ region: "Yokohama",
+ },
+ expected: "Yokohama",
+ },
+ {
+ geolocation: {
+ region: "Tokyo",
+ },
+ expected: "Tokyo",
+ },
+ {
+ geolocation: {
+ city: "",
+ region: "",
+ },
+ expected: false,
+ },
+ {
+ geolocation: {},
+ expected: false,
+ },
+ {
+ geolocation: null,
+ expected: false,
+ },
+ ];
+ for (let { geolocation, expected } of TEST_DATA) {
+ info(`Test for ${JSON.stringify(geolocation)}`);
+
+ let sandbox = sinon.createSandbox();
+ sandbox.stub(WeatherFeed.prototype, "PersistentCache").returns({
+ set: () => {},
+ get: () => {},
+ });
+
+ let feed = new WeatherFeed();
+ feed.store = {
+ dispatch: sinon.spy(),
+ getState() {
+ return { Prefs: { values: {} } };
+ },
+ };
+ feed.merino = { fetch: () => {} };
+
+ // Stub merino client
+ let stub = sandbox.stub(feed.merino, "fetch").resolves(null);
+ // Stub geolocation
+ let cleanupGeolocationStub =
+ GeolocationTestUtils.stubGeolocation(geolocation);
+ await feed.onAction({ type: actionTypes.WEATHER_USER_OPT_IN_LOCATION });
+
+ if (expected) {
+ sinon.assert.calledOnce(stub);
+ sinon.assert.calledWith(stub, {
+ otherParams: { request_type: "location", source: "newtab" },
+ providers: ["accuweather"],
+ query: expected,
+ timeoutMs: 7000,
+ });
+ } else {
+ sinon.assert.notCalled(stub);
+ }
+
+ await cleanupGeolocationStub();
+ sandbox.restore();
+ }
+});
