From 2b759e9d6f326bf20faa4ca6091a2558b90b47f6 Mon Sep 17 00:00:00 2001 From: Sebastian Kaspari Date: Mon, 9 Nov 2020 12:16:48 +0100 Subject: [PATCH] Integrate new search code from Android Components into Fenix. --- .../fenix/ui/robots/HomeScreenRobot.kt | 6 +- .../java/org/mozilla/fenix/HomeActivity.kt | 26 +- .../org/mozilla/fenix/components/Analytics.kt | 3 +- .../mozilla/fenix/components/Components.kt | 2 - .../java/org/mozilla/fenix/components/Core.kt | 22 +- .../org/mozilla/fenix/components/Search.kt | 37 --- .../org/mozilla/fenix/components/UseCases.kt | 6 +- .../mozilla/fenix/components/metrics/Event.kt | 4 +- .../components/metrics/GleanMetricsService.kt | 26 +- .../fenix/components/metrics/MetricsUtils.kt | 13 +- .../components/search/SearchMigration.kt | 70 ++++ .../searchengine/CustomSearchEngineStore.kt | 129 -------- .../searchengine/FenixSearchEngineProvider.kt | 306 ------------------ .../searchengine/SearchEngineWriter.kt | 84 ----- .../java/org/mozilla/fenix/ext/Context.kt | 7 - .../org/mozilla/fenix/home/HomeFragment.kt | 62 ++-- .../intent/SpeechProcessingIntentProcessor.kt | 53 ++- .../SessionControlController.kt | 13 +- .../fenix/search/SearchDialogController.kt | 67 ++-- .../fenix/search/SearchDialogFragment.kt | 44 +-- .../fenix/search/SearchDialogInteractor.kt | 2 +- .../fenix/search/SearchFragmentStore.kt | 68 ++-- .../search/awesomebar/AwesomeBarInteractor.kt | 2 +- .../fenix/search/awesomebar/AwesomeBarView.kt | 35 +- .../awesomebar/ShortcutsSuggestionProvider.kt | 11 +- .../fenix/search/ext/SearchEngineProvider.kt | 17 - .../fenix/search/toolbar/ToolbarView.kt | 24 +- .../search/AddSearchEngineFragment.kt | 108 +++---- .../search/EditCustomSearchEngineFragment.kt | 73 ++--- .../search/RadioSearchEngineListPreference.kt | 156 ++++++++- .../settings/search/SearchEngineFragment.kt | 4 - .../search/SearchEngineListPreference.kt | 242 -------------- app/src/main/res/navigation/nav_graph.xml | 3 - .../fenix/MigratingFenixApplication.kt | 1 - .../fenix/components/TestComponents.kt | 2 - .../metrics/GleanMetricsServiceTest.kt | 5 +- .../metrics/MetricsUtilsTestRoboelectric.kt | 16 +- .../components/metrics/PerformedSearchTest.kt | 207 ------------ .../FenixSearchEngineProviderTest.kt | 174 ---------- .../DefaultSessionControlControllerTest.kt | 28 +- .../SpeechProcessingIntentProcessorTest.kt | 59 ++-- .../search/SearchDialogControllerTest.kt | 10 +- .../search/SearchDialogInteractorTest.kt | 2 +- .../fenix/search/SearchFragmentStoreTest.kt | 167 +++++++--- .../ShortcutsSuggestionProviderTest.kt | 38 +-- .../fenix/search/toolbar/ToolbarViewTest.kt | 3 +- .../perf/MozillaUseLazyMonitored.kt | 1 - 47 files changed, 801 insertions(+), 1637 deletions(-) delete mode 100644 app/src/main/java/org/mozilla/fenix/components/Search.kt create mode 100644 app/src/main/java/org/mozilla/fenix/components/search/SearchMigration.kt delete mode 100644 app/src/main/java/org/mozilla/fenix/components/searchengine/CustomSearchEngineStore.kt delete mode 100644 app/src/main/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProvider.kt delete mode 100644 app/src/main/java/org/mozilla/fenix/components/searchengine/SearchEngineWriter.kt delete mode 100644 app/src/main/java/org/mozilla/fenix/search/ext/SearchEngineProvider.kt delete mode 100644 app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineListPreference.kt delete mode 100644 app/src/test/java/org/mozilla/fenix/components/metrics/PerformedSearchTest.kt delete mode 100644 app/src/test/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProviderTest.kt diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/HomeScreenRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/HomeScreenRobot.kt index a68b19f32..a4234354c 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/HomeScreenRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/HomeScreenRobot.kt @@ -38,6 +38,7 @@ import androidx.test.uiautomator.UiSelector import androidx.test.uiautomator.Until import androidx.test.uiautomator.Until.findObject import mozilla.components.support.ktx.android.content.appName +import mozilla.components.browser.state.state.searchEngines import org.hamcrest.CoreMatchers.allOf import org.hamcrest.CoreMatchers.containsString import org.hamcrest.CoreMatchers.instanceOf @@ -45,7 +46,7 @@ import org.hamcrest.CoreMatchers.not import org.hamcrest.Matchers import org.junit.Assert import org.mozilla.fenix.R -import org.mozilla.fenix.components.Search +import org.mozilla.fenix.ext.components import org.mozilla.fenix.helpers.TestAssetHelper import org.mozilla.fenix.helpers.TestAssetHelper.waitingTime import org.mozilla.fenix.helpers.TestHelper.scrollToElementByText @@ -579,10 +580,11 @@ private fun verifySearchEngineIcon(searchEngineIcon: Bitmap, searchEngineName: S } private fun getSearchEngine(searchEngineName: String) = - Search(appContext).searchEngineManager.getDefaultSearchEngine(appContext, searchEngineName) + appContext.components.core.store.state.search.searchEngines.find { it.name == searchEngineName } private fun verifySearchEngineIcon(searchEngineName: String) { val ddgSearchEngine = getSearchEngine(searchEngineName) + ?: throw AssertionError("No search engine with name $searchEngineName") verifySearchEngineIcon(ddgSearchEngine.icon, ddgSearchEngine.name) } diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 67f662879..a8bfd89d9 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -37,7 +37,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.launch -import mozilla.components.browser.search.SearchEngine +import mozilla.components.browser.state.search.SearchEngine import mozilla.components.browser.state.selector.getNormalOrPrivateTabs import mozilla.components.browser.state.state.SessionState import mozilla.components.browser.state.state.WebExtensionState @@ -46,6 +46,7 @@ import mozilla.components.concept.engine.EngineView import mozilla.components.feature.contextmenu.DefaultSelectionActionDelegate import mozilla.components.feature.privatemode.notification.PrivateNotificationFeature import mozilla.components.feature.search.BrowserStoreSearchAdapter +import mozilla.components.feature.search.ext.legacy import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.components.support.ktx.android.arch.lifecycle.addObservers @@ -139,7 +140,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { private val externalSourceIntentProcessors by lazy { listOf( - SpeechProcessingIntentProcessor(this, components.analytics.metrics), + SpeechProcessingIntentProcessor(this, components.core.store, components.analytics.metrics), StartSearchIntentProcessor(components.analytics.metrics), DeepLinkIntentProcessor(this, components.analytics.leanplumMetricsService), OpenBrowserIntentProcessor(this, ::getIntentSessionId), @@ -737,23 +738,24 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { } } else components.useCases.sessionUseCases.loadUrl - val searchUseCase: (String) -> Unit = { searchTerms -> + // In situations where we want to perform a search but have no search engine (e.g. the user + // has removed all of them, or we couldn't load any) we will pass searchTermOrURL to Gecko + // and let it try to load whatever was entered. + if ((!forceSearch && searchTermOrURL.isUrl()) || engine == null) { + loadUrlUseCase.invoke(searchTermOrURL.toNormalizedUrl(), flags) + } else { if (newTab) { components.useCases.searchUseCases.newTabSearch .invoke( - searchTerms, + searchTermOrURL, SessionState.Source.USER_ENTERED, true, mode.isPrivate, - searchEngine = engine + searchEngine = engine.legacy() ) - } else components.useCases.searchUseCases.defaultSearch.invoke(searchTerms, engine) - } - - if (!forceSearch && searchTermOrURL.isUrl()) { - loadUrlUseCase.invoke(searchTermOrURL.toNormalizedUrl(), flags) - } else { - searchUseCase.invoke(searchTermOrURL) + } else { + components.useCases.searchUseCases.defaultSearch.invoke(searchTermOrURL, engine.legacy()) + } } if (components.core.engine.profiler?.isProfilerActive() == true) { diff --git a/app/src/main/java/org/mozilla/fenix/components/Analytics.kt b/app/src/main/java/org/mozilla/fenix/components/Analytics.kt index fc4a8e6fb..6510777aa 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Analytics.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Analytics.kt @@ -24,6 +24,7 @@ import org.mozilla.fenix.components.metrics.AdjustMetricsService import org.mozilla.fenix.components.metrics.GleanMetricsService import org.mozilla.fenix.components.metrics.LeanplumMetricsService import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings import org.mozilla.fenix.perf.lazyMonitored import org.mozilla.fenix.utils.Mockable @@ -92,7 +93,7 @@ class Analytics( val metrics: MetricController by lazyMonitored { MetricController.create( listOf( - GleanMetricsService(context), + GleanMetricsService(context, lazy { context.components.core.store }), leanplumMetricsService, AdjustMetricsService(context as Application) ), diff --git a/app/src/main/java/org/mozilla/fenix/components/Components.kt b/app/src/main/java/org/mozilla/fenix/components/Components.kt index 38ee5d26c..55f7b8035 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -54,14 +54,12 @@ class Components(private val context: Context) { } val services by lazyMonitored { Services(context, backgroundServices.accountManager) } val core by lazyMonitored { Core(context, analytics.crashReporter, strictMode) } - val search by lazyMonitored { Search(context) } val useCases by lazyMonitored { UseCases( context, core.engine, core.sessionManager, core.store, - search.searchEngineManager, core.webAppShortcutManager, core.topSitesStorage ) diff --git a/app/src/main/java/org/mozilla/fenix/components/Core.kt b/app/src/main/java/org/mozilla/fenix/components/Core.kt index 7e7aee6ae..644319e3a 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Core.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Core.kt @@ -46,6 +46,8 @@ import mozilla.components.feature.pwa.ManifestStorage import mozilla.components.feature.pwa.WebAppShortcutManager import mozilla.components.feature.readerview.ReaderViewMiddleware import mozilla.components.feature.recentlyclosed.RecentlyClosedMiddleware +import mozilla.components.feature.search.middleware.SearchMiddleware +import mozilla.components.feature.search.region.RegionMiddleware import mozilla.components.feature.session.HistoryDelegate import mozilla.components.feature.top.sites.DefaultTopSitesStorage import mozilla.components.feature.top.sites.PinnedSiteStorage @@ -57,14 +59,18 @@ import mozilla.components.lib.dataprotect.generateEncryptionKey import mozilla.components.service.digitalassetlinks.RelationChecker import mozilla.components.service.digitalassetlinks.local.StatementApi import mozilla.components.service.digitalassetlinks.local.StatementRelationChecker +import mozilla.components.service.location.LocationService +import mozilla.components.service.location.MozillaLocationService import mozilla.components.service.sync.logins.SyncableLoginsStorage import mozilla.components.support.locale.LocaleManager import org.mozilla.fenix.AppRequestInterceptor +import org.mozilla.fenix.BuildConfig import org.mozilla.fenix.Config import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.perf.StrictModeManager import org.mozilla.fenix.TelemetryMiddleware +import org.mozilla.fenix.components.search.SearchMigration import org.mozilla.fenix.downloads.DownloadService import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings @@ -153,6 +159,14 @@ class Core( SessionStorage(context, engine = engine) } + private val locationService: LocationService by lazyMonitored { + if (Config.channel.isDebug || BuildConfig.MLS_TOKEN.isEmpty()) { + LocationService.default() + } else { + MozillaLocationService(context, client, BuildConfig.MLS_TOKEN) + } + } + /** * The [BrowserStore] holds the global [BrowserState]. */ @@ -169,7 +183,13 @@ class Core( metrics ), ThumbnailsMiddleware(thumbnailStorage), - UndoMiddleware(::lookupSessionManager, context.getUndoDelay()) + UndoMiddleware(::lookupSessionManager, context.getUndoDelay()), + RegionMiddleware(context, locationService), + SearchMiddleware( + context, + additionalBundledSearchEngineIds = listOf("reddit", "youtube"), + migration = SearchMigration(context) + ) ) + EngineMiddleware.create(engine, ::findSessionById) ).also { it.dispatch(RecentlyClosedAction.InitializeRecentlyClosedState) diff --git a/app/src/main/java/org/mozilla/fenix/components/Search.kt b/app/src/main/java/org/mozilla/fenix/components/Search.kt deleted file mode 100644 index 9b752b20a..000000000 --- a/app/src/main/java/org/mozilla/fenix/components/Search.kt +++ /dev/null @@ -1,37 +0,0 @@ -/* 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/. */ - -package org.mozilla.fenix.components - -import android.content.Context -import kotlinx.coroutines.Dispatchers.IO -import kotlinx.coroutines.GlobalScope -import kotlinx.coroutines.launch -import mozilla.components.browser.search.SearchEngineManager -import org.mozilla.fenix.components.searchengine.FenixSearchEngineProvider -import org.mozilla.fenix.perf.lazyMonitored -import org.mozilla.fenix.utils.Mockable - -/** - * Component group for all search engine integration related functionality. - */ -@Mockable -class Search(private val context: Context) { - val provider = FenixSearchEngineProvider(context) - - /** - * This component provides access to a centralized registry of search engines. - */ - val searchEngineManager by lazyMonitored { - SearchEngineManager( - coroutineContext = IO, - providers = listOf(provider) - ).apply { - registerForLocaleUpdates(context) - GlobalScope.launch { - defaultSearchEngine = provider.getDefaultEngine(context) - } - } - } -} diff --git a/app/src/main/java/org/mozilla/fenix/components/UseCases.kt b/app/src/main/java/org/mozilla/fenix/components/UseCases.kt index 86b9bfabc..317a971fd 100644 --- a/app/src/main/java/org/mozilla/fenix/components/UseCases.kt +++ b/app/src/main/java/org/mozilla/fenix/components/UseCases.kt @@ -5,7 +5,6 @@ package org.mozilla.fenix.components import android.content.Context -import mozilla.components.browser.search.SearchEngineManager import mozilla.components.browser.session.SessionManager import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.Engine @@ -15,7 +14,7 @@ import mozilla.components.feature.downloads.DownloadsUseCases import mozilla.components.feature.pwa.WebAppShortcutManager import mozilla.components.feature.pwa.WebAppUseCases import mozilla.components.feature.search.SearchUseCases -import mozilla.components.browser.search.ext.toDefaultSearchEngineProvider +import mozilla.components.feature.search.ext.toDefaultSearchEngineProvider import mozilla.components.feature.session.SessionUseCases import mozilla.components.feature.session.SettingsUseCases import mozilla.components.feature.session.TrackingProtectionUseCases @@ -36,7 +35,6 @@ class UseCases( private val engine: Engine, private val sessionManager: SessionManager, private val store: BrowserStore, - private val searchEngineManager: SearchEngineManager, private val shortcutManager: WebAppShortcutManager, private val topSitesStorage: TopSitesStorage ) { @@ -56,7 +54,7 @@ class UseCases( val searchUseCases by lazyMonitored { SearchUseCases( store, - searchEngineManager.toDefaultSearchEngineProvider(context), + store.toDefaultSearchEngineProvider(), sessionManager ) } diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt index d25cf3770..87d10a519 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt @@ -6,7 +6,7 @@ package org.mozilla.fenix.components.metrics import android.content.Context import mozilla.components.browser.errorpages.ErrorType -import mozilla.components.browser.search.SearchEngine +import mozilla.components.browser.state.search.SearchEngine import mozilla.components.feature.top.sites.TopSite import org.mozilla.fenix.GleanMetrics.Addons import org.mozilla.fenix.GleanMetrics.AppTheme @@ -404,7 +404,7 @@ sealed class Event { // https://github.com/mozilla-mobile/fenix/issues/1607 // Sanitize identifiers for custom search engines. val identifier: String - get() = if (isCustom) "custom" else engine.identifier + get() = if (isCustom) "custom" else engine.id val searchEngine: SearchEngine get() = when (this) { diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index b624e2906..82e462d7b 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -5,6 +5,9 @@ package org.mozilla.fenix.components.metrics import android.content.Context +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.feature.search.ext.legacy +import mozilla.components.feature.search.ext.waitForSelectedOrDefaultSearchEngine import mozilla.components.service.fxa.manager.SyncEnginesStorage import mozilla.components.service.glean.Glean import mozilla.components.service.glean.private.NoExtraKeys @@ -682,6 +685,7 @@ private val Event.wrapper: EventWrapper<*>? class GleanMetricsService( private val context: Context, + private val store: Lazy, private val browsersCache: BrowsersCache = BrowsersCache, private val mozillaProductDetector: MozillaProductDetector = MozillaProductDetector ) : MetricsService { @@ -756,20 +760,18 @@ class GleanMetricsService( closeTabSetting.set(context.settings().getTabTimeoutPingString()) } - SearchDefaultEngine.apply { - val defaultEngine = context - .components - .search - .searchEngineManager - .defaultSearchEngine ?: return@apply + store.value.waitForSelectedOrDefaultSearchEngine { searchEngine -> + if (searchEngine != null) { + SearchDefaultEngine.apply { + code.set(searchEngine.id) + name.set(searchEngine.name) + submissionUrl.set(searchEngine.legacy().buildSearchUrl("")) + } + } - code.set(defaultEngine.identifier) - name.set(defaultEngine.name) - submissionUrl.set(defaultEngine.buildSearchUrl("")) + activationPing.checkAndSend() + installationPing.checkAndSend() } - - activationPing.checkAndSend() - installationPing.checkAndSend() } private fun setPreferenceMetrics() { diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt index 2c9bf3eff..18b53961a 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt @@ -12,11 +12,12 @@ import com.google.android.gms.common.GooglePlayServicesNotAvailableException import com.google.android.gms.common.GooglePlayServicesRepairableException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext -import mozilla.components.browser.search.SearchEngine +import mozilla.components.browser.state.search.SearchEngine +import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.support.base.log.logger.Logger import org.mozilla.fenix.components.metrics.Event.PerformedSearch.SearchAccessPoint -import org.mozilla.fenix.components.searchengine.CustomSearchEngineStore -import org.mozilla.fenix.ext.searchEngineManager +import org.mozilla.fenix.ext.components import java.io.IOException import java.security.NoSuchAlgorithmException import java.security.spec.InvalidKeySpecException @@ -26,11 +27,11 @@ import javax.crypto.spec.PBEKeySpec object MetricsUtils { fun createSearchEvent( engine: SearchEngine, - context: Context, + store: BrowserStore, searchAccessPoint: SearchAccessPoint ): Event.PerformedSearch? { - val isShortcut = engine != context.searchEngineManager.defaultSearchEngine - val isCustom = CustomSearchEngineStore.isCustomSearchEngine(context, engine.identifier) + val isShortcut = engine != store.state.search.selectedOrDefaultSearchEngine + val isCustom = engine.type == SearchEngine.Type.CUSTOM val engineSource = if (isShortcut) Event.PerformedSearch.EngineSource.Shortcut(engine, isCustom) diff --git a/app/src/main/java/org/mozilla/fenix/components/search/SearchMigration.kt b/app/src/main/java/org/mozilla/fenix/components/search/SearchMigration.kt new file mode 100644 index 000000000..9d421a82d --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/components/search/SearchMigration.kt @@ -0,0 +1,70 @@ +/* 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/. */ + +package org.mozilla.fenix.components.search + +import android.content.Context +import android.content.SharedPreferences +import mozilla.components.browser.search.SearchEngineParser +import mozilla.components.browser.state.search.SearchEngine +import mozilla.components.feature.search.ext.migrate +import mozilla.components.feature.search.middleware.SearchMiddleware +import org.mozilla.fenix.ext.components +import org.xmlpull.v1.XmlPullParserException +import java.io.BufferedInputStream +import java.io.IOException + +private const val PREF_FILE_SEARCH_ENGINES = "custom-search-engines" + +private const val PREF_KEY_CUSTOM_SEARCH_ENGINES = "pref_custom_search_engines" +private const val PREF_KEY_MIGRATED = "pref_search_migrated" + +/** + * Helper class to migrate the search related data in Fenix to the "Android Components" implementation. + */ +internal class SearchMigration( + private val context: Context +) : SearchMiddleware.Migration { + + override fun getValuesToMigrate(): SearchMiddleware.Migration.MigrationValues? { + val preferences = context.getSharedPreferences(PREF_FILE_SEARCH_ENGINES, Context.MODE_PRIVATE) + if (preferences.getBoolean(PREF_KEY_MIGRATED, false)) { + return null + } + + val values = SearchMiddleware.Migration.MigrationValues( + customSearchEngines = loadCustomSearchEngines(preferences), + defaultSearchEngineName = context.components.settings.defaultSearchEngineName + ) + + preferences.edit() + .putBoolean(PREF_KEY_MIGRATED, true) + .apply() + + return values + } + + private fun loadCustomSearchEngines( + preferences: SharedPreferences + ): List { + val ids = preferences.getStringSet(PREF_KEY_CUSTOM_SEARCH_ENGINES, emptySet()) ?: emptySet() + + val parser = SearchEngineParser() + + return ids.mapNotNull { id -> + val xml = preferences.getString(id, null) + parser.loadSafely(id, xml?.byteInputStream()?.buffered()) + } + } +} + +private fun SearchEngineParser.loadSafely(id: String, stream: BufferedInputStream?): SearchEngine? { + return try { + stream?.let { load(id, it).migrate() } + } catch (e: IOException) { + null + } catch (e: XmlPullParserException) { + null + } +} diff --git a/app/src/main/java/org/mozilla/fenix/components/searchengine/CustomSearchEngineStore.kt b/app/src/main/java/org/mozilla/fenix/components/searchengine/CustomSearchEngineStore.kt deleted file mode 100644 index ef0c840e5..000000000 --- a/app/src/main/java/org/mozilla/fenix/components/searchengine/CustomSearchEngineStore.kt +++ /dev/null @@ -1,129 +0,0 @@ -/* 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/. */ - -package org.mozilla.fenix.components.searchengine - -import android.content.Context -import android.content.SharedPreferences -import mozilla.components.browser.icons.IconRequest -import mozilla.components.browser.search.SearchEngine -import mozilla.components.browser.search.SearchEngineParser -import mozilla.components.browser.search.provider.SearchEngineList -import mozilla.components.browser.search.provider.SearchEngineProvider -import mozilla.components.support.ktx.android.content.PreferencesHolder -import mozilla.components.support.ktx.android.content.stringSetPreference -import org.mozilla.fenix.ext.components - -/** - * SearchEngineProvider implementation to load user entered custom search engines. - */ -class CustomSearchEngineProvider : SearchEngineProvider { - override suspend fun loadSearchEngines(context: Context): SearchEngineList { - return SearchEngineList(CustomSearchEngineStore.loadCustomSearchEngines(context), null) - } -} - -/** - * Object to handle storing custom search engines - */ -object CustomSearchEngineStore { - class EngineNameAlreadyExists : Exception() - - /** - * Add a search engine to the store. - * @param context [Context] used for various Android interactions. - * @param engineName The name of the search engine - * @param searchQuery The templated search string for the search engine - * @throws EngineNameAlreadyExists if you try to add a search engine that already exists - */ - suspend fun addSearchEngine(context: Context, engineName: String, searchQuery: String) { - val storage = engineStorage(context) - if (storage.customSearchEngineIds.contains(engineName)) { throw EngineNameAlreadyExists() } - - val icon = context.components.core.icons.loadIcon(IconRequest(searchQuery)).await() - val searchEngineXml = SearchEngineWriter.buildSearchEngineXML(engineName, searchQuery, icon.bitmap) - val engines = storage.customSearchEngineIds.toMutableSet() - engines.add(engineName) - storage.customSearchEngineIds = engines - storage[engineName] = searchEngineXml - } - - /** - * Updates an existing search engine. - * To prevent duplicate search engines we want to remove the old engine before adding the new one - * @param context [Context] used for various Android interactions. - * @param oldEngineName the name of the engine you want to replace - * @param newEngineName the name of the engine you want to save - * @param searchQuery The templated search string for the search engine - */ - suspend fun updateSearchEngine( - context: Context, - oldEngineName: String, - newEngineName: String, - searchQuery: String - ) { - removeSearchEngine(context, oldEngineName) - addSearchEngine(context, newEngineName, searchQuery) - } - - /** - * Removes a search engine from the store - * @param context [Context] used for various Android interactions. - * @param engineId the id of the engine you want to remove - */ - fun removeSearchEngine(context: Context, engineId: String) { - val storage = engineStorage(context) - val customEngines = storage.customSearchEngineIds - storage.customSearchEngineIds = customEngines.filterNot { it == engineId }.toSet() - storage[engineId] = null - } - - /** - * Checks the store to see if it contains a search engine - * @param context [Context] used for various Android interactions. - * @param engineId The name of the engine to check - */ - fun isCustomSearchEngine(context: Context, engineId: String): Boolean { - val storage = engineStorage(context) - return storage.customSearchEngineIds.contains(engineId) - } - - /** - * Creates a list of [SearchEngine] from the store - * @param context [Context] used for various Android interactions. - */ - fun loadCustomSearchEngines(context: Context): List { - val storage = engineStorage(context) - val parser = SearchEngineParser() - val engines = storage.customSearchEngineIds - - return engines.mapNotNull { - val engineXml = storage[it] ?: return@mapNotNull null - val engineInputStream = engineXml.byteInputStream().buffered() - parser.load(it, engineInputStream) - } - } - - /** - * Creates a helper object to help interact with [SharedPreferences] - * @param context [Context] used for various Android interactions. - */ - private fun engineStorage(context: Context) = object : PreferencesHolder { - override val preferences: SharedPreferences - get() = context.getSharedPreferences(PREF_FILE_SEARCH_ENGINES, Context.MODE_PRIVATE) - - var customSearchEngineIds by stringSetPreference(PREF_KEY_CUSTOM_SEARCH_ENGINES, emptySet()) - - operator fun get(engineId: String): String? { - return preferences.getString(engineId, null) - } - - operator fun set(engineId: String, value: String?) { - preferences.edit().putString(engineId, value).apply() - } - } - - private const val PREF_KEY_CUSTOM_SEARCH_ENGINES = "pref_custom_search_engines" - const val PREF_FILE_SEARCH_ENGINES = "custom-search-engines" -} diff --git a/app/src/main/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProvider.kt b/app/src/main/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProvider.kt deleted file mode 100644 index 6b4b2a78d..000000000 --- a/app/src/main/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProvider.kt +++ /dev/null @@ -1,306 +0,0 @@ -/* 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/. */ - -package org.mozilla.fenix.components.searchengine - -import android.content.Context -import androidx.annotation.VisibleForTesting -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Deferred -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.Job -import kotlinx.coroutines.async -import kotlinx.coroutines.launch -import mozilla.components.browser.search.SearchEngine -import mozilla.components.browser.search.provider.AssetsSearchEngineProvider -import mozilla.components.browser.search.provider.SearchEngineList -import mozilla.components.browser.search.provider.SearchEngineProvider -import mozilla.components.browser.search.provider.filter.SearchEngineFilter -import mozilla.components.browser.search.provider.localization.LocaleSearchLocalizationProvider -import mozilla.components.browser.search.provider.localization.SearchLocalizationProvider -import mozilla.components.service.location.LocationService -import mozilla.components.service.location.MozillaLocationService -import mozilla.components.service.location.search.RegionSearchLocalizationProvider -import org.mozilla.fenix.BuildConfig -import org.mozilla.fenix.Config -import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.settings -import org.mozilla.fenix.perf.runBlockingIncrement -import java.util.Locale - -@SuppressWarnings("TooManyFunctions") -open class FenixSearchEngineProvider( - private val context: Context -) : SearchEngineProvider, CoroutineScope by CoroutineScope(Job() + Dispatchers.IO) { - private val shouldMockMLS = Config.channel.isDebug || BuildConfig.MLS_TOKEN.isEmpty() - private val locationService: LocationService = if (shouldMockMLS) { - LocationService.dummy() - } else { - MozillaLocationService( - context, - context.components.core.client, - BuildConfig.MLS_TOKEN - ) - } - - // We have two search engine types: one based on MLS reported region, one based only on Locale. - // There are multiple steps involved in returning the default search engine for example. - // Simplest and most effective way to make sure the MLS engines do not mix with Locale based engines - // is to use the same type of engines for the entire duration of the app's run. - // See fenix/issues/11875 - private val isRegionCachedByLocationService = locationService.hasRegionCached() - - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - open val localizationProvider: SearchLocalizationProvider = - RegionSearchLocalizationProvider(locationService) - - /** - * Unfiltered list of search engines based on locale. - */ - open var baseSearchEngines = async { - AssetsSearchEngineProvider(localizationProvider) - .loadSearchEngines(context) - } - - private val loadedRegion = async { localizationProvider.determineRegion() } - - // https://github.com/mozilla-mobile/fenix/issues/9935 - // Adds a Locale search engine provider as a fallback in case the MLS lookup takes longer - // than the time it takes for a user to try to search. - private val fallbackLocationService: SearchLocalizationProvider = LocaleSearchLocalizationProvider() - private val fallBackProvider = - AssetsSearchEngineProvider(fallbackLocationService) - - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - open val fallbackEngines = async { fallBackProvider.loadSearchEngines(context) } - private val fallbackRegion = async { fallbackLocationService.determineRegion() } - - /** - * Default bundled search engines based on locale. - */ - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - open val bundledSearchEngines = async { - val defaultEngineIdentifiers = - baseSearchEngines.await().list.map { it.identifier }.toSet() - AssetsSearchEngineProvider( - localizationProvider, - filters = listOf(object : SearchEngineFilter { - override fun filter(context: Context, searchEngine: SearchEngine): Boolean { - return BUNDLED_SEARCH_ENGINES.contains(searchEngine.identifier) && - !defaultEngineIdentifiers.contains(searchEngine.identifier) - } - }), - additionalIdentifiers = BUNDLED_SEARCH_ENGINES - ).loadSearchEngines(context) - } - - /** - * Search engines that have been manually added by a user. - */ - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - open var customSearchEngines = async { - CustomSearchEngineProvider().loadSearchEngines(context) - } - - private var loadedSearchEngines = refreshInstalledEngineListAsync(baseSearchEngines) - - // https://github.com/mozilla-mobile/fenix/issues/9935 - // Create new getter that will return the fallback SearchEngineList if - // the main one hasn't completed yet - private val searchEngines: Deferred - get() = - if (isRegionCachedByLocationService) { - loadedSearchEngines - } else { - refreshInstalledEngineListAsync(fallbackEngines) - } - - fun getDefaultEngine(context: Context): SearchEngine { - val engines = installedSearchEngines(context) - val selectedName = context.settings().defaultSearchEngineName - - return engines.list.find { it.name == selectedName } - ?: engines.default - ?: engines.list.first() - } - - // We should only be setting the default search engine here - fun setDefaultEngine(context: Context, id: String) { - val engines = installedSearchEngines(context) - val newDefault = engines.list.find { it.name == id } - ?: engines.default - ?: engines.list.first() - - context.settings().defaultSearchEngineName = newDefault.name - context.components.search.searchEngineManager.defaultSearchEngine = newDefault - } - - /** - * @return a list of all SearchEngines that are currently active. These are the engines that - * are readily available throughout the app. Includes all installed engines, both - * default and custom - */ - fun installedSearchEngines(context: Context): SearchEngineList = runBlockingIncrement { - val installedIdentifiers = installedSearchEngineIdentifiers(context) - val defaultList = searchEngines.await() - - defaultList.copy( - list = defaultList.list.filter { - installedIdentifiers.contains(it.identifier) - }.sortedBy { - it.name.toLowerCase(Locale.getDefault()) - }, - default = defaultList.default?.let { - if (installedIdentifiers.contains(it.identifier)) { - it - } else { - null - } - } - ) - } - - fun allSearchEngineIdentifiers() = runBlockingIncrement { - loadedSearchEngines.await().list.map { it.identifier } - } - - fun uninstalledSearchEngines(context: Context): SearchEngineList = runBlockingIncrement { - val installedIdentifiers = installedSearchEngineIdentifiers(context) - val engineList = loadedSearchEngines.await() - - return@runBlockingIncrement engineList.copy( - list = engineList.list.filterNot { installedIdentifiers.contains(it.identifier) } - ) - } - - override suspend fun loadSearchEngines(context: Context): SearchEngineList { - return installedSearchEngines(context) - } - - fun installSearchEngine( - context: Context, - searchEngine: SearchEngine, - isCustom: Boolean = false - ) = runBlockingIncrement { - if (isCustom) { - val searchUrl = searchEngine.getSearchTemplate() - CustomSearchEngineStore.addSearchEngine(context, searchEngine.name, searchUrl) - reload() - } else { - val installedIdentifiers = installedSearchEngineIdentifiers(context).toMutableSet() - installedIdentifiers.add(searchEngine.identifier) - prefs(context).edit() - .putStringSet( - localeAwareInstalledEnginesKey(), installedIdentifiers - ).apply() - } - } - - fun uninstallSearchEngine( - context: Context, - searchEngine: SearchEngine, - isCustom: Boolean = false - ) = runBlockingIncrement { - if (isCustom) { - CustomSearchEngineStore.removeSearchEngine(context, searchEngine.identifier) - reload() - } else { - val installedIdentifiers = installedSearchEngineIdentifiers(context).toMutableSet() - installedIdentifiers.remove(searchEngine.identifier) - prefs(context).edit().putStringSet( - localeAwareInstalledEnginesKey(), - installedIdentifiers - ).apply() - } - } - - fun reload() { - launch { - customSearchEngines = async { CustomSearchEngineProvider().loadSearchEngines(context) } - loadedSearchEngines = refreshInstalledEngineListAsync(baseSearchEngines) - } - } - - // When we change the locale we need to update the baseSearchEngines list - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - open fun updateBaseSearchEngines() { - baseSearchEngines = async { - AssetsSearchEngineProvider(localizationProvider) - .loadSearchEngines(context) - } - } - - private fun refreshInstalledEngineListAsync( - engines: Deferred - ): Deferred = async { - val engineList = engines.await() - val bundledList = bundledSearchEngines.await().list - val customList = customSearchEngines.await().list - - return@async engineList.copy(list = engineList.list + bundledList + customList) - } - - private fun prefs(context: Context) = context.getSharedPreferences( - PREF_FILE_SEARCH_ENGINES, - Context.MODE_PRIVATE - ) - - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - suspend fun installedSearchEngineIdentifiers(context: Context): Set { - val prefs = prefs(context) - val installedEnginesKey = localeAwareInstalledEnginesKey() - - if (!prefs.contains(installedEnginesKey)) { - val searchEngines = - if (isRegionCachedByLocationService) { - baseSearchEngines - } else { - fallbackEngines - } - - val defaultSet = searchEngines.await() - .list - .map { it.identifier } - .toSet() - - prefs.edit().putStringSet(installedEnginesKey, defaultSet).apply() - } - - val installedIdentifiers: Set = - prefs(context).getStringSet(installedEnginesKey, setOf()) ?: setOf() - - val customEngineIdentifiers = - customSearchEngines.await().list.map { it.identifier }.toSet() - - return installedIdentifiers + customEngineIdentifiers - } - - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - suspend fun localeAwareInstalledEnginesKey(): String { - val tag = if (isRegionCachedByLocationService) { - val localization = loadedRegion.await() - val region = localization.region?.let { - if (it.isEmpty()) "" else "-$it" - } - - "${localization.languageTag}$region" - } else { - val localization = fallbackRegion.await() - val region = localization.region?.let { - if (it.isEmpty()) "" else "-$it" - } - - "${localization.languageTag}$region-fallback" - } - - return "$INSTALLED_ENGINES_KEY-$tag" - } - - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - companion object { - val BUNDLED_SEARCH_ENGINES = listOf("reddit", "youtube") - const val PREF_FILE_SEARCH_ENGINES = "fenix-search-engine-provider" - const val INSTALLED_ENGINES_KEY = "fenix-installed-search-engines" - } -} diff --git a/app/src/main/java/org/mozilla/fenix/components/searchengine/SearchEngineWriter.kt b/app/src/main/java/org/mozilla/fenix/components/searchengine/SearchEngineWriter.kt deleted file mode 100644 index 7366942e8..000000000 --- a/app/src/main/java/org/mozilla/fenix/components/searchengine/SearchEngineWriter.kt +++ /dev/null @@ -1,84 +0,0 @@ -/* 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/. */ - -package org.mozilla.fenix.components.searchengine - -import android.graphics.Bitmap -import android.util.Base64 -import android.util.Log -import org.w3c.dom.Document -import java.io.ByteArrayOutputStream -import java.io.StringWriter -import javax.xml.parsers.DocumentBuilderFactory -import javax.xml.parsers.ParserConfigurationException -import javax.xml.transform.OutputKeys -import javax.xml.transform.TransformerConfigurationException -import javax.xml.transform.TransformerException -import javax.xml.transform.TransformerFactory -import javax.xml.transform.dom.DOMSource -import javax.xml.transform.stream.StreamResult - -private const val BITMAP_COMPRESS_QUALITY = 100 -private fun Bitmap.toBase64(): String { - val stream = ByteArrayOutputStream() - compress(Bitmap.CompressFormat.PNG, BITMAP_COMPRESS_QUALITY, stream) - val encodedImage = Base64.encodeToString(stream.toByteArray(), Base64.DEFAULT) - return "data:image/png;base64,$encodedImage" -} - -class SearchEngineWriter { - companion object { - private const val LOG_TAG = "SearchEngineWriter" - - fun buildSearchEngineXML(engineName: String, searchQuery: String, iconBitmap: Bitmap): String? { - try { - val document = DocumentBuilderFactory.newInstance().newDocumentBuilder().newDocument() - val rootElement = document!!.createElement("OpenSearchDescription") - rootElement.setAttribute("xmlns", "http://a9.com/-/spec/opensearch/1.1/") - document.appendChild(rootElement) - - val shortNameElement = document.createElement("ShortName") - shortNameElement.textContent = engineName - rootElement.appendChild(shortNameElement) - - val imageElement = document.createElement("Image") - imageElement.setAttribute("width", "16") - imageElement.setAttribute("height", "16") - imageElement.textContent = iconBitmap.toBase64() - rootElement.appendChild(imageElement) - - val descriptionElement = document.createElement("Description") - descriptionElement.textContent = engineName - rootElement.appendChild(descriptionElement) - - val urlElement = document.createElement("Url") - urlElement.setAttribute("type", "text/html") - - val templateSearchString = searchQuery.replace("%s", "{searchTerms}") - urlElement.setAttribute("template", templateSearchString) - rootElement.appendChild(urlElement) - - return xmlToString(document) - } catch (e: ParserConfigurationException) { - Log.e(LOG_TAG, "Couldn't create new Document for building search engine XML", e) - return null - } - } - - private fun xmlToString(doc: Document): String? { - val writer = StringWriter() - try { - val tf = TransformerFactory.newInstance().newTransformer() - tf.setOutputProperty(OutputKeys.ENCODING, "UTF-8") - tf.transform(DOMSource(doc), StreamResult(writer)) - } catch (e: TransformerConfigurationException) { - return null - } catch (e: TransformerException) { - return null - } - - return writer.toString() - } - } -} diff --git a/app/src/main/java/org/mozilla/fenix/ext/Context.kt b/app/src/main/java/org/mozilla/fenix/ext/Context.kt index d10637026..e12382da5 100644 --- a/app/src/main/java/org/mozilla/fenix/ext/Context.kt +++ b/app/src/main/java/org/mozilla/fenix/ext/Context.kt @@ -14,7 +14,6 @@ import android.view.View import android.view.ViewGroup import android.view.accessibility.AccessibilityManager import androidx.annotation.StringRes -import mozilla.components.browser.search.SearchEngineManager import mozilla.components.support.locale.LocaleManager import org.mozilla.fenix.FenixApplication import org.mozilla.fenix.components.Components @@ -41,12 +40,6 @@ val Context.components: Components val Context.metrics: MetricController get() = this.components.analytics.metrics -/** - * Helper function to get the SearchEngineManager off of context. - */ -val Context.searchEngineManager: SearchEngineManager - get() = this.components.search.searchEngineManager - fun Context.asActivity() = (this as? ContextThemeWrapper)?.baseContext as? Activity ?: this as? Activity diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index 44ec66b96..94dd6c009 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -40,15 +40,26 @@ import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.RecyclerView import androidx.recyclerview.widget.RecyclerView.SCROLL_STATE_IDLE import com.google.android.material.snackbar.Snackbar -import kotlinx.android.synthetic.main.fragment_home.* -import kotlinx.android.synthetic.main.fragment_home.view.* -import kotlinx.android.synthetic.main.no_collections_message.view.* +import kotlinx.android.synthetic.main.fragment_home.privateBrowsingButton +import kotlinx.android.synthetic.main.fragment_home.search_engine_icon +import kotlinx.android.synthetic.main.fragment_home.toolbarLayout +import kotlinx.android.synthetic.main.fragment_home.view.bottomBarShadow +import kotlinx.android.synthetic.main.fragment_home.view.bottom_bar +import kotlinx.android.synthetic.main.fragment_home.view.homeAppBar +import kotlinx.android.synthetic.main.fragment_home.view.menuButton +import kotlinx.android.synthetic.main.fragment_home.view.sessionControlRecyclerView +import kotlinx.android.synthetic.main.fragment_home.view.tab_button +import kotlinx.android.synthetic.main.fragment_home.view.toolbar +import kotlinx.android.synthetic.main.fragment_home.view.toolbarLayout +import kotlinx.android.synthetic.main.fragment_home.view.toolbar_wrapper +import kotlinx.android.synthetic.main.no_collections_message.view.add_tabs_to_collections_button import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import mozilla.appservices.places.BookmarkRoot import mozilla.components.browser.menu.view.MenuButton import mozilla.components.browser.session.Session @@ -58,6 +69,7 @@ import mozilla.components.browser.state.selector.getNormalOrPrivateTabs import mozilla.components.browser.state.selector.normalTabs import mozilla.components.browser.state.selector.privateTabs import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.sync.AccountObserver import mozilla.components.concept.sync.AuthType @@ -65,9 +77,11 @@ import mozilla.components.concept.sync.OAuthAccount import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.top.sites.TopSitesConfig import mozilla.components.feature.top.sites.TopSitesFeature +import mozilla.components.lib.state.ext.consumeFlow import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.support.base.feature.ViewBoundFeatureWrapper import mozilla.components.support.ktx.android.content.res.resolveAttribute +import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R @@ -162,6 +176,7 @@ class HomeFragment : Fragment() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) + postponeEnterTransition() bundleArgs = args.toBundle() lifecycleScope.launch(IO) { @@ -227,6 +242,7 @@ class HomeFragment : Fragment() { settings = components.settings, engine = components.core.engine, metrics = components.analytics.metrics, + store = store, sessionManager = sessionManager, tabCollectionStorage = components.core.tabCollectionStorage, addTabUseCase = components.useCases.tabsUseCases.addTab, @@ -338,23 +354,7 @@ class HomeFragment : Fragment() { delay(ANIMATION_DELAY) } - viewLifecycleOwner.lifecycleScope.launch(IO) { - // This is necessary due to a bug in viewLifecycleOwner. See: - // https://github.com/mozilla-mobile/android-components/blob/master/components/lib/state/src/main/java/mozilla/components/lib/state/ext/Fragment.kt#L32-L56 - // TODO remove when viewLifecycleOwner is fixed - val context = context ?: return@launch - - val iconSize = - context.resources.getDimensionPixelSize(R.dimen.preference_icon_drawable_size) - - val searchEngine = context.components.search.provider.getDefaultEngine(context) - val searchIcon = BitmapDrawable(context.resources, searchEngine.icon) - searchIcon.setBounds(0, 0, iconSize, iconSize) - - withContext(Main) { - search_engine_icon?.setImageDrawable(searchIcon) - } - } + observeSearchEngineChanges() createHomeMenu(requireContext(), WeakReference(view.menuButton)) val tabCounterMenu = TabCounterMenu( @@ -446,6 +446,24 @@ class HomeFragment : Fragment() { } } + private fun observeSearchEngineChanges() { + consumeFlow(store) { flow -> + flow.map { state -> state.search.selectedOrDefaultSearchEngine } + .ifChanged() + .collect { searchEngine -> + if (searchEngine != null) { + val iconSize = + requireContext().resources.getDimensionPixelSize(R.dimen.preference_icon_drawable_size) + val searchIcon = BitmapDrawable(requireContext().resources, searchEngine.icon) + searchIcon.setBounds(0, 0, iconSize, iconSize) + search_engine_icon?.setImageDrawable(searchIcon) + } else { + search_engine_icon.setImageDrawable(null) + } + } + } + } + private fun removeAllTabsAndShowSnackbar(sessionCode: String) { if (sessionCode == ALL_PRIVATE_TABS) { sessionManager.removePrivateSessions() @@ -499,6 +517,7 @@ class HomeFragment : Fragment() { override fun onDestroyView() { super.onDestroyView() + _sessionControlInteractor = null sessionControlView = null bundleArgs.clear() @@ -507,6 +526,7 @@ class HomeFragment : Fragment() { override fun onStart() { super.onStart() + subscribeToTabCollections() val context = requireContext() diff --git a/app/src/main/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessor.kt b/app/src/main/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessor.kt index be6fe57de..ace0aeacc 100644 --- a/app/src/main/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessor.kt +++ b/app/src/main/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessor.kt @@ -7,6 +7,9 @@ package org.mozilla.fenix.home.intent import android.content.Intent import android.os.StrictMode import androidx.navigation.NavController +import mozilla.components.browser.state.search.SearchEngine +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.feature.search.ext.waitForSelectedOrDefaultSearchEngine import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.components.metrics.Event @@ -21,30 +24,48 @@ import org.mozilla.fenix.widget.VoiceSearchActivity.Companion.SPEECH_PROCESSING */ class SpeechProcessingIntentProcessor( private val activity: HomeActivity, + private val store: BrowserStore, private val metrics: MetricController ) : HomeIntentProcessor { override fun process(intent: Intent, navController: NavController, out: Intent): Boolean { - return if (intent.extras?.getBoolean(HomeActivity.OPEN_TO_BROWSER_AND_LOAD) == true) { - out.putExtra(HomeActivity.OPEN_TO_BROWSER_AND_LOAD, false) - activity.components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { - val searchEvent = MetricsUtils.createSearchEvent( - activity.components.search.provider.getDefaultEngine(activity), - activity, - Event.PerformedSearch.SearchAccessPoint.WIDGET + if ( + !intent.hasExtra(SPEECH_PROCESSING) || + intent.extras?.getBoolean(HomeActivity.OPEN_TO_BROWSER_AND_LOAD) != true + ) { + return false + } + + out.putExtra(HomeActivity.OPEN_TO_BROWSER_AND_LOAD, false) + + store.waitForSelectedOrDefaultSearchEngine { searchEngine -> + if (searchEngine != null) { + launchToBrowser( + searchEngine, + intent.getStringExtra(SPEECH_PROCESSING).orEmpty() ) - searchEvent?.let { metrics.track(it) } } + } - activity.openToBrowserAndLoad( - searchTermOrURL = intent.getStringExtra(SPEECH_PROCESSING).orEmpty(), - newTab = true, - from = BrowserDirection.FromGlobal, - forceSearch = true + return true + } + + private fun launchToBrowser(searchEngine: SearchEngine, text: String) { + activity.components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + val searchEvent = MetricsUtils.createSearchEvent( + searchEngine, + store, + Event.PerformedSearch.SearchAccessPoint.WIDGET ) - true - } else { - false + searchEvent?.let { metrics.track(it) } } + + activity.openToBrowserAndLoad( + searchTermOrURL = text, + newTab = true, + from = BrowserDirection.FromGlobal, + engine = searchEngine, + forceSearch = true + ) } } diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt index d52c1ac18..9e94fcaff 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt @@ -12,6 +12,8 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import mozilla.components.browser.session.SessionManager +import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.Engine import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.feature.tab.collections.TabCollection @@ -171,6 +173,7 @@ class DefaultSessionControlController( private val engine: Engine, private val metrics: MetricController, private val sessionManager: SessionManager, + private val store: BrowserStore, private val tabCollectionStorage: TabCollectionStorage, private val addTabUseCase: TabsUseCases.AddNewTabUseCase, private val fragmentStore: HomeFragmentStore, @@ -462,21 +465,23 @@ class DefaultSessionControlController( } override fun handlePasteAndGo(clipboardText: String) { + val searchEngine = store.state.search.selectedOrDefaultSearchEngine + activity.openToBrowserAndLoad( searchTermOrURL = clipboardText, newTab = true, from = BrowserDirection.FromHome, - engine = activity.components.search.provider.getDefaultEngine(activity) + engine = searchEngine ) - val event = if (clipboardText.isUrl()) { + val event = if (clipboardText.isUrl() || searchEngine == null) { Event.EnteredUrl(false) } else { val searchAccessPoint = Event.PerformedSearch.SearchAccessPoint.ACTION searchAccessPoint.let { sap -> MetricsUtils.createSearchEvent( - activity.components.search.provider.getDefaultEngine(activity), - activity, + searchEngine, + store, sap ) } diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchDialogController.kt b/app/src/main/java/org/mozilla/fenix/search/SearchDialogController.kt index c6ffbf66b..92e848bb6 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchDialogController.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchDialogController.kt @@ -12,9 +12,10 @@ import android.text.SpannableString import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AlertDialog import androidx.navigation.NavController -import mozilla.components.browser.search.SearchEngine import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager +import mozilla.components.browser.state.search.SearchEngine +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.support.ktx.kotlin.isUrl import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity @@ -22,7 +23,6 @@ import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.components.metrics.MetricsUtils -import org.mozilla.fenix.components.searchengine.CustomSearchEngineStore import org.mozilla.fenix.crashes.CrashListActivity import org.mozilla.fenix.ext.navigateSafe import org.mozilla.fenix.settings.SupportUtils @@ -50,7 +50,8 @@ interface SearchController { class SearchDialogController( private val activity: HomeActivity, private val sessionManager: SessionManager, - private val store: SearchFragmentStore, + private val store: BrowserStore, + private val fragmentStore: SearchFragmentStore, private val navController: NavController, private val settings: Settings, private val metrics: MetricController, @@ -81,25 +82,27 @@ class SearchDialogController( } private fun openSearchOrUrl(url: String) { + val searchEngine = fragmentStore.state.searchEngineSource.searchEngine + activity.openToBrowserAndLoad( searchTermOrURL = url, - newTab = store.state.tabId == null, + newTab = fragmentStore.state.tabId == null, from = BrowserDirection.FromSearchDialog, - engine = store.state.searchEngineSource.searchEngine + engine = searchEngine ) - val event = if (url.isUrl()) { + val event = if (url.isUrl() || searchEngine == null) { Event.EnteredUrl(false) } else { - val searchAccessPoint = when (store.state.searchAccessPoint) { + val searchAccessPoint = when (fragmentStore.state.searchAccessPoint) { Event.PerformedSearch.SearchAccessPoint.NONE -> Event.PerformedSearch.SearchAccessPoint.ACTION - else -> store.state.searchAccessPoint + else -> fragmentStore.state.searchAccessPoint } searchAccessPoint?.let { sap -> MetricsUtils.createSearchEvent( - store.state.searchEngineSource.searchEngine, - activity, + searchEngine, + store, sap ) } @@ -114,17 +117,17 @@ class SearchDialogController( override fun handleTextChanged(text: String) { // Display the search shortcuts on each entry of the search fragment (see #5308) - val textMatchesCurrentUrl = store.state.url == text - val textMatchesCurrentSearch = store.state.searchTerms == text + val textMatchesCurrentUrl = fragmentStore.state.url == text + val textMatchesCurrentSearch = fragmentStore.state.searchTerms == text - store.dispatch(SearchFragmentAction.UpdateQuery(text)) - store.dispatch( + fragmentStore.dispatch(SearchFragmentAction.UpdateQuery(text)) + fragmentStore.dispatch( SearchFragmentAction.ShowSearchShortcutEnginePicker( (textMatchesCurrentUrl || textMatchesCurrentSearch || text.isEmpty()) && settings.shouldShowSearchShortcuts ) ) - store.dispatch( + fragmentStore.dispatch( SearchFragmentAction.AllowSearchSuggestionsInPrivateModePrompt( text.isNotEmpty() && activity.browsingModeManager.mode.isPrivate && @@ -139,7 +142,7 @@ class SearchDialogController( activity.openToBrowserAndLoad( searchTermOrURL = url, - newTab = store.state.tabId == null, + newTab = fragmentStore.state.tabId == null, from = BrowserDirection.FromSearchDialog ) @@ -149,39 +152,41 @@ class SearchDialogController( override fun handleSearchTermsTapped(searchTerms: String) { clearToolbarFocus() + val searchEngine = fragmentStore.state.searchEngineSource.searchEngine + activity.openToBrowserAndLoad( searchTermOrURL = searchTerms, - newTab = store.state.tabId == null, + newTab = fragmentStore.state.tabId == null, from = BrowserDirection.FromSearchDialog, - engine = store.state.searchEngineSource.searchEngine, + engine = searchEngine, forceSearch = true ) - val searchAccessPoint = when (store.state.searchAccessPoint) { + val searchAccessPoint = when (fragmentStore.state.searchAccessPoint) { Event.PerformedSearch.SearchAccessPoint.NONE -> Event.PerformedSearch.SearchAccessPoint.SUGGESTION - else -> store.state.searchAccessPoint + else -> fragmentStore.state.searchAccessPoint } - val event = searchAccessPoint?.let { sap -> + if (searchAccessPoint != null && searchEngine != null) { MetricsUtils.createSearchEvent( - store.state.searchEngineSource.searchEngine, - activity, - sap - ) + searchEngine, + store, + searchAccessPoint + )?.apply { + metrics.track(this) + } } - event?.let { metrics.track(it) } } override fun handleSearchShortcutEngineSelected(searchEngine: SearchEngine) { - store.dispatch(SearchFragmentAction.SearchShortcutEngineSelected(searchEngine)) - val isCustom = - CustomSearchEngineStore.isCustomSearchEngine(activity, searchEngine.identifier) + fragmentStore.dispatch(SearchFragmentAction.SearchShortcutEngineSelected(searchEngine)) + val isCustom = searchEngine.type == SearchEngine.Type.CUSTOM metrics.track(Event.SearchShortcutSelected(searchEngine, isCustom)) } override fun handleSearchShortcutsButtonClicked() { - val isOpen = store.state.showSearchShortcuts - store.dispatch(SearchFragmentAction.ShowSearchShortcutEnginePicker(!isOpen)) + val isOpen = fragmentStore.state.showSearchShortcuts + fragmentStore.dispatch(SearchFragmentAction.ShowSearchShortcutEnginePicker(!isOpen)) } override fun handleClickSearchEngineSettings() { diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt index 33c00362e..54d6413c6 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt @@ -15,7 +15,6 @@ import android.graphics.Typeface import android.graphics.drawable.ColorDrawable import android.os.Build import android.os.Bundle -import android.os.StrictMode import android.speech.RecognizerIntent import android.text.style.StyleSpan import android.view.LayoutInflater @@ -41,9 +40,12 @@ import kotlinx.android.synthetic.main.fragment_search_dialog.view.* import kotlinx.android.synthetic.main.search_suggestions_hint.view.* import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.launch +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.map import mozilla.components.browser.toolbar.BrowserToolbar import mozilla.components.concept.storage.HistoryStorage import mozilla.components.feature.qr.QrFeature +import mozilla.components.lib.state.ext.consumeFlow import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.components.support.base.feature.ViewBoundFeatureWrapper @@ -52,13 +54,12 @@ import mozilla.components.support.ktx.android.content.hasCamera import mozilla.components.support.ktx.android.content.isPermissionGranted import mozilla.components.support.ktx.android.content.res.getSpanned import mozilla.components.support.ktx.android.view.hideKeyboard +import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged import mozilla.components.ui.autocomplete.InlineAutocompleteEditText import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.components.searchengine.CustomSearchEngineStore -import org.mozilla.fenix.components.searchengine.FenixSearchEngineProvider import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.isKeyboardVisible @@ -67,7 +68,6 @@ import org.mozilla.fenix.ext.settings import org.mozilla.fenix.search.awesomebar.AwesomeBarView import org.mozilla.fenix.search.toolbar.ToolbarView import org.mozilla.fenix.settings.SupportUtils -import org.mozilla.fenix.settings.registerOnSharedPreferenceChangeListener import org.mozilla.fenix.widget.VoiceSearchActivity typealias SearchDialogFragmentStore = SearchFragmentStore @@ -143,7 +143,8 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { SearchDialogController( activity = activity, sessionManager = requireComponents.core.sessionManager, - store = store, + store = requireComponents.core.store, + fragmentStore = store, navController = findNavController(), settings = requireContext().settings(), metrics = requireComponents.analytics.metrics, @@ -170,9 +171,6 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { view.awesome_bar ) - setShortcutsChangedListener(CustomSearchEngineStore.PREF_FILE_SEARCH_ENGINES) - setShortcutsChangedListener(FenixSearchEngineProvider.PREF_FILE_SEARCH_ENGINES) - view.awesome_bar.setOnTouchListener { _, _ -> view.hideKeyboardAndSave() false @@ -203,6 +201,14 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) + consumeFlow(requireComponents.core.store) { flow -> + flow.map { state -> state.search } + .ifChanged() + .collect { search -> + store.dispatch(SearchFragmentAction.UpdateSearchState(search)) + } + } + setupConstraints(view) // When displayed above browser, dismisses dialog on clicking scrim area @@ -475,12 +481,14 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { } private fun addSearchButton(toolbarView: ToolbarView) { + val searchEngine = store.state.searchEngineSource.searchEngine + toolbarView.view.addEditAction( BrowserToolbar.Button( AppCompatResources.getDrawable(requireContext(), R.drawable.ic_microphone)!!, requireContext().getString(R.string.voice_search_content_description), visible = { - store.state.searchEngineSource.searchEngine.identifier.contains("google") && + searchEngine?.id?.contains("google") == true && isSpeechAvailable() && requireContext().settings().shouldShowVoiceSearch }, @@ -515,17 +523,6 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { private fun isSpeechAvailable(): Boolean = speechIntent.resolveActivity(requireContext().packageManager) != null - private fun setShortcutsChangedListener(preferenceFileName: String) { - requireComponents.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { - requireContext().getSharedPreferences( - preferenceFileName, - Context.MODE_PRIVATE - ).registerOnSharedPreferenceChangeListener(viewLifecycleOwner) { _, _ -> - awesomeBarView.update(store.state) - } - } - } - private fun updateClipboardSuggestion(searchState: SearchFragmentState, clipboardUrl: String?) { val shouldShowView = searchState.showClipboardSuggestions && searchState.query.isEmpty() && @@ -548,8 +545,11 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { private fun updateToolbarContentDescription(searchState: SearchFragmentState) { val urlView = toolbarView.view .findViewById(R.id.mozac_browser_toolbar_edit_url_view) - toolbarView.view.contentDescription = - searchState.searchEngineSource.searchEngine.name + ", " + urlView.hint + + searchState.searchEngineSource.searchEngine?.let { engine -> + toolbarView.view.contentDescription = engine.name + ", " + urlView.hint + } + urlView?.importantForAccessibility = View.IMPORTANT_FOR_ACCESSIBILITY_NO } diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchDialogInteractor.kt b/app/src/main/java/org/mozilla/fenix/search/SearchDialogInteractor.kt index cd35bdbce..cb7385ec7 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchDialogInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchDialogInteractor.kt @@ -4,8 +4,8 @@ package org.mozilla.fenix.search -import mozilla.components.browser.search.SearchEngine import mozilla.components.browser.session.Session +import mozilla.components.browser.state.search.SearchEngine import org.mozilla.fenix.search.awesomebar.AwesomeBarInteractor import org.mozilla.fenix.search.toolbar.ToolbarInteractor diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt index e55814e21..1c6570fe9 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt @@ -4,8 +4,11 @@ package org.mozilla.fenix.search -import mozilla.components.browser.search.SearchEngine +import mozilla.components.browser.state.search.SearchEngine import mozilla.components.browser.state.selector.findTab +import mozilla.components.browser.state.state.SearchState +import mozilla.components.browser.state.state.searchEngines +import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine import mozilla.components.lib.state.Action import mozilla.components.lib.state.State import mozilla.components.lib.state.Store @@ -13,7 +16,6 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.components.Components import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.search.ext.areShortcutsAvailable /** * The [Store] for holding the [SearchFragmentState] and applying [SearchFragmentAction]s. @@ -29,7 +31,11 @@ class SearchFragmentStore( * Wraps a `SearchEngine` to give consumers the context that it was selected as a shortcut */ sealed class SearchEngineSource { - abstract val searchEngine: SearchEngine + abstract val searchEngine: SearchEngine? + + object None : SearchEngineSource() { + override val searchEngine: SearchEngine? = null + } data class Default(override val searchEngine: SearchEngine) : SearchEngineSource() data class Shortcut(override val searchEngine: SearchEngine) : SearchEngineSource() @@ -37,17 +43,20 @@ sealed class SearchEngineSource { /** * The state for the Search Screen + * * @property query The current search query string * @property url The current URL of the tab (if this fragment is shown for an already existing tab) * @property searchTerms The search terms used to search previously in this tab (if this fragment is shown * for an already existing tab) * @property searchEngineSource The current selected search engine with the context of how it was selected - * @property defaultEngineSource The current default search engine source + * @property defaultEngine The current default search engine (or null if none is available yet) * @property showSearchSuggestions Whether or not to show search suggestions from the search engine in the AwesomeBar * @property showSearchSuggestionsHint Whether or not to show search suggestions in private hint panel * @property showSearchShortcuts Whether or not to show search shortcuts in the AwesomeBar * @property areShortcutsAvailable Whether or not there are >=2 search engines installed - * so to know to present users with certain options or not. + * so to know to present users with certain options or not. + * @property showSearchShortcutsSetting Whether the setting for showing search shortcuts is enabled + * or disabled. * @property showClipboardSuggestions Whether or not to show clipboard suggestion in the AwesomeBar * @property showHistorySuggestions Whether or not to show history suggestions in the AwesomeBar * @property showBookmarkSuggestions Whether or not to show the bookmark suggestion in the AwesomeBar @@ -58,11 +67,12 @@ data class SearchFragmentState( val url: String, val searchTerms: String, val searchEngineSource: SearchEngineSource, - val defaultEngineSource: SearchEngineSource.Default, + val defaultEngine: SearchEngine?, val showSearchSuggestions: Boolean, val showSearchSuggestionsHint: Boolean, val showSearchShortcuts: Boolean, val areShortcutsAvailable: Boolean, + val showSearchShortcutsSetting: Boolean, val showClipboardSuggestions: Boolean, val showHistorySuggestions: Boolean, val showBookmarkSuggestions: Boolean, @@ -81,16 +91,9 @@ fun createInitialSearchFragmentState( ): SearchFragmentState { val settings = components.settings val tab = tabId?.let { components.core.store.state.findTab(it) } - val url = tab?.content?.url.orEmpty() - val currentSearchEngine = SearchEngineSource.Default( - components.search.provider.getDefaultEngine(activity) - ) - val browsingMode = activity.browsingModeManager.mode - val areShortcutsAvailable = components.search.provider.areShortcutsAvailable(activity) - - val shouldShowSearchSuggestions = when (browsingMode) { + val shouldShowSearchSuggestions = when (activity.browsingModeManager.mode) { BrowsingMode.Normal -> settings.shouldShowSearchSuggestions BrowsingMode.Private -> settings.shouldShowSearchSuggestions && settings.shouldShowSearchSuggestionsInPrivate @@ -100,14 +103,13 @@ fun createInitialSearchFragmentState( query = url, url = url, searchTerms = tab?.content?.searchTerms.orEmpty(), - searchEngineSource = currentSearchEngine, - defaultEngineSource = currentSearchEngine, + searchEngineSource = SearchEngineSource.None, + defaultEngine = null, showSearchSuggestions = shouldShowSearchSuggestions, showSearchSuggestionsHint = false, - showSearchShortcuts = url.isEmpty() && - areShortcutsAvailable && - settings.shouldShowSearchShortcuts, - areShortcutsAvailable = areShortcutsAvailable, + showSearchShortcuts = false, + areShortcutsAvailable = false, + showSearchShortcutsSetting = settings.shouldShowSearchShortcuts, showClipboardSuggestions = settings.shouldShowClipboardSuggestions, showHistorySuggestions = settings.shouldShowHistorySuggestions, showBookmarkSuggestions = settings.shouldShowBookmarkSuggestions, @@ -124,11 +126,14 @@ fun createInitialSearchFragmentState( sealed class SearchFragmentAction : Action { data class SetShowSearchSuggestions(val show: Boolean) : SearchFragmentAction() data class SearchShortcutEngineSelected(val engine: SearchEngine) : SearchFragmentAction() - data class SelectNewDefaultSearchEngine(val engine: SearchEngine) : SearchFragmentAction() data class ShowSearchShortcutEnginePicker(val show: Boolean) : SearchFragmentAction() - data class UpdateShortcutsAvailability(val areShortcutsAvailable: Boolean) : SearchFragmentAction() data class AllowSearchSuggestionsInPrivateModePrompt(val show: Boolean) : SearchFragmentAction() data class UpdateQuery(val query: String) : SearchFragmentAction() + + /** + * Updates the local `SearchFragmentState` from the global `SearchState` in `BrowserStore`. + */ + data class UpdateSearchState(val search: SearchState) : SearchFragmentAction() } /** @@ -143,15 +148,26 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen ) is SearchFragmentAction.ShowSearchShortcutEnginePicker -> state.copy(showSearchShortcuts = action.show && state.areShortcutsAvailable) - is SearchFragmentAction.UpdateShortcutsAvailability -> - state.copy(areShortcutsAvailable = action.areShortcutsAvailable) is SearchFragmentAction.UpdateQuery -> state.copy(query = action.query) - is SearchFragmentAction.SelectNewDefaultSearchEngine -> - state.copy(searchEngineSource = SearchEngineSource.Default(action.engine)) is SearchFragmentAction.AllowSearchSuggestionsInPrivateModePrompt -> state.copy(showSearchSuggestionsHint = action.show) is SearchFragmentAction.SetShowSearchSuggestions -> state.copy(showSearchSuggestions = action.show) + is SearchFragmentAction.UpdateSearchState -> { + state.copy( + defaultEngine = action.search.selectedOrDefaultSearchEngine, + areShortcutsAvailable = action.search.searchEngines.size > 1, + showSearchShortcuts = state.url.isEmpty() && + state.showSearchShortcutsSetting && + action.search.searchEngines.size > 1, + searchEngineSource = if (state.searchEngineSource !is SearchEngineSource.Shortcut) { + action.search.selectedOrDefaultSearchEngine?.let { SearchEngineSource.Default(it) } + ?: SearchEngineSource.None + } else { + state.searchEngineSource + } + ) + } } } diff --git a/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarInteractor.kt b/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarInteractor.kt index e0dac77d0..e8f18cb7e 100644 --- a/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarInteractor.kt @@ -4,8 +4,8 @@ package org.mozilla.fenix.search.awesomebar -import mozilla.components.browser.search.SearchEngine import mozilla.components.browser.session.Session +import mozilla.components.browser.state.search.SearchEngine /** * Interface for the AwesomeBarView Interactor. This interface is implemented by objects that want diff --git a/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt b/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt index 69ea1b7a1..c24d17b6c 100644 --- a/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt +++ b/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarView.kt @@ -10,8 +10,8 @@ import androidx.core.graphics.BlendModeCompat.SRC_IN import androidx.core.graphics.drawable.toBitmap import mozilla.components.browser.awesomebar.BrowserAwesomeBar import mozilla.components.browser.search.DefaultSearchEngineProvider -import mozilla.components.browser.search.SearchEngine import mozilla.components.browser.session.Session +import mozilla.components.browser.state.search.SearchEngine import mozilla.components.concept.awesomebar.AwesomeBar import mozilla.components.concept.engine.EngineSession import mozilla.components.feature.awesomebar.provider.BookmarksStorageSuggestionProvider @@ -20,9 +20,10 @@ import mozilla.components.feature.awesomebar.provider.SearchActionProvider import mozilla.components.feature.awesomebar.provider.SearchSuggestionProvider import mozilla.components.feature.awesomebar.provider.SessionSuggestionProvider import mozilla.components.feature.search.SearchUseCases -import mozilla.components.browser.search.ext.toDefaultSearchEngineProvider -import mozilla.components.feature.syncedtabs.DeviceIndicators +import mozilla.components.feature.search.ext.legacy +import mozilla.components.feature.search.ext.toDefaultSearchEngineProvider import mozilla.components.feature.session.SessionUseCases +import mozilla.components.feature.syncedtabs.DeviceIndicators import mozilla.components.feature.syncedtabs.SyncedTabsStorageSuggestionProvider import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.support.ktx.android.content.getColorFromAttr @@ -32,6 +33,7 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.ext.components import org.mozilla.fenix.search.SearchEngineSource import org.mozilla.fenix.search.SearchFragmentState +import mozilla.components.browser.search.SearchEngine as LegacySearchEngine /** * View that contains and configures the BrowserAwesomeBar @@ -65,7 +67,7 @@ class AwesomeBarView( private val searchUseCase = object : SearchUseCases.SearchUseCase { override fun invoke( searchTerms: String, - searchEngine: SearchEngine?, + searchEngine: mozilla.components.browser.search.SearchEngine?, parentSession: Session? ) { interactor.onSearchTermsTapped(searchTerms) @@ -75,7 +77,7 @@ class AwesomeBarView( private val shortcutSearchUseCase = object : SearchUseCases.SearchUseCase { override fun invoke( searchTerms: String, - searchEngine: SearchEngine?, + searchEngine: mozilla.components.browser.search.SearchEngine?, parentSession: Session? ) { interactor.onSearchTermsTapped(searchTerms) @@ -148,9 +150,7 @@ class AwesomeBarView( defaultSearchSuggestionProvider = SearchSuggestionProvider( context = activity, - defaultSearchEngineProvider = components.search.searchEngineManager.toDefaultSearchEngineProvider( - activity - ), + defaultSearchEngineProvider = components.core.store.toDefaultSearchEngineProvider(), searchUseCase = searchUseCase, fetchClient = components.core.client, mode = SearchSuggestionProvider.Mode.MULTIPLE_SUGGESTIONS, @@ -163,9 +163,7 @@ class AwesomeBarView( defaultSearchActionProvider = SearchActionProvider( - defaultSearchEngineProvider = components.search.searchEngineManager.toDefaultSearchEngineProvider( - activity - ), + defaultSearchEngineProvider = components.core.store.toDefaultSearchEngineProvider(), searchUseCase = searchUseCase, icon = searchBitmap, showDescription = false @@ -173,7 +171,7 @@ class AwesomeBarView( shortcutsEnginePickerProvider = ShortcutsSuggestionProvider( - searchEngineProvider = components.search.provider, + store = components.core.store, context = activity, selectShortcutEngine = interactor::onSearchShortcutEngineSelected, selectShortcutEngineSettings = interactor::onClickSearchEngineSettings @@ -288,6 +286,7 @@ class AwesomeBarView( is SearchEngineSource.Shortcut -> getSuggestionProviderForEngine( state.searchEngineSource.searchEngine ) + is SearchEngineSource.None -> emptyList() } } @@ -311,22 +310,20 @@ class AwesomeBarView( BrowsingMode.Normal -> components.core.engine BrowsingMode.Private -> null } - val searchEngine = - components.search.provider.installedSearchEngines(activity).list.find { it.name == engine.name } - ?: components.search.provider.getDefaultEngine(activity) listOf( SearchActionProvider( defaultSearchEngineProvider = object : DefaultSearchEngineProvider { - override fun getDefaultSearchEngine(): SearchEngine? = searchEngine - override suspend fun retrieveDefaultSearchEngine(): SearchEngine? = - searchEngine + override fun getDefaultSearchEngine(): LegacySearchEngine? = + engine.legacy() + override suspend fun retrieveDefaultSearchEngine(): LegacySearchEngine? = + engine.legacy() }, searchUseCase = shortcutSearchUseCase, icon = searchBitmap ), SearchSuggestionProvider( - searchEngine, + engine.legacy(), shortcutSearchUseCase, components.core.client, limit = 3, diff --git a/app/src/main/java/org/mozilla/fenix/search/awesomebar/ShortcutsSuggestionProvider.kt b/app/src/main/java/org/mozilla/fenix/search/awesomebar/ShortcutsSuggestionProvider.kt index 9bba4db2b..039b2bf24 100644 --- a/app/src/main/java/org/mozilla/fenix/search/awesomebar/ShortcutsSuggestionProvider.kt +++ b/app/src/main/java/org/mozilla/fenix/search/awesomebar/ShortcutsSuggestionProvider.kt @@ -7,17 +7,18 @@ package org.mozilla.fenix.search.awesomebar import android.content.Context import androidx.appcompat.content.res.AppCompatResources import androidx.core.graphics.drawable.toBitmap -import mozilla.components.browser.search.SearchEngine +import mozilla.components.browser.state.search.SearchEngine +import mozilla.components.browser.state.state.searchEngines +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.awesomebar.AwesomeBar import org.mozilla.fenix.R -import org.mozilla.fenix.components.searchengine.FenixSearchEngineProvider import java.util.UUID /** * A [AwesomeBar.SuggestionProvider] implementation that provides search engine suggestions. */ class ShortcutsSuggestionProvider( - private val searchEngineProvider: FenixSearchEngineProvider, + private val store: BrowserStore, private val context: Context, private val selectShortcutEngine: (engine: SearchEngine) -> Unit, private val selectShortcutEngineSettings: () -> Unit @@ -34,10 +35,10 @@ class ShortcutsSuggestionProvider( override suspend fun onInputChanged(text: String): List { val suggestions = mutableListOf() - searchEngineProvider.installedSearchEngines(context).list.mapTo(suggestions) { + store.state.search.searchEngines.mapTo(suggestions) { AwesomeBar.Suggestion( provider = this, - id = it.identifier, + id = it.id, icon = it.icon, title = it.name, onSuggestionClicked = { diff --git a/app/src/main/java/org/mozilla/fenix/search/ext/SearchEngineProvider.kt b/app/src/main/java/org/mozilla/fenix/search/ext/SearchEngineProvider.kt deleted file mode 100644 index cbb598fd8..000000000 --- a/app/src/main/java/org/mozilla/fenix/search/ext/SearchEngineProvider.kt +++ /dev/null @@ -1,17 +0,0 @@ -/* 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/. */ - -package org.mozilla.fenix.search.ext - -import android.content.Context -import org.mozilla.fenix.components.searchengine.FenixSearchEngineProvider - -private const val MINIMUM_SEARCH_ENGINES_NUMBER_TO_SHOW_SHORTCUTS = 2 - -/** - * Return if the user has *at least 2* installed search engines. - * Useful to decide whether to show / enable certain functionalities. - */ -fun FenixSearchEngineProvider.areShortcutsAvailable(context: Context) = - installedSearchEngines(context).list.size >= MINIMUM_SEARCH_ENGINES_NUMBER_TO_SHOW_SHORTCUTS diff --git a/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt b/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt index 037c08708..dea282f36 100644 --- a/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt +++ b/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarView.kt @@ -148,18 +148,22 @@ class ToolbarView( isInitialized = true } - val iconSize = - context.resources.getDimensionPixelSize(R.dimen.preference_icon_drawable_size) + val searchEngine = searchState.searchEngineSource.searchEngine - val scaledIcon = Bitmap.createScaledBitmap( - searchState.searchEngineSource.searchEngine.icon, - iconSize, - iconSize, - true - ) + if (searchEngine != null) { + val iconSize = + context.resources.getDimensionPixelSize(R.dimen.preference_icon_drawable_size) - val icon = BitmapDrawable(context.resources, scaledIcon) + val scaledIcon = Bitmap.createScaledBitmap( + searchEngine.icon, + iconSize, + iconSize, + true + ) + + val icon = BitmapDrawable(context.resources, scaledIcon) - view.edit.setIcon(icon, searchState.searchEngineSource.searchEngine.name) + view.edit.setIcon(icon, searchEngine.name) + } } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/search/AddSearchEngineFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/search/AddSearchEngineFragment.kt index d08520e2d..4521cc365 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/search/AddSearchEngineFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/search/AddSearchEngineFragment.kt @@ -19,26 +19,34 @@ import androidx.constraintlayout.widget.ConstraintLayout import androidx.fragment.app.Fragment import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController -import kotlinx.android.synthetic.main.custom_search_engine.* -import kotlinx.android.synthetic.main.fragment_add_search_engine.* -import kotlinx.android.synthetic.main.search_engine_radio_button.view.* +import kotlinx.android.synthetic.main.custom_search_engine.custom_search_engine_form +import kotlinx.android.synthetic.main.custom_search_engine.custom_search_engine_name_field +import kotlinx.android.synthetic.main.custom_search_engine.custom_search_engine_search_string_field +import kotlinx.android.synthetic.main.custom_search_engine.custom_search_engines_learn_more +import kotlinx.android.synthetic.main.custom_search_engine.edit_engine_name +import kotlinx.android.synthetic.main.custom_search_engine.edit_search_string +import kotlinx.android.synthetic.main.fragment_add_search_engine.search_engine_group +import kotlinx.android.synthetic.main.search_engine_radio_button.view.engine_icon +import kotlinx.android.synthetic.main.search_engine_radio_button.view.engine_text +import kotlinx.android.synthetic.main.search_engine_radio_button.view.overflow_menu +import kotlinx.android.synthetic.main.search_engine_radio_button.view.radio_button import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -import mozilla.components.browser.search.SearchEngine +import mozilla.components.browser.icons.IconRequest +import mozilla.components.browser.state.search.SearchEngine +import mozilla.components.browser.state.state.availableSearchEngines +import mozilla.components.feature.search.ext.createSearchEngine import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.components.searchengine.CustomSearchEngineStore import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.showToolbar -import org.mozilla.fenix.perf.runBlockingIncrement import org.mozilla.fenix.settings.SupportUtils -import java.util.Locale @SuppressWarnings("LargeClass", "TooManyFunctions") class AddSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine), @@ -51,14 +59,13 @@ class AddSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine), super.onCreate(savedInstanceState) setHasOptionsMenu(true) - availableEngines = runBlockingIncrement { - requireContext() - .components - .search - .provider - .uninstalledSearchEngines(requireContext()) - .list - } + availableEngines = requireContext() + .components + .core + .store + .state + .search + .availableSearchEngines selectedIndex = if (availableEngines.isEmpty()) CUSTOM_INDEX else FIRST_INDEX } @@ -72,7 +79,7 @@ class AddSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine), ) val setupSearchEngineItem: (Int, SearchEngine) -> Unit = { index, engine -> - val engineId = engine.identifier + val engineId = engine.id val engineItem = makeButtonFromSearchEngine( engine = engine, layoutInflater = layoutInflater, @@ -123,7 +130,8 @@ class AddSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine), CUSTOM_INDEX -> createCustomEngine() else -> { val engine = availableEngines[selectedIndex] - installEngine(engine) + requireComponents.useCases.searchUseCases.addSearchEngine(engine) + findNavController().popBackStack() } } @@ -141,9 +149,9 @@ class AddSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine), val name = edit_engine_name.text?.toString()?.trim() ?: "" val searchString = edit_search_string.text?.toString() ?: "" - val hasError = checkForErrors(name, searchString) - - if (hasError) { return } + if (checkForErrors(name, searchString)) { + return + } viewLifecycleOwner.lifecycleScope.launch(Main) { val result = withContext(IO) { @@ -159,22 +167,14 @@ class AddSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine), .getString(R.string.search_add_custom_engine_error_cannot_reach, name) } SearchStringValidator.Result.Success -> { - try { - CustomSearchEngineStore.addSearchEngine( - context = requireContext(), - engineName = name, - searchQuery = searchString - ) - } catch (engineNameExists: CustomSearchEngineStore.EngineNameAlreadyExists) { - custom_search_engine_name_field.error = - String.format( - resources.getString( - R.string.search_add_custom_engine_error_existing_name - ), name - ) - return@launch - } - requireComponents.search.provider.reload() + val searchEngine = createSearchEngine( + name, + searchString.toSearchUrl(), + requireComponents.core.icons.loadIcon(IconRequest(searchString)).await().bitmap + ) + + requireComponents.useCases.searchUseCases.addSearchEngine(searchEngine) + val successMessage = resources .getString(R.string.search_add_custom_engine_success_message, name) @@ -196,27 +196,12 @@ class AddSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine), } fun checkForErrors(name: String, searchString: String): Boolean { - val existingIdentifiers = requireComponents - .search - .provider - .allSearchEngineIdentifiers() - .map { it.toLowerCase(Locale.ROOT) } - - val hasError = when { + return when { name.isEmpty() -> { custom_search_engine_name_field.error = resources .getString(R.string.search_add_custom_engine_error_empty_name) true } - existingIdentifiers.contains(name.toLowerCase(Locale.ROOT)) -> { - custom_search_engine_name_field.error = - String.format( - resources.getString( - R.string.search_add_custom_engine_error_existing_name - ), name - ) - true - } searchString.isEmpty() -> { custom_search_engine_search_string_field.error = resources.getString(R.string.search_add_custom_engine_error_empty_search_string) @@ -229,21 +214,6 @@ class AddSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine), } else -> false } - - return hasError - } - - private fun installEngine(engine: SearchEngine) { - viewLifecycleOwner.lifecycleScope.launch(Main) { - withContext(IO) { - requireContext().components.search.provider.installSearchEngine( - requireContext(), - engine - ) - } - - findNavController().popBackStack() - } } override fun onCheckedChanged(buttonView: CompoundButton, isChecked: Boolean) { @@ -303,3 +273,7 @@ class AddSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine), private const val FIRST_INDEX = 0 } } + +private fun String.toSearchUrl(): String { + return replace("%s", "{searchTerms}") +} diff --git a/app/src/main/java/org/mozilla/fenix/settings/search/EditCustomSearchEngineFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/search/EditCustomSearchEngineFragment.kt index 677bd349a..bc70833fe 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/search/EditCustomSearchEngineFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/search/EditCustomSearchEngineFragment.kt @@ -4,7 +4,6 @@ package org.mozilla.fenix.settings.search -import android.net.Uri import android.os.Bundle import android.view.Menu import android.view.MenuInflater @@ -14,21 +13,23 @@ import androidx.fragment.app.Fragment import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs -import kotlinx.android.synthetic.main.custom_search_engine.* +import kotlinx.android.synthetic.main.custom_search_engine.custom_search_engine_name_field +import kotlinx.android.synthetic.main.custom_search_engine.custom_search_engine_search_string_field +import kotlinx.android.synthetic.main.custom_search_engine.custom_search_engines_learn_more +import kotlinx.android.synthetic.main.custom_search_engine.edit_engine_name +import kotlinx.android.synthetic.main.custom_search_engine.edit_search_string import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -import mozilla.components.browser.search.SearchEngine +import mozilla.components.browser.state.search.SearchEngine import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar -import org.mozilla.fenix.components.searchengine.CustomSearchEngineStore import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.showToolbar import org.mozilla.fenix.settings.SupportUtils -import java.util.Locale /** * Fragment to enter a custom search engine name and URL template. @@ -41,17 +42,21 @@ class EditCustomSearchEngineFragment : Fragment(R.layout.fragment_add_search_eng override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) setHasOptionsMenu(true) - searchEngine = CustomSearchEngineStore.loadCustomSearchEngines(requireContext()).first { - it.identifier == args.searchEngineIdentifier - } + + searchEngine = requireNotNull( + requireComponents.core.store.state.search.customSearchEngines.find { engine -> + engine.id == args.searchEngineIdentifier + } + ) } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) + val url = searchEngine.resultUrls[0] + edit_engine_name.setText(searchEngine.name) - val decodedUrl = Uri.decode(searchEngine.buildSearchUrl("%s")) - edit_search_string.setText(decodedUrl) + edit_search_string.setText(url.toEditableUrl()) custom_search_engines_learn_more.setOnClickListener { (activity as HomeActivity).openToBrowserAndLoad( @@ -92,9 +97,7 @@ class EditCustomSearchEngineFragment : Fragment(R.layout.fragment_add_search_eng val name = edit_engine_name.text?.toString()?.trim() ?: "" val searchString = edit_search_string.text?.toString() ?: "" - val hasError = checkForErrors(name, searchString) - - if (hasError) { + if (checkForErrors(name, searchString)) { return } @@ -111,14 +114,15 @@ class EditCustomSearchEngineFragment : Fragment(R.layout.fragment_add_search_eng custom_search_engine_search_string_field.error = resources .getString(R.string.search_add_custom_engine_error_cannot_reach, name) } + SearchStringValidator.Result.Success -> { - CustomSearchEngineStore.updateSearchEngine( - context = requireContext(), - oldEngineName = args.searchEngineIdentifier, - newEngineName = name, - searchQuery = searchString + val update = searchEngine.copy( + name = name, + resultUrls = listOf(searchString.toSearchUrl()) ) - requireComponents.search.provider.reload() + + requireComponents.useCases.searchUseCases.addSearchEngine(update) + val successMessage = resources .getString(R.string.search_edit_custom_engine_success_message, name) @@ -131,9 +135,7 @@ class EditCustomSearchEngineFragment : Fragment(R.layout.fragment_add_search_eng .setText(successMessage) .show() } - if (args.isDefaultSearchEngine) { - requireComponents.search.provider.setDefaultEngine(requireContext(), name) - } + findNavController().popBackStack() } } @@ -141,28 +143,12 @@ class EditCustomSearchEngineFragment : Fragment(R.layout.fragment_add_search_eng } private fun checkForErrors(name: String, searchString: String): Boolean { - val existingIdentifiers = requireComponents - .search - .provider - .allSearchEngineIdentifiers() - .map { it.toLowerCase(Locale.ROOT) } - - val nameHasChanged = name != args.searchEngineIdentifier - val hasError = when { + return when { name.isEmpty() -> { custom_search_engine_name_field.error = resources .getString(R.string.search_add_custom_engine_error_empty_name) true } - existingIdentifiers.contains(name.toLowerCase(Locale.ROOT)) && nameHasChanged -> { - custom_search_engine_name_field.error = - String.format( - resources.getString( - R.string.search_add_custom_engine_error_existing_name - ), name - ) - true - } searchString.isEmpty() -> { custom_search_engine_search_string_field.error = resources.getString(R.string.search_add_custom_engine_error_empty_search_string) @@ -175,6 +161,13 @@ class EditCustomSearchEngineFragment : Fragment(R.layout.fragment_add_search_eng } else -> false } - return hasError } } + +private fun String.toEditableUrl(): String { + return replace("{searchTerms}", "%s") +} + +private fun String.toSearchUrl(): String { + return replace("%s", "{searchTerms}") +} diff --git a/app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt b/app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt index 107b908dc..caea80564 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt @@ -5,27 +5,165 @@ package org.mozilla.fenix.settings.search import android.content.Context +import android.content.res.Resources +import android.graphics.drawable.BitmapDrawable import android.util.AttributeSet +import android.view.LayoutInflater +import android.view.View +import android.view.ViewGroup import android.widget.CompoundButton -import mozilla.components.browser.search.SearchEngine +import android.widget.LinearLayout +import android.widget.RadioGroup +import androidx.core.view.isVisible +import androidx.navigation.Navigation +import androidx.preference.Preference +import androidx.preference.PreferenceViewHolder +import kotlinx.android.synthetic.main.search_engine_radio_button.view.engine_icon +import kotlinx.android.synthetic.main.search_engine_radio_button.view.engine_text +import kotlinx.android.synthetic.main.search_engine_radio_button.view.overflow_menu +import kotlinx.android.synthetic.main.search_engine_radio_button.view.radio_button +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.MainScope +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.launch +import mozilla.components.browser.state.search.SearchEngine +import mozilla.components.browser.state.state.SearchState +import mozilla.components.browser.state.state.searchEngines +import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.lib.state.ext.flow +import mozilla.components.support.ktx.android.view.toScope +import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged import org.mozilla.fenix.R import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.settings +import org.mozilla.fenix.ext.getRootView +import org.mozilla.fenix.utils.allowUndo class RadioSearchEngineListPreference @JvmOverloads constructor( context: Context, attrs: AttributeSet? = null, defStyleAttr: Int = android.R.attr.preferenceStyle -) : SearchEngineListPreference(context, attrs, defStyleAttr) { - override val itemResId: Int +) : Preference(context, attrs, defStyleAttr), CompoundButton.OnCheckedChangeListener { + val itemResId: Int get() = R.layout.search_engine_radio_button - override fun updateDefaultItem(defaultButton: CompoundButton) { - defaultButton.isChecked = true + init { + layoutResource = R.layout.preference_search_engine_chooser } - override fun onSearchEngineSelected(searchEngine: SearchEngine) { - context.components.search.provider.setDefaultEngine(context, searchEngine.identifier) - context.settings().defaultSearchEngineName = searchEngine.name + override fun onBindViewHolder(holder: PreferenceViewHolder) { + super.onBindViewHolder(holder) + + subscribeToSearchEngineUpdates( + context.components.core.store, + holder.itemView + ) + } + + @OptIn(ExperimentalCoroutinesApi::class) + private fun subscribeToSearchEngineUpdates(store: BrowserStore, view: View) = view.toScope().launch { + store.flow() + .map { state -> state.search } + .ifChanged() + .collect { state -> refreshSearchEngineViews(view, state) } + } + + private fun refreshSearchEngineViews(view: View, state: SearchState) { + val searchEngineGroup = view.findViewById(R.id.search_engine_group) + searchEngineGroup!!.removeAllViews() + + val layoutInflater = LayoutInflater.from(context) + val layoutParams = ViewGroup.LayoutParams( + ViewGroup.LayoutParams.MATCH_PARENT, + ViewGroup.LayoutParams.WRAP_CONTENT + ) + + state.searchEngines.forEach { engine -> + val searchEngineView = makeButtonFromSearchEngine( + engine = engine, + layoutInflater = layoutInflater, + res = context.resources, + allowDeletion = state.searchEngines.size > 1, + isSelected = engine == state.selectedOrDefaultSearchEngine + ) + + searchEngineGroup.addView(searchEngineView, layoutParams) + } + } + + private fun makeButtonFromSearchEngine( + engine: SearchEngine, + layoutInflater: LayoutInflater, + res: Resources, + allowDeletion: Boolean, + isSelected: Boolean + ): View { + val isCustomSearchEngine = engine.type == SearchEngine.Type.CUSTOM + + val wrapper = layoutInflater.inflate(itemResId, null) as LinearLayout + wrapper.setOnClickListener { wrapper.radio_button.isChecked = true } + wrapper.radio_button.tag = engine.id + wrapper.radio_button.isChecked = isSelected + wrapper.radio_button.setOnCheckedChangeListener(this) + wrapper.engine_text.text = engine.name + wrapper.overflow_menu.isVisible = allowDeletion || isCustomSearchEngine + wrapper.overflow_menu.setOnClickListener { + SearchEngineMenu( + context = context, + allowDeletion = allowDeletion, + isCustomSearchEngine = isCustomSearchEngine, + onItemTapped = { + when (it) { + is SearchEngineMenu.Item.Edit -> editCustomSearchEngine(wrapper, engine) + is SearchEngineMenu.Item.Delete -> deleteSearchEngine( + context, + engine + ) + } + } + ).menuBuilder.build(context).show(wrapper.overflow_menu) + } + val iconSize = res.getDimension(R.dimen.preference_icon_drawable_size).toInt() + val engineIcon = BitmapDrawable(res, engine.icon) + engineIcon.setBounds(0, 0, iconSize, iconSize) + wrapper.engine_icon.setImageDrawable(engineIcon) + return wrapper + } + + override fun onCheckedChanged(buttonView: CompoundButton, isChecked: Boolean) { + val searchEngineId = buttonView.tag.toString() + val engine = requireNotNull( + context.components.core.store.state.search.searchEngines.find { searchEngine -> + searchEngine.id == searchEngineId + } + ) + + context.components.useCases.searchUseCases.selectSearchEngine(engine) + } + + private fun editCustomSearchEngine(view: View, engine: SearchEngine) { + val directions = SearchEngineFragmentDirections + .actionSearchEngineFragmentToEditCustomSearchEngineFragment(engine.id) + + Navigation.findNavController(view).navigate(directions) + } + + private fun deleteSearchEngine( + context: Context, + engine: SearchEngine + ) { + context.components.useCases.searchUseCases.removeSearchEngine(engine) + + MainScope().allowUndo( + view = context.getRootView()!!, + message = context + .getString(R.string.search_delete_search_engine_success_message, engine.name), + undoActionTitle = context.getString(R.string.snackbar_deleted_undo), + onCancel = { + context.components.useCases.searchUseCases.addSearchEngine(engine) + }, + operation = {} + ) } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineFragment.kt index 05b3ef201..c5971e7f9 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineFragment.kt @@ -70,15 +70,11 @@ class SearchEngineFragment : PreferenceFragmentCompat() { isChecked = context.settings().shouldShowClipboardSuggestions } - val searchEngineListPreference = - requirePreference(R.string.pref_key_search_engine_list) - val showVoiceSearchPreference = requirePreference(R.string.pref_key_show_voice_search).apply { isChecked = context.settings().shouldShowVoiceSearch } - searchEngineListPreference.reload(requireContext()) searchSuggestionsPreference.onPreferenceChangeListener = SharedPreferenceUpdater() showSearchShortcuts.onPreferenceChangeListener = SharedPreferenceUpdater() showHistorySuggestions.onPreferenceChangeListener = SharedPreferenceUpdater() diff --git a/app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineListPreference.kt b/app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineListPreference.kt deleted file mode 100644 index c01c305d0..000000000 --- a/app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineListPreference.kt +++ /dev/null @@ -1,242 +0,0 @@ -/* 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/. */ - -package org.mozilla.fenix.settings.search - -import android.content.Context -import android.content.res.Resources -import android.graphics.drawable.BitmapDrawable -import android.util.AttributeSet -import android.view.LayoutInflater -import android.view.View -import android.view.ViewGroup -import android.widget.CompoundButton -import android.widget.LinearLayout -import android.widget.RadioGroup -import androidx.core.view.isVisible -import androidx.navigation.Navigation -import androidx.preference.Preference -import androidx.preference.PreferenceViewHolder -import kotlinx.android.synthetic.main.search_engine_radio_button.view.engine_icon -import kotlinx.android.synthetic.main.search_engine_radio_button.view.engine_text -import kotlinx.android.synthetic.main.search_engine_radio_button.view.overflow_menu -import kotlinx.android.synthetic.main.search_engine_radio_button.view.radio_button -import kotlinx.coroutines.MainScope -import mozilla.components.browser.search.SearchEngine -import mozilla.components.browser.search.provider.SearchEngineList -import org.mozilla.fenix.R -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.components.searchengine.CustomSearchEngineStore -import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.getRootView -import org.mozilla.fenix.ext.settings -import org.mozilla.fenix.utils.allowUndo -import java.util.Locale - -abstract class SearchEngineListPreference @JvmOverloads constructor( - context: Context, - attrs: AttributeSet? = null, - defStyleAttr: Int = android.R.attr.preferenceStyle -) : Preference(context, attrs, defStyleAttr), CompoundButton.OnCheckedChangeListener { - - protected lateinit var searchEngineList: SearchEngineList - protected var searchEngineGroup: RadioGroup? = null - - protected abstract val itemResId: Int - - init { - layoutResource = R.layout.preference_search_engine_chooser - } - - override fun onBindViewHolder(holder: PreferenceViewHolder?) { - super.onBindViewHolder(holder) - searchEngineGroup = holder!!.itemView.findViewById(R.id.search_engine_group) - reload(searchEngineGroup!!.context) - } - - fun reload(context: Context) { - searchEngineList = context.components.search.provider.installedSearchEngines(context) - refreshSearchEngineViews(context) - } - - protected abstract fun onSearchEngineSelected(searchEngine: SearchEngine) - protected abstract fun updateDefaultItem(defaultButton: CompoundButton) - - private fun refreshSearchEngineViews(context: Context) { - if (searchEngineGroup == null) { - // We want to refresh the search engine list of this preference in onResume, - // but the first time this preference is created onResume is called before onCreateView - // so searchEngineGroup is not set yet. - return - } - - val defaultEngineId = context.components.search.provider.getDefaultEngine(context).identifier - - val selectedEngine = (searchEngineList.list.find { - it.identifier == defaultEngineId - } ?: searchEngineList.list.first()).identifier - - // set the search engine manager default - context.components.search.provider.setDefaultEngine(context, selectedEngine) - - searchEngineGroup!!.removeAllViews() - - val layoutInflater = LayoutInflater.from(context) - val layoutParams = ViewGroup.LayoutParams( - ViewGroup.LayoutParams.MATCH_PARENT, - ViewGroup.LayoutParams.WRAP_CONTENT - ) - - val setupSearchEngineItem: (Int, SearchEngine) -> Unit = { index, engine -> - val engineId = engine.identifier - val engineItem = makeButtonFromSearchEngine( - engine = engine, - layoutInflater = layoutInflater, - res = context.resources, - allowDeletion = searchEngineList.list.size > 1 - ) - - engineItem.id = index + (searchEngineList.default?.let { 1 } ?: 0) - engineItem.tag = engineId - if (engineId == selectedEngine) { - updateDefaultItem(engineItem.radio_button) - /* #11465 -> radio_button.isChecked = true does not trigger - * onSearchEngineSelected because searchEngineGroup has null views at that point. - * So we trigger it here.*/ - onSearchEngineSelected(engine) - } - searchEngineGroup!!.addView(engineItem, layoutParams) - } - - searchEngineList.default?.apply { - setupSearchEngineItem(0, this) - } - - searchEngineList.list - .filter { it.identifier != searchEngineList.default?.identifier } - .sortedBy { it.name.toLowerCase(Locale.getDefault()) } - .forEachIndexed(setupSearchEngineItem) - } - - private fun makeButtonFromSearchEngine( - engine: SearchEngine, - layoutInflater: LayoutInflater, - res: Resources, - allowDeletion: Boolean - ): View { - val isCustomSearchEngine = - CustomSearchEngineStore.isCustomSearchEngine(context, engine.identifier) - - val wrapper = layoutInflater.inflate(itemResId, null) as LinearLayout - wrapper.setOnClickListener { wrapper.radio_button.isChecked = true } - wrapper.radio_button.setOnCheckedChangeListener(this) - wrapper.engine_text.text = engine.name - wrapper.overflow_menu.isVisible = allowDeletion || isCustomSearchEngine - wrapper.overflow_menu.setOnClickListener { - SearchEngineMenu( - context = context, - allowDeletion = allowDeletion, - isCustomSearchEngine = isCustomSearchEngine, - onItemTapped = { - when (it) { - is SearchEngineMenu.Item.Edit -> editCustomSearchEngine(engine) - is SearchEngineMenu.Item.Delete -> deleteSearchEngine( - context, - engine, - isCustomSearchEngine - ) - } - } - ).menuBuilder.build(context).show(wrapper.overflow_menu) - } - val iconSize = res.getDimension(R.dimen.preference_icon_drawable_size).toInt() - val engineIcon = BitmapDrawable(res, engine.icon) - engineIcon.setBounds(0, 0, iconSize, iconSize) - wrapper.engine_icon.setImageDrawable(engineIcon) - return wrapper - } - - override fun onCheckedChanged(buttonView: CompoundButton, isChecked: Boolean) { - searchEngineList.list.forEach { engine -> - val wrapper: LinearLayout = - searchEngineGroup?.findViewWithTag(engine.identifier) ?: return - - when (wrapper.radio_button == buttonView) { - true -> onSearchEngineSelected(engine) - false -> { - wrapper.radio_button.setOnCheckedChangeListener(null) - wrapper.radio_button.isChecked = false - wrapper.radio_button.setOnCheckedChangeListener(this) - } - } - } - } - - private fun editCustomSearchEngine(engine: SearchEngine) { - val wasDefault = context.components.search.provider.getDefaultEngine(context).identifier == engine.identifier - val directions = SearchEngineFragmentDirections - .actionSearchEngineFragmentToEditCustomSearchEngineFragment(engine.identifier, wasDefault) - Navigation.findNavController(searchEngineGroup!!).navigate(directions) - } - - private fun deleteSearchEngine( - context: Context, - engine: SearchEngine, - isCustomSearchEngine: Boolean - ) { - val isDefaultEngine = engine == context.components.search.provider.getDefaultEngine(context) - val initialEngineList = searchEngineList.copy() - val initialDefaultEngine = searchEngineList.default - - context.components.search.provider.uninstallSearchEngine( - context, - engine, - isCustomSearchEngine - ) - - MainScope().allowUndo( - view = context.getRootView()!!, - message = context - .getString(R.string.search_delete_search_engine_success_message, engine.name), - undoActionTitle = context.getString(R.string.snackbar_deleted_undo), - onCancel = { - context.components.search.provider.installSearchEngine( - context, - engine, - isCustomSearchEngine - ) - - searchEngineList = initialEngineList.copy( - default = initialDefaultEngine - ) - - refreshSearchEngineViews(context) - }, - operation = { - if (isDefaultEngine) { - val default = context.components.search.provider.getDefaultEngine(context) - context.components.search.provider.setDefaultEngine(context, default.identifier) - context.settings().defaultSearchEngineName = default.name - } - if (isCustomSearchEngine) { - context.components.analytics.metrics.track(Event.CustomEngineDeleted) - } - refreshSearchEngineViews(context) - } - ) - - searchEngineList = searchEngineList.copy( - list = searchEngineList.list.filter { - it.identifier != engine.identifier - }, - default = if (searchEngineList.default?.identifier == engine.identifier) { - null - } else { - searchEngineList.default - } - ) - - refreshSearchEngineViews(context) - } -} diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml index 9a6cf4ee5..2fca420f1 100644 --- a/app/src/main/res/navigation/nav_graph.xml +++ b/app/src/main/res/navigation/nav_graph.xml @@ -943,9 +943,6 @@ - diff --git a/app/src/migration/java/org/mozilla/fenix/MigratingFenixApplication.kt b/app/src/migration/java/org/mozilla/fenix/MigratingFenixApplication.kt index 9c51b44ef..d64043f14 100644 --- a/app/src/migration/java/org/mozilla/fenix/MigratingFenixApplication.kt +++ b/app/src/migration/java/org/mozilla/fenix/MigratingFenixApplication.kt @@ -38,7 +38,6 @@ class MigratingFenixApplication : FenixApplication() { this.components.addonUpdater ) .migrateTelemetryIdentifiers() - .migrateSearchEngine(this.components.search.searchEngineManager) .build() } diff --git a/app/src/test/java/org/mozilla/fenix/components/TestComponents.kt b/app/src/test/java/org/mozilla/fenix/components/TestComponents.kt index 511a24e81..577790914 100644 --- a/app/src/test/java/org/mozilla/fenix/components/TestComponents.kt +++ b/app/src/test/java/org/mozilla/fenix/components/TestComponents.kt @@ -16,14 +16,12 @@ class TestComponents(private val context: Context) : Components(context) { } override val services by lazy { Services(context, backgroundServices.accountManager) } override val core by lazy { TestCore(context, analytics.crashReporter) } - override val search by lazy { Search(context) } override val useCases by lazy { UseCases( context, core.engine, core.sessionManager, core.store, - search.searchEngineManager, core.webAppShortcutManager, core.topSitesStorage ) diff --git a/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt b/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt index 5645a2eb0..16e932db2 100644 --- a/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.components.metrics import io.mockk.MockKAnnotations import io.mockk.every import io.mockk.impl.annotations.MockK +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.service.glean.testing.GleanTestRule import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals @@ -35,7 +36,9 @@ class GleanMetricsServiceTest { @Before fun setup() { MockKAnnotations.init(this) - gleanService = GleanMetricsService(testContext, browsersCache, mozillaProductDetector) + + val store = BrowserStore() + gleanService = GleanMetricsService(testContext, lazy { store }, browsersCache, mozillaProductDetector) } @Test diff --git a/app/src/test/java/org/mozilla/fenix/components/metrics/MetricsUtilsTestRoboelectric.kt b/app/src/test/java/org/mozilla/fenix/components/metrics/MetricsUtilsTestRoboelectric.kt index 059929f86..2f867021a 100644 --- a/app/src/test/java/org/mozilla/fenix/components/metrics/MetricsUtilsTestRoboelectric.kt +++ b/app/src/test/java/org/mozilla/fenix/components/metrics/MetricsUtilsTestRoboelectric.kt @@ -6,8 +6,8 @@ package org.mozilla.fenix.components.metrics import io.mockk.every import io.mockk.mockk -import mozilla.components.browser.search.SearchEngine -import mozilla.components.support.test.robolectric.testContext +import mozilla.components.browser.state.search.SearchEngine +import mozilla.components.browser.state.store.BrowserStore import org.junit.Assert import org.junit.Test import org.junit.runner.RunWith @@ -22,16 +22,16 @@ class MetricsUtilsTestRoboelectric { @Test fun createSearchEvent() { - val context = testContext + val store = BrowserStore() val engine: SearchEngine = mockk(relaxed = true) - every { engine.identifier } returns MetricsUtilsTest.ENGINE_SOURCE_IDENTIFIER + every { engine.id } returns MetricsUtilsTest.ENGINE_SOURCE_IDENTIFIER Assert.assertEquals( "${MetricsUtilsTest.ENGINE_SOURCE_IDENTIFIER}.suggestion", MetricsUtils.createSearchEvent( engine, - context, + store, Event.PerformedSearch.SearchAccessPoint.SUGGESTION )?.eventSource?.countLabel ) @@ -39,7 +39,7 @@ class MetricsUtilsTestRoboelectric { "${MetricsUtilsTest.ENGINE_SOURCE_IDENTIFIER}.action", MetricsUtils.createSearchEvent( engine, - context, + store, Event.PerformedSearch.SearchAccessPoint.ACTION )?.eventSource?.countLabel ) @@ -47,7 +47,7 @@ class MetricsUtilsTestRoboelectric { "${MetricsUtilsTest.ENGINE_SOURCE_IDENTIFIER}.widget", MetricsUtils.createSearchEvent( engine, - context, + store, Event.PerformedSearch.SearchAccessPoint.WIDGET )?.eventSource?.countLabel ) @@ -55,7 +55,7 @@ class MetricsUtilsTestRoboelectric { "${MetricsUtilsTest.ENGINE_SOURCE_IDENTIFIER}.shortcut", MetricsUtils.createSearchEvent( engine, - context, + store, Event.PerformedSearch.SearchAccessPoint.SHORTCUT )?.eventSource?.countLabel ) diff --git a/app/src/test/java/org/mozilla/fenix/components/metrics/PerformedSearchTest.kt b/app/src/test/java/org/mozilla/fenix/components/metrics/PerformedSearchTest.kt deleted file mode 100644 index 56c47c64f..000000000 --- a/app/src/test/java/org/mozilla/fenix/components/metrics/PerformedSearchTest.kt +++ /dev/null @@ -1,207 +0,0 @@ -/* 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/. */ - -package org.mozilla.fenix.components.metrics - -import mozilla.components.browser.search.SearchEngine -import mozilla.components.browser.search.SearchEngineManager -import mozilla.components.browser.search.provider.AssetsSearchEngineProvider -import mozilla.components.browser.search.provider.localization.LocaleSearchLocalizationProvider -import mozilla.components.support.test.robolectric.testContext -import org.junit.Assert.assertTrue -import org.junit.Before -import org.junit.Test -import org.junit.runner.RunWith -import org.mozilla.fenix.components.metrics.Event.PerformedSearch -import org.mozilla.fenix.components.metrics.Event.PerformedSearch.EngineSource -import org.mozilla.fenix.components.metrics.Event.PerformedSearch.EventSource -import org.mozilla.fenix.helpers.FenixRobolectricTestRunner - -@RunWith(FenixRobolectricTestRunner::class) -class PerformedSearchTest { - private lateinit var searchEngines: List - - // Match against the Regex defined at - // https://github.com/mozilla-mobile/android-components/blob/master/components/service/glean/src/main/java/mozilla/components/service/glean/private/LabeledMetricType.kt#L43 - // We're temporarily using it until better Glean testing APIs are available. - private val countLabelRegex = Regex("^[a-z_][a-z0-9_-]{0,29}(\\.[a-z0-9_-]{0,29})*$") - - @Before - fun setUp() { - searchEngines = SearchEngineManager(listOf(provider)).getSearchEngines(testContext) - } - - @Test - fun testThatCountLabelIsValid() { - val labels = searchEngines.map { - PerformedSearch(EventSource.Action(EngineSource.Shortcut(it, false))).eventSource.countLabel - } - - labels.forEach { - assertTrue("$it does not match!", it.matches(countLabelRegex)) - } - } - - private val provider = AssetsSearchEngineProvider( - localizationProvider = LocaleSearchLocalizationProvider(), - additionalIdentifiers = listOf( - "amazon-au", - "amazon-br", - "amazon-ca", - "amazon-co-uk", - "amazon-de", - "amazon-fr", - "amazon-in", - "amazon-it", - "amazon-jp", - "amazon-mx", - "amazon-nl", - "amazondotcom", - "azerdict", - "azet-sk", - "baidu", - "bing", - "bolcom-fy-NL", - "bolcom-nl", - "ceneje", - "coccoc", - "danawa-kr", - "daum-kr", - "ddg", - "diec2", - "drae", - "duckduckgo", - "elebila", - "faclair-beag", - "google-2018", - "google-b-1-m", - "google-b-m", - "google", - "gulesider-mobile-NO", - "heureka-cz", - "hotline-ua", - "leit-is", - "leo_ende_de", - "list-am", - "mapy-cz", - "mercadolibre-ar", - "mercadolibre-cl", - "mercadolibre-mx", - "naver-kr", - "odpiralni", - "pazaruvaj", - "pledarigrond", - "prisjakt-sv-SE", - "qwant", - "rediff", - "reta-vortaro", - "salidzinilv", - "seznam-cz", - "skroutz", - "slovnik-sk", - "sslv", - "sztaki-en-hu", - "taobao", - "tearma", - "twitter-ja", - "twitter", - "vatera", - "wikipedia-NN", - "wikipedia-NO", - "wikipedia-an", - "wikipedia-ar", - "wikipedia-as", - "wikipedia-ast", - "wikipedia-az", - "wikipedia-be", - "wikipedia-bg", - "wikipedia-bn", - "wikipedia-br", - "wikipedia-bs", - "wikipedia-ca", - "wikipedia-cy", - "wikipedia-cz", - "wikipedia-da", - "wikipedia-de", - "wikipedia-dsb", - "wikipedia-el", - "wikipedia-eo", - "wikipedia-es", - "wikipedia-et", - "wikipedia-eu", - "wikipedia-fa", - "wikipedia-fi", - "wikipedia-fr", - "wikipedia-fy-NL", - "wikipedia-ga-IE", - "wikipedia-gd", - "wikipedia-gl", - "wikipedia-gn", - "wikipedia-gu", - "wikipedia-he", - "wikipedia-hi", - "wikipedia-hr", - "wikipedia-hsb", - "wikipedia-hu", - "wikipedia-hy-AM", - "wikipedia-ia", - "wikipedia-id", - "wikipedia-is", - "wikipedia-it", - "wikipedia-ja", - "wikipedia-ka", - "wikipedia-kab", - "wikipedia-kk", - "wikipedia-km", - "wikipedia-kn", - "wikipedia-lij", - "wikipedia-lo", - "wikipedia-lt", - "wikipedia-ltg", - "wikipedia-lv", - "wikipedia-ml", - "wikipedia-mr", - "wikipedia-ms", - "wikipedia-my", - "wikipedia-ne", - "wikipedia-nl", - "wikipedia-oc", - "wikipedia-or", - "wikipedia-pa", - "wikipedia-pl", - "wikipedia-pt", - "wikipedia-rm", - "wikipedia-ro", - "wikipedia-ru", - "wikipedia-sk", - "wikipedia-sl", - "wikipedia-sq", - "wikipedia-sr", - "wikipedia-sv-SE", - "wikipedia-ta", - "wikipedia-te", - "wikipedia-th", - "wikipedia-tr", - "wikipedia-uk", - "wikipedia-ur", - "wikipedia-uz", - "wikipedia-vi", - "wikipedia-wo", - "wikipedia-zh-CN", - "wikipedia-zh-TW", - "wikipedia", - "wiktionary-kn", - "wiktionary-oc", - "wiktionary-or", - "wiktionary-ta", - "wiktionary-te", - "yahoo-jp", - "yandex-en", - "yandex-ru", - "yandex-tr", - "yandex.by", - "yandex" - ) - ) -} diff --git a/app/src/test/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProviderTest.kt b/app/src/test/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProviderTest.kt deleted file mode 100644 index ecc3c40d8..000000000 --- a/app/src/test/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProviderTest.kt +++ /dev/null @@ -1,174 +0,0 @@ -package org.mozilla.fenix.components.searchengine - -import android.content.Context -import io.mockk.Runs -import io.mockk.coEvery -import io.mockk.coVerify -import io.mockk.every -import io.mockk.just -import io.mockk.mockk -import io.mockk.mockkObject -import kotlinx.coroutines.CompletableDeferred -import kotlinx.coroutines.Deferred -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.test.runBlockingTest -import mozilla.components.browser.search.SearchEngine -import mozilla.components.browser.search.provider.SearchEngineList -import mozilla.components.browser.search.provider.localization.LocaleSearchLocalizationProvider -import mozilla.components.browser.search.provider.localization.SearchLocalizationProvider -import mozilla.components.support.test.robolectric.testContext -import org.junit.Assert.assertEquals -import org.junit.Before -import org.junit.Test -import org.junit.runner.RunWith -import org.mozilla.fenix.helpers.FenixRobolectricTestRunner - -@ExperimentalCoroutinesApi -@RunWith(FenixRobolectricTestRunner::class) -class FenixSearchEngineProviderTest { - - private lateinit var fenixSearchEngineProvider: FenixSearchEngineProvider - - @Before - fun before() { - fenixSearchEngineProvider = FakeFenixSearchEngineProvider(testContext) - mockkObject(CustomSearchEngineStore) - fenixSearchEngineProvider.let { - every { CustomSearchEngineStore.loadCustomSearchEngines(testContext) } returns listOf( - (it as FakeFenixSearchEngineProvider) - .mockSearchEngine("my custom site", "my custom site") - ) - } - } - - /* - TODO TEST: - - public API happy path - - list ordering - - deduping - - the above after adding/removing - */ - - @Suppress("DEPRECATION") - @Test - fun `add custom engine`() = runBlockingTest { - val engineName = "Ecosia" - val engineQuery = "www.ecosia.com/%s" - val searchEngine: SearchEngine = mockk(relaxed = true) - every { searchEngine.getSearchTemplate() } returns engineQuery - every { searchEngine.name } returns engineName - mockkObject(CustomSearchEngineStore) - coEvery { - CustomSearchEngineStore.addSearchEngine( - testContext, - engineName, - engineQuery - ) - } just Runs - - fenixSearchEngineProvider.installSearchEngine(testContext, searchEngine, true) - - coVerify { CustomSearchEngineStore.addSearchEngine(testContext, engineName, engineQuery) } - } - - @Test - fun `GIVEN sharedprefs does not contain installed engines WHEN installedSearchEngineIdentifiers THEN defaultEngines + customEngines ids are returned`() = runBlockingTest { - val expectedDefaults = fenixSearchEngineProvider.baseSearchEngines.toIdSet() - val expectedCustom = fenixSearchEngineProvider.customSearchEngines.toIdSet() - val expected = expectedDefaults + expectedCustom - - val actual = fenixSearchEngineProvider.installedSearchEngineIdentifiers(testContext) - assertEquals(expected, actual) - } - - @Test - fun `GIVEN sharedprefs contains installed engines WHEN installedSearchEngineIdentifiers THEN defaultEngines + customEngines ids are returned`() = runBlockingTest { - val sp = testContext.getSharedPreferences( - FenixSearchEngineProvider.PREF_FILE_SEARCH_ENGINES, - Context.MODE_PRIVATE - ) - sp.edit().putStringSet( - fenixSearchEngineProvider.localeAwareInstalledEnginesKey(), - persistedInstalledEngines - ).apply() - - val expectedStored = persistedInstalledEngines - val expectedCustom = fenixSearchEngineProvider.customSearchEngines.toIdSet() - val expected = expectedStored + expectedCustom - - val actual = fenixSearchEngineProvider.installedSearchEngineIdentifiers(testContext) - assertEquals(expected, actual) - } -} - -private suspend fun Deferred.toIdSet() = - await().list.map { it.identifier }.toSet() - -private val persistedInstalledEngines = setOf("bing", "ecosia") - -class FakeFenixSearchEngineProvider(context: Context) : FenixSearchEngineProvider(context) { - override val localizationProvider: SearchLocalizationProvider - get() = LocaleSearchLocalizationProvider() - - override var baseSearchEngines: Deferred - set(_) { throw NotImplementedError("Setting not currently supported on this fake") } - get() { - val google = mockSearchEngine(id = "google-b-1-m", n = "Google") - - return CompletableDeferred( - SearchEngineList( - listOf( - google, - mockSearchEngine("bing", "Bing"), - mockSearchEngine("amazondotcom", "Amazon.com") - ), default = google - ) - ) - } - - override val fallbackEngines: Deferred - get() { - val google = mockSearchEngine(id = "google-b-1-m", n = "Google") - - return CompletableDeferred( - SearchEngineList( - listOf( - google, - mockSearchEngine("bing", "Bing"), - mockSearchEngine("amazondotcom", "Amazon.com") - ), default = google - ) - ) - } - - override val bundledSearchEngines = CompletableDeferred( - SearchEngineList( - listOf( - mockSearchEngine("ecosia", "Ecosia"), - mockSearchEngine("reddit", "Reddit"), - mockSearchEngine("startpage", "Startpage.com") - ), default = null - ) - ) - - override var customSearchEngines: Deferred = CompletableDeferred( - SearchEngineList( - listOf( - mockSearchEngine("my custom site", "my custom site") - ), default = null - ) - ) - - override fun updateBaseSearchEngines() { } - - fun mockSearchEngine( - id: String, - n: String = id - ): SearchEngine { - val engine = mockk() - every { engine.identifier } returns id - every { engine.name } returns n - every { engine.icon } returns mockk() - return engine - } -} diff --git a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt index 4041b6305..f0873c2e2 100644 --- a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt @@ -12,9 +12,11 @@ import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestCoroutineDispatcher import kotlinx.coroutines.test.TestCoroutineScope -import mozilla.components.browser.search.SearchEngine -import mozilla.components.browser.search.SearchEngineManager import mozilla.components.browser.session.SessionManager +import mozilla.components.browser.state.search.SearchEngine +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.SearchState +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.Engine import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.tabs.TabsUseCases @@ -33,7 +35,6 @@ import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.components.tips.Tip import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.searchEngineManager import org.mozilla.fenix.ext.settings import org.mozilla.fenix.home.sessioncontrol.DefaultSessionControlController import org.mozilla.fenix.settings.SupportUtils @@ -67,16 +68,27 @@ class DefaultSessionControlControllerTest { wasSwiped: Boolean, handleSwipedItemDeletionCancel: () -> Unit ) -> Unit = mockk(relaxed = true) - private val searchEngine = mockk(relaxed = true) - private val searchEngineManager = mockk(relaxed = true) private val settings: Settings = mockk(relaxed = true) private val analytics: Analytics = mockk(relaxed = true) private val scope = TestCoroutineScope() - + private val searchEngine = SearchEngine( + id = "test", + name = "Test Engine", + icon = mockk(relaxed = true), + type = SearchEngine.Type.BUNDLED, + resultUrls = listOf("https://example.org/?q={searchTerms}") + ) + private lateinit var store: BrowserStore private lateinit var controller: DefaultSessionControlController @Before fun setup() { + store = BrowserStore(BrowserState( + search = SearchState( + regionSearchEngines = listOf(searchEngine) + ) + )) + every { fragmentStore.state } returns HomeFragmentState( collections = emptyList(), expandedCollections = emptySet(), @@ -90,15 +102,13 @@ class DefaultSessionControlControllerTest { every { id } returns R.id.homeFragment } every { activity.components.settings } returns settings - every { activity.components.search.provider.getDefaultEngine(activity) } returns searchEngine every { activity.settings() } returns settings - every { activity.searchEngineManager } returns searchEngineManager - every { searchEngineManager.defaultSearchEngine } returns searchEngine every { activity.components.analytics } returns analytics every { analytics.metrics } returns metrics controller = DefaultSessionControlController( activity = activity, + store = store, settings = settings, engine = engine, metrics = metrics, diff --git a/app/src/test/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessorTest.kt b/app/src/test/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessorTest.kt index 7e58c1570..649c3e89e 100644 --- a/app/src/test/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessorTest.kt @@ -6,18 +6,21 @@ package org.mozilla.fenix.home.intent import android.content.Intent import androidx.navigation.NavController +import androidx.test.core.app.ApplicationProvider import io.mockk.Called import io.mockk.every import io.mockk.mockk import io.mockk.verify -import mozilla.components.browser.search.SearchEngine +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.SearchState +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.feature.search.ext.createSearchEngine import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.components.metrics.MetricController -import org.mozilla.fenix.ext.components import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.widget.VoiceSearchActivity.Companion.SPEECH_PROCESSING @@ -29,16 +32,32 @@ class SpeechProcessingIntentProcessorTest { private val out: Intent = mockk(relaxed = true) private val metrics: MetricController = mockk(relaxed = true) + private val searchEngine = createSearchEngine( + name = "Test", + url = "https://www.example.org/?q={searchTerms}", + icon = mockk() + ) + + private lateinit var store: BrowserStore + @Before fun setup() { - val searchEngine = mockk(relaxed = true) - every { activity.components.search.searchEngineManager.defaultSearchEngine } returns searchEngine - every { activity.components.search.provider.getDefaultEngine(activity) } returns searchEngine + val searchEngine = searchEngine + + store = BrowserStore(BrowserState( + search = SearchState( + customSearchEngines = listOf(searchEngine), + userSelectedSearchEngineId = searchEngine.id, + complete = true + ) + )) + + every { activity.applicationContext } returns ApplicationProvider.getApplicationContext() } @Test fun `do not process blank intents`() { - val processor = SpeechProcessingIntentProcessor(activity, metrics) + val processor = SpeechProcessingIntentProcessor(activity, store, metrics) processor.process(Intent(), navController, out) verify { activity wasNot Called } @@ -52,7 +71,7 @@ class SpeechProcessingIntentProcessorTest { val intent = Intent().apply { putExtra(HomeActivity.OPEN_TO_BROWSER_AND_LOAD, false) } - val processor = SpeechProcessingIntentProcessor(activity, metrics) + val processor = SpeechProcessingIntentProcessor(activity, store, metrics) processor.process(intent, navController, out) verify { activity wasNot Called } @@ -61,35 +80,14 @@ class SpeechProcessingIntentProcessorTest { verify { metrics wasNot Called } } - @Test - fun `process when open extra is true`() { - val intent = Intent().apply { - putExtra(HomeActivity.OPEN_TO_BROWSER_AND_LOAD, true) - } - val processor = SpeechProcessingIntentProcessor(activity, metrics) - - processor.process(intent, navController, out) - - verify { - activity.openToBrowserAndLoad( - searchTermOrURL = "", - newTab = true, - from = BrowserDirection.FromGlobal, - forceSearch = true - ) - } - verify { navController wasNot Called } - verify { out.putExtra(HomeActivity.OPEN_TO_BROWSER_AND_LOAD, false) } - } - @Test fun `reads the speech processing extra`() { val intent = Intent().apply { putExtra(HomeActivity.OPEN_TO_BROWSER_AND_LOAD, true) putExtra(SPEECH_PROCESSING, "hello world") } - val processor = SpeechProcessingIntentProcessor(activity, metrics) + val processor = SpeechProcessingIntentProcessor(activity, store, metrics) processor.process(intent, mockk(), mockk(relaxed = true)) verify { @@ -97,7 +95,8 @@ class SpeechProcessingIntentProcessorTest { searchTermOrURL = "hello world", newTab = true, from = BrowserDirection.FromGlobal, - forceSearch = true + forceSearch = true, + engine = searchEngine ) } } diff --git a/app/src/test/java/org/mozilla/fenix/search/SearchDialogControllerTest.kt b/app/src/test/java/org/mozilla/fenix/search/SearchDialogControllerTest.kt index 7c888d283..8155cc043 100644 --- a/app/src/test/java/org/mozilla/fenix/search/SearchDialogControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/SearchDialogControllerTest.kt @@ -19,9 +19,10 @@ import io.mockk.unmockkObject import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runBlockingTest -import mozilla.components.browser.search.SearchEngine import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager +import mozilla.components.browser.state.search.SearchEngine +import mozilla.components.browser.state.store.BrowserStore import org.junit.After import org.junit.Before import org.junit.Test @@ -56,18 +57,21 @@ class SearchDialogControllerTest { MockKAnnotations.init(this) mockkObject(MetricsUtils) + val browserStore = BrowserStore() + every { store.state.tabId } returns "test-tab-id" every { store.state.searchEngineSource.searchEngine } returns searchEngine every { sessionManager.select(any()) } just Runs every { navController.currentDestination } returns mockk { every { id } returns R.id.searchDialogFragment } - every { MetricsUtils.createSearchEvent(searchEngine, activity, any()) } returns null + every { MetricsUtils.createSearchEvent(searchEngine, browserStore, any()) } returns null controller = SearchDialogController( activity = activity, sessionManager = sessionManager, - store = store, + store = browserStore, + fragmentStore = store, navController = navController, settings = settings, metrics = metrics, diff --git a/app/src/test/java/org/mozilla/fenix/search/SearchDialogInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/search/SearchDialogInteractorTest.kt index 7fc3761d1..53a224c19 100644 --- a/app/src/test/java/org/mozilla/fenix/search/SearchDialogInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/SearchDialogInteractorTest.kt @@ -8,8 +8,8 @@ import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runBlockingTest -import mozilla.components.browser.search.SearchEngine import mozilla.components.browser.session.Session +import mozilla.components.browser.state.search.SearchEngine import org.junit.Before import org.junit.Test diff --git a/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt index 2e5b0d0e9..af3324002 100644 --- a/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt @@ -10,14 +10,17 @@ import io.mockk.impl.annotations.MockK import io.mockk.mockk import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.runBlocking -import mozilla.components.browser.search.SearchEngine -import mozilla.components.browser.search.provider.SearchEngineList +import mozilla.components.browser.state.search.RegionState +import mozilla.components.browser.state.search.SearchEngine import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.ContentState +import mozilla.components.browser.state.state.SearchState import mozilla.components.browser.state.state.TabSessionState import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull import org.junit.Assert.assertNotSame +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -26,14 +29,12 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.components.Components import org.mozilla.fenix.components.metrics.Event.PerformedSearch.SearchAccessPoint -import org.mozilla.fenix.components.searchengine.FenixSearchEngineProvider import org.mozilla.fenix.utils.Settings @ExperimentalCoroutinesApi class SearchFragmentStoreTest { @MockK private lateinit var searchEngine: SearchEngine - @MockK private lateinit var searchProvider: FenixSearchEngineProvider @MockK private lateinit var activity: HomeActivity @MockK(relaxed = true) private lateinit var components: Components @MockK(relaxed = true) private lateinit var settings: Settings @@ -45,12 +46,6 @@ class SearchFragmentStoreTest { override var mode: BrowsingMode = BrowsingMode.Normal } every { components.settings } returns settings - every { components.search.provider } returns searchProvider - every { searchProvider.getDefaultEngine(activity) } returns searchEngine - every { searchProvider.installedSearchEngines(activity) } returns SearchEngineList( - list = listOf(mockk(), mockk()), - default = searchEngine - ) } @Test @@ -63,12 +58,13 @@ class SearchFragmentStoreTest { query = "", url = "", searchTerms = "", - searchEngineSource = SearchEngineSource.Default(searchEngine), - defaultEngineSource = SearchEngineSource.Default(searchEngine), + searchEngineSource = SearchEngineSource.None, + defaultEngine = null, + showSearchShortcutsSetting = true, showSearchSuggestions = false, showSearchSuggestionsHint = false, - showSearchShortcuts = true, - areShortcutsAvailable = true, + showSearchShortcuts = false, + areShortcutsAvailable = false, showClipboardSuggestions = false, showHistorySuggestions = false, showBookmarkSuggestions = false, @@ -120,12 +116,13 @@ class SearchFragmentStoreTest { query = "https://example.com", url = "https://example.com", searchTerms = "search terms", - searchEngineSource = SearchEngineSource.Default(searchEngine), - defaultEngineSource = SearchEngineSource.Default(searchEngine), + searchEngineSource = SearchEngineSource.None, + defaultEngine = null, showSearchSuggestions = false, + showSearchShortcutsSetting = false, showSearchSuggestionsHint = false, showSearchShortcuts = false, - areShortcutsAvailable = true, + areShortcutsAvailable = false, showClipboardSuggestions = false, showHistorySuggestions = false, showBookmarkSuggestions = false, @@ -176,16 +173,6 @@ class SearchFragmentStoreTest { assertEquals(true, store.state.showSearchShortcuts) } - @Test - fun hideSearchShortcutEnginePicker() = runBlocking { - val initialState = emptyDefaultState() - val store = SearchFragmentStore(initialState) - - store.dispatch(SearchFragmentAction.UpdateShortcutsAvailability(false)).join() - assertNotSame(initialState, store.state) - assertEquals(false, store.state.showSearchShortcuts) - } - @Test fun showSearchSuggestions() = runBlocking { val initialState = emptyDefaultState() @@ -213,26 +200,132 @@ class SearchFragmentStoreTest { } @Test - fun selectNewDefaultEngine() = runBlocking { - val initialState = emptyDefaultState() - val store = SearchFragmentStore(initialState) + fun `Updating SearchFragmentState from SearchState`() = runBlocking { + val store = SearchFragmentStore(emptyDefaultState( + searchEngineSource = SearchEngineSource.None, + areShortcutsAvailable = false, + defaultEngine = null, + showSearchShortcutsSetting = true + )) - store.dispatch(SearchFragmentAction.SelectNewDefaultSearchEngine(searchEngine)).join() - assertNotSame(initialState, store.state) - assertEquals(SearchEngineSource.Default(searchEngine), store.state.searchEngineSource) + assertNull(store.state.defaultEngine) + assertFalse(store.state.areShortcutsAvailable) + assertFalse(store.state.showSearchShortcuts) + assertEquals(SearchEngineSource.None, store.state.searchEngineSource) + + store.dispatch( + SearchFragmentAction.UpdateSearchState( + SearchState( + region = RegionState("US", "US"), + regionSearchEngines = listOf( + SearchEngine("engine-a", "Engine A", mockk(), type = SearchEngine.Type.BUNDLED), + SearchEngine("engine-b", "Engine B", mockk(), type = SearchEngine.Type.BUNDLED), + SearchEngine("engine-c", "Engine C", mockk(), type = SearchEngine.Type.BUNDLED) + ), + customSearchEngines = listOf( + SearchEngine("engine-d", "Engine D", mockk(), type = SearchEngine.Type.CUSTOM), + SearchEngine("engine-e", "Engine E", mockk(), type = SearchEngine.Type.CUSTOM) + ), + additionalSearchEngines = listOf( + SearchEngine("engine-f", "Engine F", mockk(), type = SearchEngine.Type.BUNDLED_ADDITIONAL) + ), + additionalAvailableSearchEngines = listOf( + SearchEngine("engine-g", "Engine G", mockk(), type = SearchEngine.Type.BUNDLED_ADDITIONAL), + SearchEngine("engine-h", "Engine H", mockk(), type = SearchEngine.Type.BUNDLED_ADDITIONAL) + ), + hiddenSearchEngines = listOf( + SearchEngine("engine-i", "Engine I", mockk(), type = SearchEngine.Type.BUNDLED) + ), + regionDefaultSearchEngineId = "engine-b", + userSelectedSearchEngineId = null, + userSelectedSearchEngineName = null + ) + ) + ).join() + + assertNotNull(store.state.defaultEngine) + assertEquals("Engine B", store.state.defaultEngine!!.name) + + assertTrue(store.state.areShortcutsAvailable) + assertTrue(store.state.showSearchShortcuts) + + assertTrue(store.state.searchEngineSource is SearchEngineSource.Default) + assertNotNull(store.state.searchEngineSource.searchEngine) + assertEquals("Engine B", store.state.searchEngineSource.searchEngine!!.name) + } + + @Test + fun `Updating SearchFragmentState from SearchState - shortcuts disabled`() = runBlocking { + val store = SearchFragmentStore(emptyDefaultState( + searchEngineSource = SearchEngineSource.None, + areShortcutsAvailable = false, + defaultEngine = null, + showSearchShortcutsSetting = false + )) + + assertNull(store.state.defaultEngine) + assertFalse(store.state.areShortcutsAvailable) + assertFalse(store.state.showSearchShortcuts) + assertEquals(SearchEngineSource.None, store.state.searchEngineSource) + + store.dispatch( + SearchFragmentAction.UpdateSearchState( + SearchState( + region = RegionState("US", "US"), + regionSearchEngines = listOf( + SearchEngine("engine-a", "Engine A", mockk(), type = SearchEngine.Type.BUNDLED), + SearchEngine("engine-b", "Engine B", mockk(), type = SearchEngine.Type.BUNDLED), + SearchEngine("engine-c", "Engine C", mockk(), type = SearchEngine.Type.BUNDLED) + ), + customSearchEngines = listOf( + SearchEngine("engine-d", "Engine D", mockk(), type = SearchEngine.Type.CUSTOM), + SearchEngine("engine-e", "Engine E", mockk(), type = SearchEngine.Type.CUSTOM) + ), + additionalSearchEngines = listOf( + SearchEngine("engine-f", "Engine F", mockk(), type = SearchEngine.Type.BUNDLED_ADDITIONAL) + ), + additionalAvailableSearchEngines = listOf( + SearchEngine("engine-g", "Engine G", mockk(), type = SearchEngine.Type.BUNDLED_ADDITIONAL), + SearchEngine("engine-h", "Engine H", mockk(), type = SearchEngine.Type.BUNDLED_ADDITIONAL) + ), + hiddenSearchEngines = listOf( + SearchEngine("engine-i", "Engine I", mockk(), type = SearchEngine.Type.BUNDLED) + ), + regionDefaultSearchEngineId = "engine-b", + userSelectedSearchEngineId = null, + userSelectedSearchEngineName = null + ) + ) + ).join() + + assertNotNull(store.state.defaultEngine) + assertEquals("Engine B", store.state.defaultEngine!!.name) + + assertTrue(store.state.areShortcutsAvailable) + assertFalse(store.state.showSearchShortcuts) + + assertTrue(store.state.searchEngineSource is SearchEngineSource.Default) + assertNotNull(store.state.searchEngineSource.searchEngine) + assertEquals("Engine B", store.state.searchEngineSource.searchEngine!!.name) } - private fun emptyDefaultState(): SearchFragmentState = SearchFragmentState( + private fun emptyDefaultState( + searchEngineSource: SearchEngineSource = mockk(), + defaultEngine: SearchEngine? = mockk(), + areShortcutsAvailable: Boolean = true, + showSearchShortcutsSetting: Boolean = false + ): SearchFragmentState = SearchFragmentState( tabId = null, url = "", searchTerms = "", query = "", - searchEngineSource = mockk(), - defaultEngineSource = mockk(), + searchEngineSource = searchEngineSource, + defaultEngine = defaultEngine, showSearchSuggestionsHint = false, + showSearchShortcutsSetting = showSearchShortcutsSetting, showSearchSuggestions = false, showSearchShortcuts = false, - areShortcutsAvailable = true, + areShortcutsAvailable = areShortcutsAvailable, showClipboardSuggestions = false, showHistorySuggestions = false, showBookmarkSuggestions = false, diff --git a/app/src/test/java/org/mozilla/fenix/search/awesomebar/ShortcutsSuggestionProviderTest.kt b/app/src/test/java/org/mozilla/fenix/search/awesomebar/ShortcutsSuggestionProviderTest.kt index d5ad43485..655c8cc1a 100644 --- a/app/src/test/java/org/mozilla/fenix/search/awesomebar/ShortcutsSuggestionProviderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/awesomebar/ShortcutsSuggestionProviderTest.kt @@ -13,15 +13,16 @@ import io.mockk.unmockkStatic import io.mockk.verifySequence import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runBlockingTest -import mozilla.components.browser.search.SearchEngine -import mozilla.components.browser.search.provider.SearchEngineList +import mozilla.components.browser.state.search.SearchEngine +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.SearchState +import mozilla.components.browser.state.store.BrowserStore import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Before import org.junit.Test import org.mozilla.fenix.R -import org.mozilla.fenix.components.searchengine.FenixSearchEngineProvider @ExperimentalCoroutinesApi class ShortcutsSuggestionProviderTest { @@ -52,33 +53,32 @@ class ShortcutsSuggestionProviderTest { @Test fun `returns suggestions from search engine provider`() = runBlockingTest { val engineOne = mockk { - every { identifier } returns "1" + every { id } returns "1" every { name } returns "EngineOne" every { icon } returns mockk() } val engineTwo = mockk { - every { identifier } returns "2" + every { id } returns "2" every { name } returns "EngineTwo" every { icon } returns mockk() } - val searchEngineProvider = mockk { - every { installedSearchEngines(context) } returns SearchEngineList( - list = listOf(engineOne, engineTwo), - default = null + val store = BrowserStore(BrowserState( + search = SearchState( + regionSearchEngines = listOf(engineOne, engineTwo) ) - } - val provider = ShortcutsSuggestionProvider(searchEngineProvider, context, mockk(), mockk()) + )) + val provider = ShortcutsSuggestionProvider(store, context, mockk(), mockk()) val suggestions = provider.onInputChanged("") assertEquals(3, suggestions.size) assertEquals(provider, suggestions[0].provider) - assertEquals(engineOne.identifier, suggestions[0].id) + assertEquals(engineOne.id, suggestions[0].id) assertEquals(engineOne.icon, suggestions[0].icon) assertEquals(engineOne.name, suggestions[0].title) assertEquals(provider, suggestions[1].provider) - assertEquals(engineTwo.identifier, suggestions[1].id) + assertEquals(engineTwo.id, suggestions[1].id) assertEquals(engineTwo.icon, suggestions[1].icon) assertEquals(engineTwo.name, suggestions[1].title) @@ -90,16 +90,16 @@ class ShortcutsSuggestionProviderTest { @Test fun `callbacks are triggered when suggestions are clicked`() = runBlockingTest { val engineOne = mockk(relaxed = true) - val searchEngineProvider = mockk { - every { installedSearchEngines(context) } returns SearchEngineList( - list = listOf(engineOne), - default = null + val store = BrowserStore(BrowserState( + search = SearchState( + regionSearchEngines = listOf(engineOne) ) - } + )) + val selectShortcutEngine = mockk<(SearchEngine) -> Unit>(relaxed = true) val selectShortcutEngineSettings = mockk<() -> Unit>(relaxed = true) val provider = ShortcutsSuggestionProvider( - searchEngineProvider, + store, context, selectShortcutEngine, selectShortcutEngineSettings diff --git a/app/src/test/java/org/mozilla/fenix/search/toolbar/ToolbarViewTest.kt b/app/src/test/java/org/mozilla/fenix/search/toolbar/ToolbarViewTest.kt index 2312ebb94..0cb32ce9c 100644 --- a/app/src/test/java/org/mozilla/fenix/search/toolbar/ToolbarViewTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/toolbar/ToolbarViewTest.kt @@ -48,7 +48,8 @@ class ToolbarViewTest { every { name } returns "Search Engine" every { icon } returns testContext.getDrawable(R.drawable.ic_search)!!.toBitmap() }), - defaultEngineSource = mockk(relaxed = true), + defaultEngine = null, + showSearchShortcutsSetting = false, showSearchSuggestionsHint = false, showSearchSuggestions = false, showSearchShortcuts = false, diff --git a/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/perf/MozillaUseLazyMonitored.kt b/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/perf/MozillaUseLazyMonitored.kt index 3a9430af3..abfaf8720 100644 --- a/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/perf/MozillaUseLazyMonitored.kt +++ b/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/perf/MozillaUseLazyMonitored.kt @@ -78,7 +78,6 @@ open class MozillaUseLazyMonitored(config: Config) : Rule(config) { "IntentProcessors", "PerformanceComponent", "Push", - "Search", "Services", "UseCases" ).map { "app/src/main/java/org/mozilla/fenix/components/$it.kt" }