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

[fenix] For https://github.com/mozilla-mobile/fenix/issues/25816: Ensure the "show search suggestions" user option is followed

Browse Source 
Previously the check for the "Search -> Show search suggestions" user setting
was only used in the default SearchFragmentState but not again if users change
the current search engine as part of the unified search feature.
This comes to ensure that that check is always made when needing to configure
new search engine results.
pull/600/head
Mugurell 1 year ago committed by mergify[bot]
parent 7c033fc8cd
commit d9074aaa2c
6 changed files with 202 additions and 80 deletions
Show Stats Download Patch File Download Diff File

16
app/src/main/java/org/mozilla/fenix/search/SearchDialogController.kt
Unescape Escape View File

@ -217,10 +217,22 @@ class SearchDialogController(
fragmentStore.dispatch(SearchFragmentAction.SearchTabsEngineSelected(searchEngine))
}
searchEngine == store.state.search.selectedOrDefaultSearchEngine -> {
fragmentStore.dispatch(SearchFragmentAction.SearchDefaultEngineSelected(searchEngine, settings))
fragmentStore.dispatch(
SearchFragmentAction.SearchDefaultEngineSelected(
engine = searchEngine,
browsingMode = activity.browsingModeManager.mode,
settings = settings,
),
)
}
else -> {
fragmentStore.dispatch(SearchFragmentAction.SearchShortcutEngineSelected(searchEngine, settings))
fragmentStore.dispatch(
SearchFragmentAction.SearchShortcutEngineSelected(
engine = searchEngine,
browsingMode = activity.browsingModeManager.mode,
settings = settings,
),
)
}
}

18
app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt
Unescape Escape View File

