Archives
/
iceraven-browser
mirror of https://github.com/fork-maintainers/iceraven-browser
2
0
Fork 0
Code Issues Releases Wiki Activity

For #15080: Handle default search engine when editing or removing (#15768)

Browse Source
upstream-sync
Elise Richards 4 years ago committed by GitHub
parent eb0712d9b4
commit 7e3c26914c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 206 additions and 128 deletions
Show Stats Download Patch File Download Diff File

6
app/src/main/java/org/mozilla/fenix/components/Search.kt
Unescape Escape View File

@ -10,7 +10,6 @@ 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.ext.settings
import org.mozilla.fenix.utils.Mockable
/**
@ -30,10 +29,7 @@ class Search(private val context: Context) {
).apply {
registerForLocaleUpdates(context)
GlobalScope.launch {
defaultSearchEngine = getDefaultSearchEngineAsync(
context,
context.settings().defaultSearchEngineName
)
defaultSearchEngine = provider.getDefaultEngine(context)
}
}
}

104
app/src/main/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProvider.kt
Unescape Escape View File

@ -33,7 +33,7 @@ import java.util.Locale
open class FenixSearchEngineProvider(
private val context: Context
) : SearchEngineProvider, CoroutineScope by CoroutineScope(Job() + Dispatchers.IO) {
private val shouldMockMLS = Config.channel.isDebug || BuildConfig.MLS_TOKEN.isNullOrEmpty()
private val shouldMockMLS = Config.channel.isDebug || BuildConfig.MLS_TOKEN.isEmpty()
private val locationService: LocationService = if (shouldMockMLS) {
LocationService.dummy()
} else {
@ -55,8 +55,12 @@ open class FenixSearchEngineProvider(
open val localizationProvider: SearchLocalizationProvider =
RegionSearchLocalizationProvider(locationService)
/**
* Unfiltered list of search engines based on locale.
*/
open var baseSearchEngines = async {
AssetsSearchEngineProvider(localizationProvider).loadSearchEngines(context)
AssetsSearchEngineProvider(localizationProvider)
.loadSearchEngines(context)
}
private val loadedRegion = async { localizationProvider.determineRegion() }
@ -72,9 +76,13 @@ open class FenixSearchEngineProvider(
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()
val defaultEngineIdentifiers =
baseSearchEngines.await().list.map { it.identifier }.toSet()
AssetsSearchEngineProvider(
localizationProvider,
filters = listOf(object : SearchEngineFilter {
@ -87,12 +95,15 @@ open class FenixSearchEngineProvider(
).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 = refreshAsync(baseSearchEngines)
private var loadedSearchEngines = refreshInstalledEngineListAsync(baseSearchEngines)
// https://github.com/mozilla-mobile/fenix/issues/9935
// Create new getter that will return the fallback SearchEngineList if
@ -102,29 +113,45 @@ open class FenixSearchEngineProvider(
if (isRegionCachedByLocationService) {
loadedSearchEngines
} else {
refreshAsync(fallbackEngines)
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()
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.
* are readily available throughout the app. Includes all installed engines, both
* default and custom
*/
fun installedSearchEngines(context: Context): SearchEngineList = runBlocking {
val installedIdentifiers = installedSearchEngineIdentifiers(context)
val engineList = searchEngines.await()
val defaultList = searchEngines.await()
engineList.copy(
list = engineList.list.filter {
defaultList.copy(
list = defaultList.list.filter {
installedIdentifiers.contains(it.identifier)
}.sortedBy { it.name.toLowerCase(Locale.getDefault()) },
default = engineList.default?.let {
}.sortedBy {
it.name.toLowerCase(Locale.getDefault())
},
default = defaultList.default?.let {
if (installedIdentifiers.contains(it.identifier)) {
it
} else {
@ -142,14 +169,20 @@ open class FenixSearchEngineProvider(
val installedIdentifiers = installedSearchEngineIdentifiers(context)
val engineList = loadedSearchEngines.await()
engineList.copy(list = engineList.list.filterNot { installedIdentifiers.contains(it.identifier) })
return@runBlocking 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) = runBlocking {
fun installSearchEngine(
context: Context,
searchEngine: SearchEngine,
isCustom: Boolean = false
) = runBlocking {
if (isCustom) {
val searchUrl = searchEngine.getSearchTemplate()
CustomSearchEngineStore.addSearchEngine(context, searchEngine.name, searchUrl)
@ -158,25 +191,34 @@ open class FenixSearchEngineProvider(
val installedIdentifiers = installedSearchEngineIdentifiers(context).toMutableSet()
installedIdentifiers.add(searchEngine.identifier)
prefs(context).edit()
.putStringSet(localeAwareInstalledEnginesKey(), installedIdentifiers).apply()
.putStringSet(
localeAwareInstalledEnginesKey(), installedIdentifiers
).apply()
}
}
fun uninstallSearchEngine(context: Context, searchEngine: SearchEngine, isCustom: Boolean = false) = runBlocking {
fun uninstallSearchEngine(
context: Context,
searchEngine: SearchEngine,
isCustom: Boolean = false
) = runBlocking {
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()
prefs(context).edit().putStringSet(
localeAwareInstalledEnginesKey(),
installedIdentifiers
).apply()
}
}
fun reload() {
launch {
customSearchEngines = async { CustomSearchEngineProvider().loadSearchEngines(context) }
loadedSearchEngines = refreshAsync(baseSearchEngines)
loadedSearchEngines = refreshInstalledEngineListAsync(baseSearchEngines)
}
}
@ -184,16 +226,19 @@ open class FenixSearchEngineProvider(
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
open fun updateBaseSearchEngines() {
baseSearchEngines = async {
AssetsSearchEngineProvider(localizationProvider).loadSearchEngines(context)
AssetsSearchEngineProvider(localizationProvider)
.loadSearchEngines(context)
}
}
private fun refreshAsync(baseList: Deferred<SearchEngineList>) = async {
val engineList = baseList.await()
private fun refreshInstalledEngineListAsync(
engines: Deferred<SearchEngineList>
): Deferred<SearchEngineList> = async {
val engineList = engines.await()
val bundledList = bundledSearchEngines.await().list
val customList = customSearchEngines.await().list
engineList.copy(list = engineList.list + bundledList + customList)
return@async engineList.copy(list = engineList.list + bundledList + customList)
}
private fun prefs(context: Context) = context.getSharedPreferences(
@ -208,8 +253,11 @@ open class FenixSearchEngineProvider(
if (!prefs.contains(installedEnginesKey)) {
val searchEngines =
if (isRegionCachedByLocationService) baseSearchEngines
else fallbackEngines
if (isRegionCachedByLocationService) {
baseSearchEngines
} else {
fallbackEngines
}
val defaultSet = searchEngines.await()
.list
@ -219,9 +267,12 @@ open class FenixSearchEngineProvider(
prefs.edit().putStringSet(installedEnginesKey, defaultSet).apply()
}
val installedIdentifiers = prefs(context).getStringSet(installedEnginesKey, setOf()) ?: setOf()
val installedIdentifiers: Set<String> =
prefs(context).getStringSet(installedEnginesKey, setOf()) ?: setOf()
val customEngineIdentifiers =
customSearchEngines.await().list.map { it.identifier }.toSet()
val customEngineIdentifiers = customSearchEngines.await().list.map { it.identifier }.toSet()
return installedIdentifiers + customEngineIdentifiers
}
@ -251,6 +302,5 @@ open class FenixSearchEngineProvider(
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"
const val CURRENT_LOCALE_KEY = "fenix-current-locale"
}
}

94
app/src/main/java/org/mozilla/fenix/settings/search/AddSearchEngineFragment.kt
Unescape Escape View File

@ -141,37 +141,7 @@ class AddSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine),
val name = edit_engine_name.text?.toString()?.trim() ?: ""
val searchString = edit_search_string.text?.toString() ?: ""
var hasError = false
if (name.isEmpty()) {
custom_search_engine_name_field.error = resources
.getString(R.string.search_add_custom_engine_error_empty_name)
hasError = true
}
val existingIdentifiers = requireComponents
.search
.provider
.allSearchEngineIdentifiers()
.map { it.toLowerCase(Locale.ROOT) }
if (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
)
hasError = true
}
custom_search_engine_search_string_field.error = when {
searchString.isEmpty() ->
resources.getString(R.string.search_add_custom_engine_error_empty_search_string)
!searchString.contains("%s") ->
resources.getString(R.string.search_add_custom_engine_error_missing_template)
else -> null
}
if (custom_search_engine_search_string_field.error != null) {
hasError = true
}
val hasError = checkForErrors(name, searchString)
if (hasError) { return }
@ -189,17 +159,28 @@ class AddSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine),
.getString(R.string.search_add_custom_engine_error_cannot_reach, name)
}
SearchStringValidator.Result.Success -> {
CustomSearchEngineStore.addSearchEngine(
context = requireContext(),
engineName = name,
searchQuery = searchString
)
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 successMessage = resources
.getString(R.string.search_add_custom_engine_success_message, name)
view?.also {
FenixSnackbar.make(view = it,
FenixSnackbar.make(
view = it,
duration = FenixSnackbar.LENGTH_SHORT,
isDisplayedWithBrowserToolbar = false
)
@ -214,6 +195,44 @@ 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 {
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)
true
}
!searchString.contains("%s") -> {
custom_search_engine_search_string_field.error =
resources.getString(R.string.search_add_custom_engine_error_missing_template)
true
}
else -> false
}
return hasError
}
private fun installEngine(engine: SearchEngine) {
viewLifecycleOwner.lifecycleScope.launch(Main) {
withContext(IO) {
@ -282,6 +301,5 @@ class AddSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine),
private const val DISABLED_ALPHA = 0.2f
private const val CUSTOM_INDEX = -1
private const val FIRST_INDEX = 0
private const val DPS_TO_INCREASE = 20
}
}

82
app/src/main/java/org/mozilla/fenix/settings/search/EditCustomSearchEngineFragment.kt
Unescape Escape View File

@ -36,7 +36,6 @@ import java.util.Locale
class EditCustomSearchEngineFragment : Fragment(R.layout.fragment_add_search_engine) {
private val args by navArgs<EditCustomSearchEngineFragmentArgs>()
private lateinit var searchEngine: SearchEngine
override fun onCreate(savedInstanceState: Bundle?) {
@ -93,48 +92,13 @@ class EditCustomSearchEngineFragment : Fragment(R.layout.fragment_add_search_eng
val name = edit_engine_name.text?.toString()?.trim() ?: ""
val searchString = edit_search_string.text?.toString() ?: ""
var hasError = false
if (name.isEmpty()) {
custom_search_engine_name_field.error = resources
.getString(R.string.search_add_custom_engine_error_empty_name)
hasError = true
}
val existingIdentifiers = requireComponents
.search
.provider
.allSearchEngineIdentifiers()
.map { it.toLowerCase(Locale.ROOT) }
val nameHasChanged = name != args.searchEngineIdentifier
if (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
)
hasError = true
}
if (searchString.isEmpty()) {
custom_search_engine_search_string_field
.error =
resources.getString(R.string.search_add_custom_engine_error_empty_search_string)
hasError = true
}
if (!searchString.contains("%s")) {
custom_search_engine_search_string_field
.error =
resources.getString(R.string.search_add_custom_engine_error_missing_template)
hasError = true
}
val hasError = checkForErrors(name, searchString)
if (hasError) {
return
}
viewLifecycleOwner.lifecycleScope.launch(Main) {
lifecycleScope.launch(Main) {
val result = withContext(IO) {
SearchStringValidator.isSearchStringValid(
requireComponents.core.client,
@ -167,10 +131,50 @@ class EditCustomSearchEngineFragment : Fragment(R.layout.fragment_add_search_eng
.setText(successMessage)
.show()
}
if (args.isDefaultSearchEngine) {
requireComponents.search.provider.setDefaultEngine(requireContext(), name)
}
findNavController().popBackStack()
}
}
}
}
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 {
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)
true
}
!searchString.contains("%s") -> {
custom_search_engine_search_string_field.error =
resources.getString(R.string.search_add_custom_engine_error_missing_template)
true
}
else -> false
}
return hasError
}
}

2
app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt
Unescape Escape View File

@ -25,7 +25,7 @@ class RadioSearchEngineListPreference @JvmOverloads constructor(
}
override fun onSearchEngineSelected(searchEngine: SearchEngine) {
context.components.search.searchEngineManager.defaultSearchEngine = searchEngine
context.components.search.provider.setDefaultEngine(context, searchEngine.identifier)
context.settings().defaultSearchEngineName = searchEngine.name
}
}

3
app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineFragment.kt
Unescape Escape View File

@ -10,6 +10,7 @@ import androidx.preference.CheckBoxPreference
import androidx.preference.Preference
import androidx.preference.PreferenceFragmentCompat
import androidx.preference.SwitchPreference
import mozilla.components.support.ktx.android.view.hideKeyboard
import org.mozilla.fenix.R
import org.mozilla.fenix.ext.getPreferenceKey
import org.mozilla.fenix.ext.settings
@ -21,10 +22,12 @@ class SearchEngineFragment : PreferenceFragmentCompat() {
override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) {
setPreferencesFromResource(R.xml.search_preferences, rootKey)
view?.hideKeyboard()
}
override fun onResume() {
super.onResume()
view?.hideKeyboard()
showToolbar(getString(R.string.preferences_search))
val searchSuggestionsPreference =

23
app/src/main/java/org/mozilla/fenix/settings/search/SearchEngineListPreference.kt
Unescape Escape View File

@ -71,15 +71,14 @@ abstract class SearchEngineListPreference @JvmOverloads constructor(
return
}
val defaultEngine = context.components.search.provider.getDefaultEngine(context).identifier
val defaultEngineId = context.components.search.provider.getDefaultEngine(context).identifier
val selectedEngine = (searchEngineList.list.find {
it.identifier == defaultEngine
it.identifier == defaultEngineId
} ?: searchEngineList.list.first()).identifier
context.components.search.searchEngineManager.defaultSearchEngine =
searchEngineList.list.find {
it.identifier == selectedEngine
}
// set the search engine manager default
context.components.search.provider.setDefaultEngine(context, selectedEngine)
searchEngineGroup!!.removeAllViews()
@ -175,8 +174,9 @@ abstract class SearchEngineListPreference @JvmOverloads constructor(
}
private fun editCustomSearchEngine(engine: SearchEngine) {
val wasDefault = context.components.search.provider.getDefaultEngine(context).identifier == engine.identifier
val directions = SearchEngineFragmentDirections
.actionSearchEngineFragmentToEditCustomSearchEngineFragment(engine.identifier)
.actionSearchEngineFragmentToEditCustomSearchEngineFragment(engine.identifier, wasDefault)
Navigation.findNavController(searchEngineGroup!!).navigate(directions)
}
@ -215,12 +215,9 @@ abstract class SearchEngineListPreference @JvmOverloads constructor(
},
operation = {
if (isDefaultEngine) {
context.settings().defaultSearchEngineName = context
.components
.search
.provider
.getDefaultEngine(context)
.name
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)

3
app/src/main/res/navigation/nav_graph.xml
Unescape Escape View File

@ -943,6 +943,9 @@
<argument
android:name="searchEngineIdentifier"
app:argType="string" />
<argument
android:name="isDefaultSearchEngine"
app:argType="boolean" />
</fragment>
</navigation>
</navigation>

17
app/src/test/java/org/mozilla/fenix/components/searchengine/FenixSearchEngineProviderTest.kt
Unescape Escape View File

@ -1,12 +1,12 @@
package org.mozilla.fenix.components.searchengine
import android.content.Context
import io.mockk.every
import io.mockk.mockk
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
@ -35,7 +35,8 @@ class FenixSearchEngineProviderTest {
mockkObject(CustomSearchEngineStore)
fenixSearchEngineProvider.let {
every { CustomSearchEngineStore.loadCustomSearchEngines(testContext) } returns listOf(
(it as FakeFenixSearchEngineProvider).mockSearchEngine("my custom site", "my custom site")
(it as FakeFenixSearchEngineProvider)
.mockSearchEngine("my custom site", "my custom site")
)
}
}
@ -82,8 +83,14 @@ class FenixSearchEngineProviderTest {
@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 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()

Write Preview
Loading…
Cancel
Save