From e7b5bc672f489177680bf9ad9da9e46fafd49bb8 Mon Sep 17 00:00:00 2001 From: Lina Butler Date: Fri, 27 Oct 2023 17:06:21 -0700 Subject: [PATCH] Bug 1861416 - Don't show sponsored or non-sponsored suggestions in private mode. In Firefox Desktop, sponsored and non-sponsored suggestions aren't shown in private windows, even when the "Show search suggestions in Private Windows" option is checked. This commit makes Fenix's behavior match Desktop's. (cherry picked from commit 1258f6cb2fc1acf0804365b87fd9145e7309b6f5) --- .../fenix/search/SearchFragmentStore.kt | 16 ++- .../fenix/search/SearchFragmentStoreTest.kt | 107 ++++++++++++++++-- 2 files changed, 110 insertions(+), 13 deletions(-) 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 e9fc01223..f1f45ba8c 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt @@ -101,9 +101,9 @@ sealed class SearchEngineSource { * the current search engine. * @property showAllSessionSuggestions Whether or not to show the session suggestion in the AwesomeBar. * @property showSponsoredSuggestions Whether or not to show sponsored Firefox Suggest search suggestions in the - * AwesomeBar. Always `false` when a non-default engine is selected. + * AwesomeBar. Always `false` in private mode, or when a non-default engine is selected. * @property showNonSponsoredSuggestions Whether or not to show Firefox Suggest search suggestions for web content - * in the AwesomeBar. Always `false` when a non-default engine is selected. + * in the AwesomeBar. Always `false` in private mode, or when a non-default engine is selected. * @property tabId The ID of the current tab. * @property pastedText The text pasted from the long press toolbar menu. * @property searchAccessPoint The source of the performed search. @@ -184,8 +184,10 @@ fun createInitialSearchFragmentState( showAllSyncedTabsSuggestions = settings.shouldShowSyncedTabsSuggestions, showSessionSuggestionsForCurrentEngine = false, showAllSessionSuggestions = true, - showSponsoredSuggestions = settings.showSponsoredSuggestions, - showNonSponsoredSuggestions = settings.showNonSponsoredSuggestions, + showSponsoredSuggestions = activity.browsingModeManager.mode == BrowsingMode.Normal && + settings.showSponsoredSuggestions, + showNonSponsoredSuggestions = activity.browsingModeManager.mode == BrowsingMode.Normal && + settings.showNonSponsoredSuggestions, tabId = tabId, pastedText = pastedText, searchAccessPoint = searchAccessPoint, @@ -283,8 +285,10 @@ private fun searchStateReducer(state: SearchFragmentState, action: SearchFragmen showSyncedTabsSuggestionsForCurrentEngine = false, // we'll show all synced tabs showAllSyncedTabsSuggestions = action.settings.shouldShowSyncedTabsSuggestions, showSessionSuggestionsForCurrentEngine = false, // we'll show all local tabs - showSponsoredSuggestions = action.settings.showSponsoredSuggestions, - showNonSponsoredSuggestions = action.settings.showNonSponsoredSuggestions, + showSponsoredSuggestions = action.browsingMode == BrowsingMode.Normal && + action.settings.showSponsoredSuggestions, + showNonSponsoredSuggestions = action.browsingMode == BrowsingMode.Normal && + action.settings.showNonSponsoredSuggestions, showAllSessionSuggestions = true, ) is SearchFragmentAction.SearchShortcutEngineSelected -> 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 6c5c81c83..4a6f59b4b 100644 --- a/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/SearchFragmentStoreTest.kt @@ -56,12 +56,14 @@ class SearchFragmentStoreTest { } @Test - fun `createInitialSearchFragmentState with no tab`() { + fun `createInitialSearchFragmentState with no tab in normal browsing mode`() { activity.browsingModeManager.mode = BrowsingMode.Normal every { components.core.store.state } returns BrowserState() every { settings.shouldShowSearchShortcuts } returns true every { settings.showUnifiedSearchFeature } returns true every { settings.shouldShowHistorySuggestions } returns true + every { settings.shouldShowSearchSuggestions } returns true + every { settings.shouldShowSearchSuggestionsInPrivate } returns false every { settings.showSponsoredSuggestions } returns true every { settings.showNonSponsoredSuggestions } returns true @@ -73,7 +75,7 @@ class SearchFragmentStoreTest { searchEngineSource = SearchEngineSource.None, defaultEngine = null, showSearchShortcutsSetting = true, - showSearchSuggestions = false, + showSearchSuggestions = true, showSearchSuggestionsHint = false, showSearchShortcuts = false, areShortcutsAvailable = false, @@ -119,6 +121,58 @@ class SearchFragmentStoreTest { } } + @Test + fun `createInitialSearchFragmentState with no tab in private browsing mode`() { + activity.browsingModeManager.mode = BrowsingMode.Private + every { components.core.store.state } returns BrowserState() + every { settings.shouldShowSearchShortcuts } returns true + every { settings.showUnifiedSearchFeature } returns true + every { settings.shouldShowHistorySuggestions } returns true + every { settings.shouldShowSearchSuggestions } returns true + every { settings.shouldShowSearchSuggestionsInPrivate } returns false + every { settings.showSponsoredSuggestions } returns true + every { settings.showNonSponsoredSuggestions } 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, + showBookmarksSuggestionsForCurrentEngine = false, + showAllBookmarkSuggestions = false, + showSyncedTabsSuggestionsForCurrentEngine = false, + showAllSyncedTabsSuggestions = false, + showSessionSuggestionsForCurrentEngine = false, + showAllSessionSuggestions = true, + showSponsoredSuggestions = false, + showNonSponsoredSuggestions = false, + tabId = null, + pastedText = "pastedText", + searchAccessPoint = MetricsUtils.Source.ACTION, + ) + + assertEquals( + expected, + createInitialSearchFragmentState( + activity, + components, + tabId = null, + pastedText = "pastedText", + searchAccessPoint = MetricsUtils.Source.ACTION, + ), + ) + } + @Test fun `createInitialSearchFragmentState with tab`() { activity.browsingModeManager.mode = BrowsingMode.Private @@ -504,7 +558,7 @@ class SearchFragmentStoreTest { } @Test - fun `WHEN the search engine is the default one THEN search suggestions providers are updated`() = runTest { + fun `GIVEN private browsing mode WHEN the search engine is the default one THEN search suggestions providers are updated`() = runTest { val initialState = emptyDefaultState(showHistorySuggestionsForCurrentEngine = false) val store = SearchFragmentStore(initialState) every { settings.shouldShowSearchShortcuts } returns false @@ -539,12 +593,51 @@ class SearchFragmentStoreTest { assertFalse(store.state.showAllBookmarkSuggestions) assertFalse(store.state.showAllSyncedTabsSuggestions) assertTrue(store.state.showAllSessionSuggestions) - assertTrue(store.state.showSponsoredSuggestions) - assertTrue(store.state.showNonSponsoredSuggestions) + assertFalse(store.state.showSponsoredSuggestions) + assertFalse(store.state.showNonSponsoredSuggestions) verify { shouldShowSearchSuggestions(BrowsingMode.Private, settings) } } } + @Test + fun `GIVEN normal browsing mode WHEN the search engine is the default one THEN search suggestions providers are updated`() = runTest { + val initialState = emptyDefaultState(showHistorySuggestionsForCurrentEngine = false) + val store = SearchFragmentStore(initialState) + every { settings.shouldShowSearchShortcuts } returns false + every { settings.shouldShowSearchSuggestions } returns true + every { settings.shouldShowClipboardSuggestions } returns true + every { settings.shouldShowHistorySuggestions } returns true + every { settings.shouldShowBookmarkSuggestions } returns false + every { settings.shouldShowSyncedTabsSuggestions } returns false + every { settings.shouldShowSearchSuggestions } returns true + every { settings.shouldShowSearchSuggestionsInPrivate } returns true + every { settings.showSponsoredSuggestions } returns true + every { settings.showNonSponsoredSuggestions } returns true + + store.dispatch( + SearchFragmentAction.SearchDefaultEngineSelected( + engine = searchEngine, + browsingMode = BrowsingMode.Normal, + 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.showAllBookmarkSuggestions) + assertFalse(store.state.showAllSyncedTabsSuggestions) + assertTrue(store.state.showAllSessionSuggestions) + assertTrue(store.state.showSponsoredSuggestions) + assertTrue(store.state.showNonSponsoredSuggestions) + } + @Test fun `GIVEN unified search is enabled WHEN the search engine is updated to a general engine shortcut THEN search suggestions providers are updated`() = runTest { val initialState = emptyDefaultState(showHistorySuggestionsForCurrentEngine = false) @@ -1078,7 +1171,7 @@ class SearchFragmentStoreTest { } @Test - fun `GIVEN normal browsing mode and search suggestions enabled WHEN checking if search suggedtions should be shown THEN return true`() { + 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 @@ -1093,7 +1186,7 @@ class SearchFragmentStoreTest { } @Test - fun `GIVEN private browsing mode and search suggestions enabled WHEN checking if search suggedtions should be shown THEN return true`() { + 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