commit 5045f8d5522ef0195eae03ce28c8325d3a82ef2e parent f82602e62d85c27f320b3e3f999f961ae363b637 Author: Roger Yang <royang@mozilla.com> Date: Mon, 29 Dec 2025 14:43:06 +0000 Bug 2004048 - Support the "{acceptLanguages}" parameter replacement for search engines. r=android-reviewers,petru Differential Revision: https://phabricator.services.mozilla.com/D277197 Diffstat:
13 files changed, 133 insertions(+), 29 deletions(-)
diff --git a/mobile/android/android-components/.buildconfig.yml b/mobile/android/android-components/.buildconfig.yml @@ -716,6 +716,7 @@ projects: - components:support-base - components:support-images - components:support-ktx + - components:support-locale - components:support-remotesettings - components:support-test - components:support-utils @@ -782,6 +783,7 @@ projects: - components:support-base - components:support-images - components:support-ktx + - components:support-locale - components:support-remotesettings - components:support-test - components:support-test-libstate @@ -827,6 +829,7 @@ projects: - components:support-base - components:support-images - components:support-ktx + - components:support-locale - components:support-remotesettings - components:support-test - components:support-test-libstate @@ -943,6 +946,7 @@ projects: - components:support-base - components:support-images - components:support-ktx + - components:support-locale - components:support-remotesettings - components:support-test - components:support-test-libstate @@ -1122,6 +1126,7 @@ projects: - components:support-base - components:support-images - components:support-ktx + - components:support-locale - components:support-remotesettings - components:support-test - components:support-test-libstate @@ -1239,6 +1244,7 @@ projects: - components:support-base - components:support-images - components:support-ktx + - components:support-locale - components:support-remotesettings - components:support-test - components:support-test-fakes @@ -1612,6 +1618,7 @@ projects: - components:support-base - components:support-images - components:support-ktx + - components:support-locale - components:support-remotesettings - components:support-test - components:support-utils diff --git a/mobile/android/android-components/components/feature/search/build.gradle b/mobile/android/android-components/components/feature/search/build.gradle @@ -29,6 +29,7 @@ dependencies { implementation project(':components:service-location') implementation project(':components:support-base') implementation project(':components:support-ktx') + implementation project(':components:support-locale') implementation project(':components:support-remotesettings') implementation project(':components:support-utils') implementation project(':components:ui-colors') diff --git a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/ext/SearchEngine.kt b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/ext/SearchEngine.kt @@ -4,6 +4,7 @@ package mozilla.components.feature.search.ext +import android.content.Context import android.graphics.Bitmap import android.net.Uri import androidx.annotation.VisibleForTesting @@ -113,8 +114,8 @@ fun SearchEngine.buildSearchUrl(searchTerm: String): String { * Parses a [SearchEngine] from the given [stream]. */ @Deprecated("Only for migrating legacy search engines. Will eventually be removed.") -fun parseLegacySearchEngine(id: String, stream: InputStream): SearchEngine { - val reader = SearchEngineReader(SearchEngine.Type.CUSTOM) +fun parseLegacySearchEngine(context: Context, id: String, stream: InputStream): SearchEngine { + val reader = SearchEngineReader(context, SearchEngine.Type.CUSTOM) return reader.loadStream(id, stream) } diff --git a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/middleware/SearchMiddleware.kt b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/middleware/SearchMiddleware.kt @@ -81,7 +81,7 @@ class SearchMiddleware( searchEngineSelectorConfig?.service?.remoteSettingsService?.makeClient(SEARCH_CONFIG_ICONS_COLLECTION_NAME) private val searchEngineSelectorRepository: SearchEngineRepository? = searchEngineSelectorConfig?.let { - SearchEngineSelectorRepository(it, defaultSearchEngineIcon, client) + SearchEngineSelectorRepository(context, it, defaultSearchEngineIcon, client) } override fun invoke( diff --git a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/storage/BundledSearchEnginesStorage.kt b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/storage/BundledSearchEnginesStorage.kt @@ -238,7 +238,7 @@ private suspend fun loadSearchEnginesFromList( coroutineContext: CoroutineContext, ): List<SearchEngine> { val assets = context.assets - val reader = SearchEngineReader(type, searchExtraParams) + val reader = SearchEngineReader(context, type, searchExtraParams) val deferredSearchEngines = mutableListOf<Deferred<SearchEngine?>>() diff --git a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/storage/CustomSearchEnginesStorage.kt b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/storage/CustomSearchEnginesStorage.kt @@ -26,7 +26,7 @@ internal class CustomSearchEngineStorage( private val context: Context, private val coroutineContext: CoroutineContext = Dispatchers.IO, ) : SearchMiddleware.CustomStorage { - private val reader = SearchEngineReader(SearchEngine.Type.CUSTOM) + private val reader = SearchEngineReader(context, SearchEngine.Type.CUSTOM) private val writer = SearchEngineWriter() override suspend fun loadSearchEngineList(): List<SearchEngine> = withContext(coroutineContext) { diff --git a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/storage/SearchEngineReader.kt b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/storage/SearchEngineReader.kt @@ -4,6 +4,7 @@ package mozilla.components.feature.search.storage +import android.content.Context import android.graphics.Bitmap import android.graphics.BitmapFactory import android.net.Uri @@ -18,6 +19,7 @@ import mozilla.components.browser.icons.decoder.SvgIconDecoder import mozilla.components.browser.state.search.SearchEngine import mozilla.components.feature.search.middleware.SearchExtraParams import mozilla.components.support.images.DesiredSize +import mozilla.components.support.locale.LocaleManager import org.xmlpull.v1.XmlPullParser import org.xmlpull.v1.XmlPullParserException import org.xmlpull.v1.XmlPullParserFactory @@ -53,13 +55,18 @@ internal val GENERAL_SEARCH_ENGINE_IDS = setOf( /** * A simple XML reader for search engine plugins. * + * @param context the [Context] used to resolve the current locale dynamically. The application + * context will be stored to ensure locale changes made by the user are reflected in search URLs. * @param type the [SearchEngine.Type] that the read [SearchEngine]s will get assigned. * @param searchExtraParams Optional search extra params. */ internal class SearchEngineReader( + context: Context, private val type: SearchEngine.Type, private val searchExtraParams: SearchExtraParams? = null, ) { + private val applicationContext = context.applicationContext + private class SearchEngineBuilder( private val type: SearchEngine.Type, private val identifier: String, @@ -359,6 +366,8 @@ internal class SearchEngineReader( for (param in params) { if (param.value == "{partnerCode}") { uriBuilder.appendQueryParameter(param.name, partnerCode) + } else if (param.value == "{acceptLanguages}") { + uriBuilder.appendQueryParameter(param.name, applicationContext.getAcceptLanguage()) } else if (param.value != null) { uriBuilder.appendQueryParameter(param.name, param.value) } @@ -395,4 +404,9 @@ internal class SearchEngineReader( else -> defaultIcon } } + + private fun Context.getAcceptLanguage(): String { + return LocaleManager.getCurrentLocale(this)?.toLanguageTag() + ?: LocaleManager.getSystemDefault().toLanguageTag() + } } diff --git a/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/storage/SearchEngineSelectorRepository.kt b/mobile/android/android-components/components/feature/search/src/main/java/mozilla/components/feature/search/storage/SearchEngineSelectorRepository.kt @@ -4,6 +4,7 @@ package mozilla.components.feature.search.storage +import android.content.Context import android.graphics.Bitmap import mozilla.appservices.remotesettings.RemoteSettingsClient import mozilla.appservices.remotesettings.RemoteSettingsRecord @@ -30,10 +31,16 @@ import kotlin.coroutines.CoroutineContext /** * A repository implementation for loading and reading [SearchEngineDefinition]s from RemoteSettings. * + * @param context [Context] used by [SearchEngineReader] to resolve the current locale. * @param searchEngineSelectorConfig [SearchEngineSelectorConfig] holds configuration options for - * [SearchUserEnvironment] + * [SearchUserEnvironment]. + * @param defaultSearchEngineIcon The [Bitmap] to use as the fallback icon for any search engine + * when a specific remote icon cannot be found or loaded. + * @param client Optional [RemoteSettingsClient] for fetching search engine definitions. + * @param selector The [SearchEngineSelector] which manages and applies search configuration */ class SearchEngineSelectorRepository( + context: Context, private val searchEngineSelectorConfig: SearchEngineSelectorConfig, private val defaultSearchEngineIcon: Bitmap, client: RemoteSettingsClient?, @@ -41,7 +48,7 @@ class SearchEngineSelectorRepository( ) : SearchEngineRepository { private val searchConfigIconsUpdateService: SearchConfigIconsUpdateService = SearchConfigIconsUpdateService(client) - private val reader: SearchEngineReader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + private val reader: SearchEngineReader = SearchEngineReader(context = context, type = SearchEngine.Type.BUNDLED) private val logger = Logger("SearchEngineSelectorRepository") private val parser = SearchConfigIconsParser() diff --git a/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/storage/ParseSearchPluginsTest.kt b/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/storage/ParseSearchPluginsTest.kt @@ -7,6 +7,7 @@ package mozilla.components.feature.search.storage import android.text.TextUtils import mozilla.components.browser.state.search.SearchEngine import mozilla.components.feature.search.middleware.SearchExtraParams +import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull @@ -24,7 +25,7 @@ class ParseSearchPluginsTest(private val searchEngineIdentifier: String) { @Throws(Exception::class) fun parserNoSearchExtraParams() { val stream = FileInputStream(File(basePath, searchEngineIdentifier)) - val searchEngine = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val searchEngine = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) .loadStream(searchEngineIdentifier, stream) assertEquals(searchEngineIdentifier, searchEngine.id) @@ -58,6 +59,7 @@ class ParseSearchPluginsTest(private val searchEngineIdentifier: String) { ) val searchEngine = SearchEngineReader( + context = testContext, type = SearchEngine.Type.BUNDLED, searchExtraParams = searchExtraParams, ).loadStream( diff --git a/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/storage/SearchEngineReaderTest.kt b/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/storage/SearchEngineReaderTest.kt @@ -15,6 +15,7 @@ import mozilla.appservices.search.SearchEngineUrls import mozilla.appservices.search.SearchUrlParam import mozilla.components.browser.state.search.SearchEngine import mozilla.components.feature.search.icons.AttachmentModel +import mozilla.components.support.locale.LocaleManager import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals @@ -26,6 +27,7 @@ import org.junit.Test import org.junit.runner.RunWith import java.io.File import java.io.IOException +import java.util.Locale @RunWith(AndroidJUnit4::class) class SearchEngineReaderTest { @@ -70,7 +72,7 @@ class SearchEngineReaderTest { type = SearchEngine.Type.CUSTOM, resultUrls = listOf("https://www.example.com/search"), ) - val reader = SearchEngineReader(type = SearchEngine.Type.CUSTOM) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.CUSTOM) val invalidFile = AtomicFile(File("", "")) reader.loadFile(searchEngine.id, invalidFile) } @@ -115,7 +117,7 @@ class SearchEngineReaderTest { private fun saveAndLoadSearchEngine(searchEngine: SearchEngine): SearchEngine { val storage = CustomSearchEngineStorage(testContext) val writer = SearchEngineWriter() - val reader = SearchEngineReader(type = searchEngine.type) + val reader = SearchEngineReader(context = testContext, type = searchEngine.type) val file = storage.getSearchFile(searchEngine.id) writer.saveSearchEngineXML(searchEngine, file) @@ -125,7 +127,7 @@ class SearchEngineReaderTest { @Test fun `GIVEN {partnerCode} in value of a SearchURLParam THEN it is replaced by actual partnerCode`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() searchEngineDefinition.urls.search.params += @@ -142,7 +144,7 @@ class SearchEngineReaderTest { @Test fun `Given null value of a SearchURLParam THEN it is not appended to the URL`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() searchEngineDefinition.urls.search.params += SearchUrlParam( @@ -158,7 +160,7 @@ class SearchEngineReaderTest { @Test fun `GIVEN searchTermParamName in SearchEngineUrl THEN add a new param with name searchTermParamName and value {searchTerms}`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() searchEngineDefinition.urls.search.searchTermParamName = "test" @@ -168,7 +170,7 @@ class SearchEngineReaderTest { @Test fun `GIVEN searchTermParamName in SearchEngineUrl and {searchTerms} in base url THEN don't add a new param with value {searchTerms}`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() searchEngineDefinition.urls.search.searchTermParamName = "test" searchEngineDefinition.urls.search.base = "https://www.google.com/q={searchTerms}" @@ -179,7 +181,7 @@ class SearchEngineReaderTest { @Test fun `GIVEN search, suggest and trending URLs THEN they are correctly parsed`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() searchEngineDefinition.urls.search.base = "https://www.google.com/search" searchEngineDefinition.urls.search.params += SearchUrlParam(name = "search-test-name", value = "search-test-value", enterpriseValue = null, experimentConfig = null) @@ -228,7 +230,7 @@ class SearchEngineReaderTest { @Test fun `GIVEN null name THEN throw exception`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() searchEngineDefinition.name = "" @@ -240,7 +242,7 @@ class SearchEngineReaderTest { @Test fun `GIVEN null identifier THEN throw exception`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() searchEngineDefinition.identifier = "" @@ -252,7 +254,7 @@ class SearchEngineReaderTest { @Test fun `GIVEN valid jpeg image THEN readImageAPI decodes it`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() val searchEngine = reader.loadStreamAPI(searchEngineDefinition, emptyByteArray, validMimeType, mock()) @@ -261,7 +263,7 @@ class SearchEngineReaderTest { @Test fun `GIVEN valid png image THEN readImageAPI decodes it`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() val searchEngine = reader.loadStreamAPI(searchEngineDefinition, emptyByteArray, validMimeType, mock()) @@ -270,7 +272,7 @@ class SearchEngineReaderTest { @Test fun `GIVEN valid ico image THEN readImageAPI decodes it`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() val searchEngine = reader.loadStreamAPI(searchEngineDefinition, emptyByteArray, validMimeType, mock()) @@ -279,7 +281,7 @@ class SearchEngineReaderTest { @Test fun `GIVEN invalid image mimetype THEN readImageAPI returns defaultIcon`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() val defaultIcon = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888) @@ -289,7 +291,7 @@ class SearchEngineReaderTest { @Test fun `GIVEN invalid image location THEN readImageAPI returns defaultIcon`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() val attachmentModel = AttachmentModel( @@ -307,7 +309,7 @@ class SearchEngineReaderTest { @Test fun `GIVEN specific icons url prefix THEN readImageAPI reads from correct url`() { - val reader = SearchEngineReader(type = SearchEngine.Type.BUNDLED) + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) val searchEngineDefinition = sampleSearchEngineDefinitionData() val attachmentModel = AttachmentModel( filename = "test", @@ -323,6 +325,75 @@ class SearchEngineReaderTest { assertEquals(defaultIcon, searchEngine.icon) } + @Test + fun `GIVEN {acceptLanguages} in params THEN it is replaced by current locale`() { + val reader = SearchEngineReader(context = testContext, type = SearchEngine.Type.BUNDLED) + val searchEngineDefinition = sampleSearchEngineDefinitionData() + + searchEngineDefinition.urls.search.params += + SearchUrlParam( + name = "lang", + value = "{acceptLanguages}", + enterpriseValue = null, + experimentConfig = null, + ) + + val searchEngine = reader.loadStreamAPI(searchEngineDefinition, emptyByteArray, validMimeType, mock()) + val expected = LocaleManager.getCurrentLocale(testContext)?.toLanguageTag() + ?: Locale.getDefault().toLanguageTag() + + assertEquals("https://www.google.com/search?lang=$expected", searchEngine.resultUrls[0]) + } + + @Test + fun `GIVEN {acceptLanguages} and {partnerCode} in trending params with context THEN both are replaced`() { + val reader = SearchEngineReader( + context = testContext, + type = SearchEngine.Type.BUNDLED, + ) + val searchEngineDefinition = sampleSearchEngineDefinitionData() + + // Override partner code so we can assert it explicitly + searchEngineDefinition.partnerCode = "test-firefox-code" + + searchEngineDefinition.urls.trending = SearchEngineUrl( + base = "https://www.google.com/trending/search", + method = "GET", + params = listOf( + SearchUrlParam( + name = "lang", + value = "{acceptLanguages}", + enterpriseValue = null, + experimentConfig = null, + ), + SearchUrlParam( + name = "client", + value = "{partnerCode}", + enterpriseValue = null, + experimentConfig = null, + ), + ), + searchTermParamName = null, + displayName = null, + ) + + val searchEngine = reader.loadStreamAPI( + searchEngineDefinition, + emptyByteArray, + validMimeType, + mock(), + ) + + val expectedLanguage = + LocaleManager.getCurrentLocale(testContext)?.toLanguageTag() + ?: Locale.getDefault().toLanguageTag() + + assertEquals( + "https://www.google.com/trending/search?lang=$expectedLanguage&client=test-firefox-code", + searchEngine.trendingUrl, + ) + } + private fun sampleSearchEngineDefinitionData(): SearchEngineDefinition { val engineDefinition = SearchEngineDefinition( aliases = listOf("google"), diff --git a/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/storage/SearchEngineSelectorRepositoryTest.kt b/mobile/android/android-components/components/feature/search/src/test/java/mozilla/components/feature/search/storage/SearchEngineSelectorRepositoryTest.kt @@ -61,6 +61,7 @@ class SearchEngineSelectorRepositoryTest { // Instantiate the repository with the mocked config repository = SearchEngineSelectorRepository( + context = mock(), searchEngineSelectorConfig = mockConfig, defaultSearchEngineIcon = mock(), selector = mockSelector, diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/search/SearchMigration.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/search/SearchMigration.kt @@ -50,15 +50,15 @@ internal class SearchMigration( return ids.mapNotNull { id -> val xml = preferences.getString(id, null) - loadSafely(id, xml?.byteInputStream()?.buffered()) + loadSafely(context, id, xml?.byteInputStream()?.buffered()) } } } @Suppress("DEPRECATION") -private fun loadSafely(id: String, stream: BufferedInputStream?): SearchEngine? { +private fun loadSafely(context: Context, id: String, stream: BufferedInputStream?): SearchEngine? { return try { - stream?.let { parseLegacySearchEngine(id, it) } + stream?.let { parseLegacySearchEngine(context, id, it) } } catch (e: IOException) { null } catch (e: XmlPullParserException) { diff --git a/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/search/SearchMigration.kt b/mobile/android/focus-android/app/src/main/java/org/mozilla/focus/search/SearchMigration.kt @@ -55,15 +55,15 @@ class SearchMigration( .byteInputStream() .buffered() - loadSafely(engine, engineInputStream) + loadSafely(context, engine, engineInputStream) } } } @Suppress("DEPRECATION") -private fun loadSafely(id: String, stream: BufferedInputStream?): SearchEngine? { +private fun loadSafely(context: Context, id: String, stream: BufferedInputStream?): SearchEngine? { return try { - stream?.let { parseLegacySearchEngine(id, it) } + stream?.let { parseLegacySearchEngine(context, id, it) } } catch (e: IOException) { null } catch (e: XmlPullParserException) {