[fenix] For https://github.com/mozilla-mobile/fenix/issues/25816: Show past search terms from the current search engine

For all search engines minus the application added ones we will also show past
search terms used for previous searches with the currently selected search
engine.
pull/600/head
Mugurell 2 years ago committed by mergify[bot]
parent 4c76307b08
commit 7c033fc8cd

@ -7746,6 +7746,23 @@ awesomebar:
metadata:
tags:
- Search
search_term_suggestion_clicked:
type: event
description: |
The search term suggestion in awesomebar was clicked.
bugs:
- https://github.com/mozilla-mobile/fenix/issues/25816
data_reviews:
- https://github.com/mozilla-mobile/fenix/pull/27810#issuecomment-1380720028
data_sensitivity:
- interaction
notification_emails:
- android-probes@mozilla.com
- kbrosnan@mozilla.com
expires: never
metadata:
tags:
- Search
android_autofill:
supported:
type: boolean

@ -246,6 +246,9 @@ internal class ReleaseMetricController(
Component.FEATURE_AWESOMEBAR to AwesomeBarFacts.Items.OPENED_TAB_SUGGESTION_CLICKED -> {
Awesomebar.openedTabSuggestionClicked.record(NoExtras())
}
Component.FEATURE_AWESOMEBAR to AwesomeBarFacts.Items.SEARCH_TERM_SUGGESTION_CLICKED -> {
Awesomebar.searchTermSuggestionClicked.record(NoExtras())
}
Component.FEATURE_CONTEXTMENU to ContextMenuFacts.Items.TEXT_SELECTION_OPTION -> {
when (metadata?.get("textSelectionOption")?.toString()) {
CONTEXT_MENU_COPY -> ContextualMenu.copyTapped.record(NoExtras())

@ -39,6 +39,10 @@ class SearchDialogInteractor(
searchController.handleSearchTermsTapped(searchTerms)
}
override fun onHistorySearchTermTapped(searchTerms: String) {
searchController.handleSearchTermsTapped(searchTerms)
}
override fun onSearchEngineSuggestionSelected(searchEngine: SearchEngine) {
searchController.handleSearchEngineSuggestionClicked(searchEngine)
}

@ -85,6 +85,8 @@ sealed class SearchEngineSource {
* @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 showSearchTermHistory Whether or not to show suggestions based on the previously used search terms
* with the currently selected search engine.
* @property showHistorySuggestionsForCurrentEngine Whether or not to show history suggestions for only
* the current search engine.
* @property showAllHistorySuggestions Whether or not to show history suggestions in the AwesomeBar
@ -106,6 +108,7 @@ data class SearchFragmentState(
val areShortcutsAvailable: Boolean,
val showSearchShortcutsSetting: Boolean,
val showClipboardSuggestions: Boolean,
val showSearchTermHistory: Boolean,
val showHistorySuggestionsForCurrentEngine: Boolean,
val showAllHistorySuggestions: Boolean,
val showBookmarkSuggestions: Boolean,
@ -157,6 +160,7 @@ fun createInitialSearchFragmentState(
areShortcutsAvailable = false,
showSearchShortcutsSetting = settings.shouldShowSearchShortcuts,
showClipboardSuggestions = settings.shouldShowClipboardSuggestions,
showSearchTermHistory = settings.showUnifiedSearchFeature && settings.shouldShowHistorySuggestions,
showHistorySuggestionsForCurrentEngine = false,
showAllHistorySuggestions = settings.shouldShowHistorySuggestions,
showBookmarkSuggestions = settings.shouldShowBookmarkSuggestions,
@ -241,6 +245,8 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen
showSearchSuggestions = action.settings.shouldShowSearchSuggestions,
showSearchShortcuts = action.settings.shouldShowSearchShortcuts,
showClipboardSuggestions = action.settings.shouldShowClipboardSuggestions,
showSearchTermHistory = action.settings.showUnifiedSearchFeature &&
action.settings.shouldShowHistorySuggestions,
showHistorySuggestionsForCurrentEngine = false, // we'll show all history
showAllHistorySuggestions = action.settings.shouldShowHistorySuggestions,
showBookmarkSuggestions = action.settings.shouldShowBookmarkSuggestions,
@ -256,6 +262,8 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen
false -> action.settings.shouldShowSearchShortcuts
},
showClipboardSuggestions = action.settings.shouldShowClipboardSuggestions,
showSearchTermHistory = action.settings.showUnifiedSearchFeature &&
action.settings.shouldShowHistorySuggestions,
showHistorySuggestionsForCurrentEngine = action.settings.showUnifiedSearchFeature &&
action.settings.shouldShowHistorySuggestions && !action.engine.isGeneral,
showAllHistorySuggestions = when (action.settings.showUnifiedSearchFeature) {
@ -281,6 +289,7 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen
showSearchSuggestions = false,
showSearchShortcuts = false,
showClipboardSuggestions = false,
showSearchTermHistory = false,
showHistorySuggestionsForCurrentEngine = false,
showAllHistorySuggestions = true,
showBookmarkSuggestions = false,
@ -293,6 +302,7 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen
showSearchSuggestions = false,
showSearchShortcuts = false,
showClipboardSuggestions = false,
showSearchTermHistory = false,
showHistorySuggestionsForCurrentEngine = false,
showAllHistorySuggestions = false,
showBookmarkSuggestions = true,
@ -305,6 +315,7 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen
showSearchSuggestions = false,
showSearchShortcuts = false,
showClipboardSuggestions = false,
showSearchTermHistory = false,
showHistorySuggestionsForCurrentEngine = false,
showAllHistorySuggestions = false,
showBookmarkSuggestions = false,

@ -25,6 +25,12 @@ interface AwesomeBarInteractor {
*/
fun onSearchTermsTapped(searchTerms: String)
/**
* Called whenever a suggestion for a previously used search term is tapped.
* @param searchTerms the query contained by the search suggestion.
*/
fun onHistorySearchTermTapped(searchTerms: String)
/**
* Called whenever a search engine shortcut is tapped
* @param searchEngine the searchEngine that was selected

@ -23,6 +23,7 @@ import mozilla.components.feature.awesomebar.provider.HistoryStorageSuggestionPr
import mozilla.components.feature.awesomebar.provider.SearchActionProvider
import mozilla.components.feature.awesomebar.provider.SearchEngineSuggestionProvider
import mozilla.components.feature.awesomebar.provider.SearchSuggestionProvider
import mozilla.components.feature.awesomebar.provider.SearchTermSuggestionsProvider
import mozilla.components.feature.awesomebar.provider.SessionSuggestionProvider
import mozilla.components.feature.search.SearchUseCases
import mozilla.components.feature.session.SessionUseCases
@ -84,6 +85,16 @@ class AwesomeBarView(
}
}
private val historySearchTermUseCase = object : SearchUseCases.SearchUseCase {
override fun invoke(
searchTerms: String,
searchEngine: SearchEngine?,
parentSessionId: String?,
) {
interactor.onSearchTermsTapped(searchTerms)
}
}
private val shortcutSearchUseCase = object : SearchUseCases.SearchUseCase {
override fun invoke(
searchTerms: String,
@ -277,6 +288,12 @@ class AwesomeBarView(
}
}
if (state.showSearchTermHistory) {
getSearchTermSuggestionsProvider(state.searchEngineSource)?.let {
providersToAdd.add(it)
}
}
if (state.showAllHistorySuggestions) {
if (activity.settings().historyMetadataUIFeature) {
providersToAdd.add(defaultCombinedHistoryProvider)
@ -370,6 +387,22 @@ class AwesomeBarView(
}
}
@VisibleForTesting
internal fun getSearchTermSuggestionsProvider(
searchEngineSource: SearchEngineSource,
): AwesomeBar.SuggestionProvider? {
val validSearchEngine = searchEngineSource.searchEngine ?: return null
return SearchTermSuggestionsProvider(
historyStorage = components.core.historyStorage,
searchUseCase = historySearchTermUseCase,
searchEngine = validSearchEngine,
icon = getDrawable(activity, R.drawable.ic_history)?.toBitmap(),
engine = engineForSpeculativeConnects,
suggestionsHeader = getSearchEngineSuggestionsHeader(),
)
}
private fun handleDisplayShortcutsProviders() {
view.addProviders(shortcutsEnginePickerProvider)
}
@ -419,6 +452,7 @@ class AwesomeBarView(
data class SearchProviderState(
val showSearchShortcuts: Boolean,
val showSearchTermHistory: Boolean,
val showHistorySuggestionsForCurrentEngine: Boolean,
val showAllHistorySuggestions: Boolean,
val showBookmarkSuggestions: Boolean,
@ -443,6 +477,7 @@ class AwesomeBarView(
fun SearchFragmentState.toSearchProviderState() = AwesomeBarView.SearchProviderState(
showSearchShortcuts,
showSearchTermHistory,
showHistorySuggestionsForCurrentEngine,
showAllHistorySuggestions,
showBookmarkSuggestions,

@ -702,6 +702,16 @@ class MetricControllerTest {
}
assertNotNull(Awesomebar.openedTabSuggestionClicked.testGetValue())
// Verify a search term suggestion clicked
assertNull(Awesomebar.searchTermSuggestionClicked.testGetValue())
fact = Fact(Component.FEATURE_AWESOMEBAR, Action.CANCEL, AwesomeBarFacts.Items.SEARCH_TERM_SUGGESTION_CLICKED)
with(controller) {
fact.process()
}
assertNotNull(Awesomebar.searchTermSuggestionClicked.testGetValue())
}
@Test

@ -69,6 +69,14 @@ class SearchDialogInteractorTest {
}
}
@Test
fun `WHEN the search term from history is tapped THEN delegate the event to the controller`() {
interactor.onHistorySearchTermTapped("test")
verify {
searchController.handleSearchTermsTapped("test")
}
}
@Test
fun onSearchShortcutEngineSelected() {
val searchEngine: SearchEngine = mockk(relaxed = true)

@ -73,6 +73,7 @@ class SearchFragmentStoreTest {
showSearchShortcuts = false,
areShortcutsAvailable = false,
showClipboardSuggestions = false,
showSearchTermHistory = true,
showHistorySuggestionsForCurrentEngine = false,
showAllHistorySuggestions = true,
showBookmarkSuggestions = false,
@ -133,6 +134,7 @@ class SearchFragmentStoreTest {
showSearchShortcuts = false,
areShortcutsAvailable = false,
showClipboardSuggestions = false,
showSearchTermHistory = false,
showHistorySuggestionsForCurrentEngine = false,
showAllHistorySuggestions = false,
showBookmarkSuggestions = false,
@ -181,6 +183,7 @@ class SearchFragmentStoreTest {
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)
@ -207,6 +210,7 @@ class SearchFragmentStoreTest {
assertTrue(store.state.showSearchSuggestions)
assertFalse(store.state.showSearchShortcuts)
assertTrue(store.state.showClipboardSuggestions)
assertTrue(store.state.showSearchTermHistory)
assertFalse(store.state.showHistorySuggestionsForCurrentEngine)
assertFalse(store.state.showAllHistorySuggestions)
assertFalse(store.state.showBookmarkSuggestions)
@ -234,6 +238,7 @@ class SearchFragmentStoreTest {
assertTrue(store.state.showSearchSuggestions)
assertFalse(store.state.showSearchShortcuts)
assertFalse(store.state.showClipboardSuggestions)
assertTrue(store.state.showSearchTermHistory)
assertTrue(store.state.showHistorySuggestionsForCurrentEngine)
assertFalse(store.state.showAllHistorySuggestions)
assertFalse(store.state.showBookmarkSuggestions)
@ -259,6 +264,7 @@ class SearchFragmentStoreTest {
assertTrue(store.state.showSearchSuggestions)
assertTrue(store.state.showSearchShortcuts)
assertFalse(store.state.showClipboardSuggestions)
assertFalse(store.state.showSearchTermHistory)
assertFalse(store.state.showHistorySuggestionsForCurrentEngine)
assertTrue(store.state.showAllHistorySuggestions)
assertFalse(store.state.showBookmarkSuggestions)
@ -278,6 +284,7 @@ class SearchFragmentStoreTest {
assertFalse(store.state.showSearchSuggestions)
assertFalse(store.state.showSearchShortcuts)
assertFalse(store.state.showClipboardSuggestions)
assertFalse(store.state.showSearchTermHistory)
assertFalse(store.state.showHistorySuggestionsForCurrentEngine)
assertTrue(store.state.showAllHistorySuggestions)
assertFalse(store.state.showBookmarkSuggestions)
@ -297,6 +304,7 @@ class SearchFragmentStoreTest {
assertFalse(store.state.showSearchSuggestions)
assertFalse(store.state.showSearchShortcuts)
assertFalse(store.state.showClipboardSuggestions)
assertFalse(store.state.showSearchTermHistory)
assertFalse(store.state.showHistorySuggestionsForCurrentEngine)
assertFalse(store.state.showAllHistorySuggestions)
assertTrue(store.state.showBookmarkSuggestions)
@ -316,6 +324,7 @@ class SearchFragmentStoreTest {
assertFalse(store.state.showSearchSuggestions)
assertFalse(store.state.showSearchShortcuts)
assertFalse(store.state.showClipboardSuggestions)
assertFalse(store.state.showSearchTermHistory)
assertFalse(store.state.showHistorySuggestionsForCurrentEngine)
assertFalse(store.state.showAllHistorySuggestions)
assertFalse(store.state.showBookmarkSuggestions)
@ -559,6 +568,7 @@ class SearchFragmentStoreTest {
showSearchShortcuts = false,
areShortcutsAvailable = areShortcutsAvailable,
showClipboardSuggestions = false,
showSearchTermHistory = true,
showHistorySuggestionsForCurrentEngine = showHistorySuggestionsForCurrentEngine,
showAllHistorySuggestions = false,
showBookmarkSuggestions = false,

@ -18,6 +18,7 @@ import mozilla.components.feature.awesomebar.provider.HistoryStorageSuggestionPr
import mozilla.components.feature.awesomebar.provider.SearchActionProvider
import mozilla.components.feature.awesomebar.provider.SearchEngineSuggestionProvider
import mozilla.components.feature.awesomebar.provider.SearchSuggestionProvider
import mozilla.components.feature.awesomebar.provider.SearchTermSuggestionsProvider
import mozilla.components.feature.awesomebar.provider.SessionSuggestionProvider
import mozilla.components.feature.syncedtabs.SyncedTabsStorageSuggestionProvider
import mozilla.components.support.ktx.android.content.getColorFromAttr
@ -58,6 +59,7 @@ class AwesomeBarViewTest {
every { any<Activity>().components.core.client } returns mockk()
every { any<Activity>().components.backgroundServices.syncedTabsStorage } returns mockk()
every { any<Activity>().components.core.store.state.search } returns mockk(relaxed = true)
every { any<Activity>().components.core.store.state.search } returns mockk(relaxed = true)
every { any<Activity>().getColorFromAttr(any()) } returns 0
every { AwesomeBarView.Companion.getDrawable(any(), any()) } returns mockk<VectorDrawable>(relaxed = true) {
every { intrinsicWidth } returns 10
@ -413,6 +415,48 @@ class AwesomeBarViewTest {
assertNotNull((result as HistoryStorageSuggestionProvider).resultsHostFilter)
assertEquals(AwesomeBarView.METADATA_SUGGESTION_LIMIT, result.getMaxNumberOfSuggestions())
}
@Test
fun `GIVEN a search engine is not available WHEN asking for a search term provider THEN return null`() {
val searchEngineSource: SearchEngineSource = SearchEngineSource.None
val result = awesomeBarView.getSearchTermSuggestionsProvider(searchEngineSource)
assertNull(result)
}
@Test
fun `GIVEN a search engine is available WHEN asking for a search term provider THEN return a valid provider`() {
val searchEngineSource = SearchEngineSource.Default(mockk())
val result = awesomeBarView.getSearchTermSuggestionsProvider(searchEngineSource)
assertTrue(result is SearchTermSuggestionsProvider)
}
@Test
fun `GIVEN history search term suggestions disabled WHEN getting suggestions providers THEN don't search term provider of past searches`() {
every { activity.settings() } returns mockk(relaxed = true)
val state = getSearchProviderState(
searchEngineSource = SearchEngineSource.Default(mockk(relaxed = true)),
showSearchTermHistory = false,
)
val result = awesomeBarView.getProvidersToAdd(state)
assertEquals(0, result.filterIsInstance<SearchTermSuggestionsProvider>().size)
}
@Test
fun `GIVEN history search term suggestions enabled WHEN getting suggestions providers THEN add a search term provider of past searches`() {
every { activity.settings() } returns mockk(relaxed = true)
val state = getSearchProviderState(
searchEngineSource = SearchEngineSource.Default(mockk(relaxed = true)),
showSearchTermHistory = true,
)
val result = awesomeBarView.getProvidersToAdd(state)
assertEquals(1, result.filterIsInstance<SearchTermSuggestionsProvider>().size)
}
}
/**
@ -420,6 +464,7 @@ class AwesomeBarViewTest {
*/
private fun getSearchProviderState(
showSearchShortcuts: Boolean = true,
showSearchTermHistory: Boolean = true,
showHistorySuggestionsForCurrentEngine: Boolean = true,
showAllHistorySuggestions: Boolean = true,
showBookmarkSuggestions: Boolean = true,
@ -429,6 +474,7 @@ private fun getSearchProviderState(
searchEngineSource: SearchEngineSource = SearchEngineSource.None,
) = SearchProviderState(
showSearchShortcuts = showSearchShortcuts,
showSearchTermHistory = showSearchTermHistory,
showHistorySuggestionsForCurrentEngine = showHistorySuggestionsForCurrentEngine,
showAllHistorySuggestions = showAllHistorySuggestions,
showBookmarkSuggestions = showBookmarkSuggestions,

@ -253,6 +253,7 @@ private val testSearchFragmentState = SearchFragmentState(
searchTerms = "search terms",
searchEngineSource = SearchEngineSource.None,
defaultEngine = null,
showSearchTermHistory = true,
showSearchSuggestions = false,
showSearchShortcutsSetting = false,
showSearchSuggestionsHint = false,

@ -57,6 +57,7 @@ class ToolbarViewTest {
},
),
defaultEngine = null,
showSearchTermHistory = true,
showSearchShortcutsSetting = false,
showSearchSuggestionsHint = false,
showSearchSuggestions = false,

Loading…
Cancel
Save