@ -53,7 +53,7 @@ import mozilla.components.browser.toolbar.BrowserToolbar
import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.menu.candidate.DrawableMenuIcon
import mozilla.components.concept.menu.candidate.TextMenuCandidate
import mozilla.components.concept.storage.HistoryStorage
import mozilla.components.concept.toolbar.AutocompleteProvider
import mozilla.components.concept.toolbar.Toolbar
import mozilla.components.feature.qr.QrFeature
import mozilla.components.feature.toolbar.ToolbarAutocompleteFeature
@ -247,12 +247,16 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler {
engineForSpeculativeConnects,
{ store.state.searchEngineSource.searchEngine?.type != SearchEngine.Type.APPLICATION },
).apply {
addDomainProvider(
ShippedDomainsProvider().also { shippedDomainsProvider ->
shippedDomainsProvider.initialize(requireContext())
},
updateAutocompleteProviders(
listOfNotNull(
historyStorageProvider(),
// Assume the history provider has priority 0 and set priority 1 for the domains provider
// to ensure the first source checked for autocomplete suggestions is history.
ShippedDomainsProvider(1).also { shippedDomainsProvider ->
shippedDomainsProvider.initialize(requireContext())
},
),
)
historyStorageProvider()?.also(::addHistoryStorageProvider)
}
}
@ -659,7 +663,7 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler {
dismissAllowingStateLoss()
}
private fun historyStorageProvider(): HistoryStorage? {
private fun historyStorageProvider(): AutocompleteProvider? {
return if (requireContext().settings().shouldShowHistorySuggestions) {
requireComponents.core.historyStorage
} else {

46
app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt
Unescape Escape View File

@ -4,6 +4,7 @@
package org.mozilla.fenix.search
import androidx.annotation.VisibleForTesting
import mozilla.components.browser.state.search.SearchEngine
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.state.SearchState
@ -136,12 +137,6 @@ fun createInitialSearchFragmentState(
val tab = tabId?.let { components.core.store.state.findTab(it) }
val url = tab?.content?.url.orEmpty()
val shouldShowSearchSuggestions = when (activity.browsingModeManager.mode) {
BrowsingMode.Normal -> settings.shouldShowSearchSuggestions
BrowsingMode.Private ->
settings.shouldShowSearchSuggestions && settings.shouldShowSearchSuggestionsInPrivate
}
val searchEngineSource = if (searchEngine != null) {
SearchEngineSource.Shortcut(searchEngine)
} else {
@ -154,7 +149,10 @@ fun createInitialSearchFragmentState(
searchTerms = tab?.content?.searchTerms.orEmpty(),
searchEngineSource = searchEngineSource,
defaultEngine = null,
showSearchSuggestions = shouldShowSearchSuggestions,
showSearchSuggestions = shouldShowSearchSuggestions(
browsingMode = activity.browsingModeManager.mode,
settings = settings,
),
showSearchSuggestionsHint = false,
showSearchShortcuts = false,
areShortcutsAvailable = false,
@ -184,12 +182,20 @@ sealed class SearchFragmentAction : Action {
/**
* Action when default search engine is selected.
*/
data class SearchDefaultEngineSelected(val engine: SearchEngine, val settings: Settings) : SearchFragmentAction()
data class SearchDefaultEngineSelected(
val engine: SearchEngine,
val browsingMode: BrowsingMode,
val settings: Settings,
) : SearchFragmentAction()
/**
* Action when shortcut search engine is selected.
*/
data class SearchShortcutEngineSelected(val engine: SearchEngine, val settings: Settings) : SearchFragmentAction()
data class SearchShortcutEngineSelected(
val engine: SearchEngine,
val browsingMode: BrowsingMode,
val settings: Settings,
) : SearchFragmentAction()
/**
* Action when history search engine is selected.
@ -242,7 +248,7 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen
is SearchFragmentAction.SearchDefaultEngineSelected ->
state.copy(
searchEngineSource = SearchEngineSource.Default(action.engine),
showSearchSuggestions = action.settings.shouldShowSearchSuggestions,
showSearchSuggestions = shouldShowSearchSuggestions(action.browsingMode, action.settings),
showSearchShortcuts = action.settings.shouldShowSearchShortcuts,
showClipboardSuggestions = action.settings.shouldShowClipboardSuggestions,
showSearchTermHistory = action.settings.showUnifiedSearchFeature &&
@ -256,7 +262,7 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen
is SearchFragmentAction.SearchShortcutEngineSelected ->
state.copy(
searchEngineSource = SearchEngineSource.Shortcut(action.engine),
showSearchSuggestions = true,
showSearchSuggestions = shouldShowSearchSuggestions(action.browsingMode, action.settings),
showSearchShortcuts = when (action.settings.showUnifiedSearchFeature) {
true -> false
false -> action.settings.shouldShowSearchShortcuts
@ -356,3 +362,21 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen
}
}
}
/**
* Check whether search suggestions should be shown in the AwesomeBar.
*
* @param browsingMode Current browsing mode: normal or private.
* @param settings Persistence layer containing user option's for showing search suggestions.
*
* @return `true` if search suggestions should be shown `false` otherwise.
*/
@VisibleForTesting
internal fun shouldShowSearchSuggestions(
browsingMode: BrowsingMode,
settings: Settings,
) = when (browsingMode) {
BrowsingMode.Normal -> settings.shouldShowSearchSuggestions
BrowsingMode.Private ->
settings.shouldShowSearchSuggestions && settings.shouldShowSearchSuggestionsInPrivate
}

5
app/src/test/java/org/mozilla/fenix/search/SearchDialogControllerTest.kt
Unescape Escape View File

@ -45,6 +45,7 @@ import org.mozilla.fenix.GleanMetrics.SearchShortcuts
import org.mozilla.fenix.GleanMetrics.UnifiedSearch
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.components.Core
import org.mozilla.fenix.components.metrics.MetricsUtils
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@ -375,6 +376,8 @@ class SearchDialogControllerTest {
@Test
fun handleSearchShortcutEngineSelected() {
val searchEngine: SearchEngine = mockk(relaxed = true)
val browsingMode = BrowsingMode.Private
every { activity.browsingModeManager.mode } returns browsingMode
var focusToolbarInvoked = false
createController(
@ -384,7 +387,7 @@ class SearchDialogControllerTest {
).handleSearchShortcutEngineSelected(searchEngine)
assertTrue(focusToolbarInvoked)
verify { store.dispatch(SearchFragmentAction.SearchShortcutEngineSelected(searchEngine, settings)) }
verify { store.dispatch(SearchFragmentAction.SearchShortcutEngineSelected(searchEngine, browsingMode, settings)) }
assertNotNull(SearchShortcuts.selected.testGetValue())
val recordedEvents = SearchShortcuts.selected.testGetValue()!!

192
app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt
Unescape Escape View File

@ -8,6 +8,8 @@ import io.mockk.MockKAnnotations
import io.mockk.every
import io.mockk.impl.annotations.MockK
import io.mockk.mockk
import io.mockk.mockkStatic
import io.mockk.verify
import kotlinx.coroutines.test.runTest
import mozilla.components.browser.state.search.RegionState
import mozilla.components.browser.state.search.SearchEngine
@ -60,50 +62,54 @@ class SearchFragmentStoreTest {
every { settings.shouldShowSearchShortcuts } returns true
every { settings.showUnifiedSearchFeature } returns true
every { settings.shouldShowHistorySuggestions } returns true
val expected = SearchFragmentState(
query = "",
url = "",
searchTerms = "",
searchEngineSource = SearchEngineSource.None,
defaultEngine = null,
showSearchShortcutsSetting = true,
showSearchSuggestions = false,
showSearchSuggestionsHint = false,
showSearchShortcuts = false,
areShortcutsAvailable = false,
showClipboardSuggestions = false,
showSearchTermHistory = true,
showHistorySuggestionsForCurrentEngine = false,
showAllHistorySuggestions = true,
showBookmarkSuggestions = false,
showSyncedTabsSuggestions = false,
showSessionSuggestions = true,
tabId = null,
pastedText = "pastedText",
searchAccessPoint = MetricsUtils.Source.ACTION,
)
assertEquals(
expected,
createInitialSearchFragmentState(
activity,
components,
mockkStatic("org.mozilla.fenix.search.SearchFragmentStoreKt") {
val expected = SearchFragmentState(
query = "",
url = "",
searchTerms = "",
searchEngineSource = SearchEngineSource.None,
defaultEngine = null,
showSearchShortcutsSetting = true,
showSearchSuggestions = false,
showSearchSuggestionsHint = false,
showSearchShortcuts = false,
areShortcutsAvailable = false,
showClipboardSuggestions = false,
showSearchTermHistory = true,
showHistorySuggestionsForCurrentEngine = false,
showAllHistorySuggestions = true,
showBookmarkSuggestions = false,
showSyncedTabsSuggestions = false,
showSessionSuggestions = true,
tabId = null,
pastedText = "pastedText",
searchAccessPoint = MetricsUtils.Source.ACTION,
),
)
assertEquals(
expected.copy(tabId = "tabId"),
createInitialSearchFragmentState(
activity,
components,
tabId = "tabId",
pastedText = "pastedText",
searchAccessPoint = MetricsUtils.Source.ACTION,
),
)
)
assertEquals(
expected,
createInitialSearchFragmentState(
activity,
components,
tabId = null,
pastedText = "pastedText",
searchAccessPoint = MetricsUtils.Source.ACTION,
),
)
assertEquals(
expected.copy(tabId = "tabId"),
createInitialSearchFragmentState(
activity,
components,
tabId = "tabId",
pastedText = "pastedText",
searchAccessPoint = MetricsUtils.Source.ACTION,
),
)
verify(exactly = 2) { shouldShowSearchSuggestions(BrowsingMode.Normal, settings) }
}
}
@Test
@ -175,20 +181,31 @@ class SearchFragmentStoreTest {
every { settings.shouldShowHistorySuggestions } returns true
every { settings.shouldShowBookmarkSuggestions } returns false
every { settings.shouldShowSyncedTabsSuggestions } returns false
store.dispatch(SearchFragmentAction.SearchDefaultEngineSelected(searchEngine, settings)).join()
assertNotSame(initialState, store.state)
assertEquals(SearchEngineSource.Default(searchEngine), store.state.searchEngineSource)
assertTrue(store.state.showSearchSuggestions)
assertFalse(store.state.showSearchShortcuts)
assertTrue(store.state.showClipboardSuggestions)
assertFalse(store.state.showSearchTermHistory)
assertFalse(store.state.showHistorySuggestionsForCurrentEngine)
assertTrue(store.state.showAllHistorySuggestions)
assertFalse(store.state.showBookmarkSuggestions)
assertFalse(store.state.showSyncedTabsSuggestions)
assertTrue(store.state.showSessionSuggestions)
every { settings.shouldShowSearchSuggestions } returns true
every { settings.shouldShowSearchSuggestionsInPrivate } returns true
mockkStatic("org.mozilla.fenix.search.SearchFragmentStoreKt") {
store.dispatch(
SearchFragmentAction.SearchDefaultEngineSelected(
engine = searchEngine,
browsingMode = BrowsingMode.Private,
settings = settings,
),
).join()
assertNotSame(initialState, store.state)
assertEquals(SearchEngineSource.Default(searchEngine), store.state.searchEngineSource)
assertTrue(store.state.showSearchSuggestions)
assertFalse(store.state.showSearchShortcuts)
assertTrue(store.state.showClipboardSuggestions)
assertFalse(store.state.showSearchTermHistory)
assertFalse(store.state.showHistorySuggestionsForCurrentEngine)
assertTrue(store.state.showAllHistorySuggestions)
assertFalse(store.state.showBookmarkSuggestions)
assertFalse(store.state.showSyncedTabsSuggestions)
assertTrue(store.state.showSessionSuggestions)
verify { shouldShowSearchSuggestions(BrowsingMode.Private, settings) }
}
}
@Test
@ -202,8 +219,15 @@ class SearchFragmentStoreTest {
every { settings.shouldShowHistorySuggestions } returns true
every { settings.shouldShowBookmarkSuggestions } returns true
every { settings.shouldShowSyncedTabsSuggestions } returns true
every { settings.shouldShowSearchSuggestions } returns true
store.dispatch(SearchFragmentAction.SearchShortcutEngineSelected(searchEngine, settings)).join()
store.dispatch(
SearchFragmentAction.SearchShortcutEngineSelected(
engine = searchEngine,
browsingMode = BrowsingMode.Normal,
settings = settings,
),
).join()
assertNotSame(initialState, store.state)
assertEquals(SearchEngineSource.Shortcut(searchEngine), store.state.searchEngineSource)
@ -231,11 +255,17 @@ class SearchFragmentStoreTest {
every { settings.shouldShowBookmarkSuggestions } returns false
every { settings.shouldShowSyncedTabsSuggestions } returns false
store.dispatch(SearchFragmentAction.SearchShortcutEngineSelected(searchEngine, settings)).join()
store.dispatch(
SearchFragmentAction.SearchShortcutEngineSelected(
engine = searchEngine,
browsingMode = BrowsingMode.Normal,
settings = settings,
),
).join()
assertNotSame(initialState, store.state)
assertEquals(SearchEngineSource.Shortcut(searchEngine), store.state.searchEngineSource)
assertTrue(store.state.showSearchSuggestions)
assertFalse(store.state.showSearchSuggestions)
assertFalse(store.state.showSearchShortcuts)
assertFalse(store.state.showClipboardSuggestions)
assertTrue(store.state.showSearchTermHistory)
@ -256,8 +286,16 @@ class SearchFragmentStoreTest {
every { settings.shouldShowHistorySuggestions } returns true
every { settings.shouldShowBookmarkSuggestions } returns false
every { settings.shouldShowSyncedTabsSuggestions } returns true
every { settings.shouldShowSearchSuggestions } returns true
every { settings.shouldShowSearchSuggestionsInPrivate } returns true
store.dispatch(SearchFragmentAction.SearchShortcutEngineSelected(searchEngine, settings)).join()
store.dispatch(
SearchFragmentAction.SearchShortcutEngineSelected(
engine = searchEngine,
browsingMode = BrowsingMode.Private,
settings = settings,
),
).join()
assertNotSame(initialState, store.state)
assertEquals(SearchEngineSource.Shortcut(searchEngine), store.state.searchEngineSource)
@ -549,6 +587,42 @@ class SearchFragmentStoreTest {
assertFalse(store.state.showSearchShortcuts)
}
@Test
fun `GIVEN normal browsing mode and search suggestions enabled WHEN checking if search suggestions should be shown THEN return true`() {
var settings: Settings = mockk {
every { shouldShowSearchSuggestions } returns false
every { shouldShowSearchSuggestionsInPrivate } returns false
}
assertFalse(shouldShowSearchSuggestions(BrowsingMode.Normal, settings))
settings = mockk {
every { shouldShowSearchSuggestions } returns true
every { shouldShowSearchSuggestionsInPrivate } returns false
}
assertTrue(shouldShowSearchSuggestions(BrowsingMode.Normal, settings))
}
@Test
fun `GIVEN private browsing mode and search suggestions enabled WHEN checking if search suggestions should be shown THEN return true`() {
var settings: Settings = mockk {
every { shouldShowSearchSuggestions } returns false
every { shouldShowSearchSuggestionsInPrivate } returns false
}
assertFalse(shouldShowSearchSuggestions(BrowsingMode.Private, settings))
settings = mockk {
every { shouldShowSearchSuggestions } returns false
every { shouldShowSearchSuggestionsInPrivate } returns true
}
assertFalse(shouldShowSearchSuggestions(BrowsingMode.Private, settings))
settings = mockk {
every { shouldShowSearchSuggestions } returns true
every { shouldShowSearchSuggestionsInPrivate } returns true
}
assertTrue(shouldShowSearchSuggestions(BrowsingMode.Private, settings))
}
private fun emptyDefaultState(
searchEngineSource: SearchEngineSource = mockk(),
defaultEngine: SearchEngine? = mockk(),

5
app/src/test/java/org/mozilla/fenix/search/toolbar/SearchSelectorToolbarActionTest.kt
Unescape Escape View File

@ -34,6 +34,7 @@ import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.GleanMetrics.UnifiedSearch
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.components.metrics.MetricsUtils
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@ -111,6 +112,7 @@ class SearchSelectorToolbarActionTest {
store.dispatch(
SearchDefaultEngineSelected(
engine = testSearchEngine,
browsingMode = BrowsingMode.Normal,
settings = mockk(relaxed = true),
),
)
@ -141,6 +143,7 @@ class SearchSelectorToolbarActionTest {
store.dispatch(
SearchDefaultEngineSelected(
engine = testSearchEngine,
browsingMode = BrowsingMode.Private,
settings = mockk(relaxed = true),
),
)
@ -169,6 +172,7 @@ class SearchSelectorToolbarActionTest {
store.dispatch(
SearchDefaultEngineSelected(
engine = testSearchEngine,
browsingMode = BrowsingMode.Private,
settings = mockk(relaxed = true),
),
)
@ -185,6 +189,7 @@ class SearchSelectorToolbarActionTest {
store.dispatch(
SearchDefaultEngineSelected(
engine = testSearchEngine,
browsingMode = BrowsingMode.Normal,
settings = mockk(relaxed = true),
),
)

Write Preview
Loading…
Cancel
Save