From 95703b9555667fba9f58cc45b933f31d8165256b Mon Sep 17 00:00:00 2001 From: DreVla Date: Wed, 7 Dec 2022 16:28:27 +0200 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/28111: Filter out topic specific search engines from default list Since topic specific search engines, like amazon, ebay, etc... will only show results from those specific websites, they will be filtered out of the default search engines list. --- .../mozilla/fenix/ui/SettingsSearchTest.kt | 1 + .../org/mozilla/fenix/FenixApplication.kt | 22 ++++++++++++ .../search/AddSearchEngineFragment.kt | 1 + .../search/RadioSearchEngineListPreference.kt | 36 +++++++++++++++++-- 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsSearchTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsSearchTest.kt index 8670d956f4..be8c1c4d65 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsSearchTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/SettingsSearchTest.kt @@ -376,6 +376,7 @@ class SettingsSearchTest { } } + @Ignore("Test failure caused by: https://bugzilla.mozilla.org/show_bug.cgi?id=1807298") // Expected for en-us defaults @Test fun deleteAllSearchEnginesTest() { diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index f5ca4efa5e..ba6fab5301 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -28,6 +28,8 @@ import kotlinx.coroutines.launch import mozilla.appservices.Megazord import mozilla.components.browser.state.action.SystemAction import mozilla.components.browser.state.selector.selectedTab +import mozilla.components.browser.state.state.searchEngines +import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.storage.sync.GlobalPlacesDependencyProvider import mozilla.components.concept.base.crash.Breadcrumb @@ -245,6 +247,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider { setupLeakCanary() startMetricsIfEnabled() setupPush() + migrateTopicSpecificSearchEngines() visibilityLifecycleCallback = VisibilityLifecycleCallback(getSystemService()) registerActivityLifecycleCallbacks(visibilityLifecycleCallback) @@ -553,6 +556,25 @@ open class FenixApplication : LocaleAwareApplication(), Provider { } } + /** + * If unified search is enabled try to migrate the topic specific engine to the + * first general or custom search engine available. + */ + @Suppress("NestedBlockDepth") + private fun migrateTopicSpecificSearchEngines() { + if (settings().showUnifiedSearchFeature) { + components.core.store.state.search.selectedOrDefaultSearchEngine.let { currentSearchEngine -> + if (currentSearchEngine?.isGeneral == false) { + components.core.store.state.search.searchEngines.firstOrNull() { nextSearchEngine -> + nextSearchEngine.isGeneral + }?.let { + components.useCases.searchUseCases.selectSearchEngine(it) + } + } + } + } + } + @OptIn(DelicateCoroutinesApi::class) // GlobalScope usage private fun warmBrowsersCache() { // We avoid blocking the main thread for BrowsersCache on startup by loading it on 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 da6831ba17..c5bfcf9000 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 @@ -182,6 +182,7 @@ class AddSearchEngineFragment : name, searchString.toSearchUrl(), requireComponents.core.icons.loadIcon(IconRequest(searchString)).await().bitmap, + isGeneral = true, ) requireComponents.useCases.searchUseCases.addSearchEngine(searchEngine) 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 25ee8e9249..5cd4d55462 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 @@ -20,7 +20,6 @@ import androidx.preference.Preference import androidx.preference.PreferenceViewHolder 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 @@ -35,6 +34,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.databinding.SearchEngineRadioButtonBinding import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getRootView +import org.mozilla.fenix.ext.settings import org.mozilla.fenix.utils.allowUndo class RadioSearchEngineListPreference @JvmOverloads constructor( @@ -76,14 +76,23 @@ class RadioSearchEngineListPreference @JvmOverloads constructor( ViewGroup.LayoutParams.WRAP_CONTENT, ) + val isLastGeneralOrCustomSearchEngine = state.searchEngines.filter { + it.isGeneral + }.size == 1 state.searchEngines.filter { engine -> engine.type != SearchEngine.Type.APPLICATION }.forEach { engine -> + val isLastSearchEngineAvailable = + state.searchEngines.count { it.type != SearchEngine.Type.APPLICATION } > 1 val searchEngineView = makeButtonFromSearchEngine( engine = engine, layoutInflater = layoutInflater, res = context.resources, - allowDeletion = state.searchEngines.count { it.type != SearchEngine.Type.APPLICATION } > 1, + allowDeletion = if (context.settings().showUnifiedSearchFeature) { + isLastSearchEngineAvailable && !(engine.isGeneral && isLastGeneralOrCustomSearchEngine) + } else { + isLastSearchEngineAvailable + }, isSelected = engine == state.selectedOrDefaultSearchEngine, ) @@ -104,7 +113,12 @@ class RadioSearchEngineListPreference @JvmOverloads constructor( val binding = SearchEngineRadioButtonBinding.bind(wrapper) - wrapper.setOnClickListener { binding.radioButton.isChecked = true } + if (context.settings().showUnifiedSearchFeature && !engine.isGeneral) { + binding.radioButton.isEnabled = false + wrapper.isEnabled = false + } else { + wrapper.setOnClickListener { binding.radioButton.isChecked = true } + } binding.radioButton.tag = engine.id binding.radioButton.isChecked = isSelected binding.radioButton.setOnCheckedChangeListener(this) @@ -155,6 +169,19 @@ class RadioSearchEngineListPreference @JvmOverloads constructor( context: Context, engine: SearchEngine, ) { + val selectedOrDefaultSearchEngine = context.components.core.store.state.search.selectedOrDefaultSearchEngine + if (selectedOrDefaultSearchEngine == engine) { + val nextSearchEngine = if (context.settings().showUnifiedSearchFeature) { + context.components.core.store.state.search.searchEngines.first { + it.id != engine.id && (it.isGeneral || it.type == SearchEngine.Type.CUSTOM) + } + } else { + context.components.core.store.state.search.searchEngines.first { + it.id != engine.id + } + } + context.components.useCases.searchUseCases.selectSearchEngine(nextSearchEngine) + } context.components.useCases.searchUseCases.removeSearchEngine(engine) MainScope().allowUndo( @@ -164,6 +191,9 @@ class RadioSearchEngineListPreference @JvmOverloads constructor( undoActionTitle = context.getString(R.string.snackbar_deleted_undo), onCancel = { context.components.useCases.searchUseCases.addSearchEngine(engine) + if (selectedOrDefaultSearchEngine == engine) { + context.components.useCases.searchUseCases.selectSearchEngine(engine) + } }, operation = {}, )