From 21dda6b6ab1884d6e35185bbf80b9ccdc0c7df74 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Tue, 6 Jul 2021 16:19:34 -0700 Subject: [PATCH] [fenix] Closes https://github.com/mozilla-mobile/fenix/issues/20276: Fall-back to URL parsing for tabs with parents to obtain search terms --- .../HistoryMetadataMiddleware.kt | 4 +- .../HistoryMetadataMiddlewareTest.kt | 77 ++++++++++--------- 2 files changed, 42 insertions(+), 39 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddleware.kt b/app/src/main/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddleware.kt index fe936688d5..423c606149 100644 --- a/app/src/main/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddleware.kt +++ b/app/src/main/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddleware.kt @@ -116,7 +116,9 @@ class HistoryMetadataMiddleware( // Obtain search terms and referrer url either from tab parent, or from the history stack. val (searchTerm, referrerUrl) = when { tabParent != null -> { - tabParent.content.searchTerms.takeUnless { it.isEmpty() } to tabParent.content.url + val searchTerms = tabParent.content.searchTerms.takeUnless { it.isEmpty() } + ?: context.state.search.parseSearchTerms(tabParent.content.url) + searchTerms to tabParent.content.url } previousUrlIndex >= 0 -> { val previousUrl = tab.content.history.items[previousUrlIndex].uri diff --git a/app/src/test/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddlewareTest.kt b/app/src/test/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddlewareTest.kt index 735db84388..8105a1df24 100644 --- a/app/src/test/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddlewareTest.kt +++ b/app/src/test/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddlewareTest.kt @@ -92,6 +92,21 @@ class HistoryMetadataMiddlewareTest { } } + @Test + fun `GIVEN normal tab has search results parent without search terms WHEN history metadata is recorded THEN search terms and referrer url are provided`() { + setupGoogleSearchEngine() + + val parentTab = createTab("https://google.com?q=mozilla+website") + val tab = createTab("https://mozilla.org", parent = parentTab) + store.dispatch(TabListAction.AddTabAction(parentTab)).joinBlocking() + store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() + + store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, emptyList(), currentIndex = 0)).joinBlocking() + verify { + service.createMetadata(any(), eq("mozilla website"), eq("https://google.com?q=mozilla+website")) + } + } + @Test fun `GIVEN normal tab has parent WHEN url is the same THEN nothing happens`() { val parentTab = createTab("https://mozilla.org", searchTerms = "mozilla website") @@ -107,25 +122,7 @@ class HistoryMetadataMiddlewareTest { fun `GIVEN normal tab has no parent WHEN history metadata is recorded THEN search terms and referrer url are provided`() { val tab = createTab("https://mozilla.org") store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() - store.dispatch(SearchAction.SetSearchEnginesAction( - regionSearchEngines = listOf( - SearchEngine( - id = "google", - name = "Google", - icon = mock(), - type = SearchEngine.Type.BUNDLED, - resultUrls = listOf("https://google.com?q={searchTerms}") - ) - ), - userSelectedSearchEngineId = null, - userSelectedSearchEngineName = null, - regionDefaultSearchEngineId = "google", - customSearchEngines = emptyList(), - hiddenSearchEngines = emptyList(), - additionalAvailableSearchEngines = emptyList(), - additionalSearchEngines = emptyList(), - regionSearchEnginesOrder = listOf("google") - )).joinBlocking() + setupGoogleSearchEngine() val historyState = listOf( HistoryItem("firefox", "https://google.com?q=mozilla+website"), @@ -142,25 +139,7 @@ class HistoryMetadataMiddlewareTest { fun `GIVEN normal tab has no parent WHEN history metadata is recorded without search terms THEN no referrer is provided`() { val tab = createTab("https://mozilla.org") store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() - store.dispatch(SearchAction.SetSearchEnginesAction( - regionSearchEngines = listOf( - SearchEngine( - id = "google", - name = "Google", - icon = mock(), - type = SearchEngine.Type.BUNDLED, - resultUrls = listOf("https://google.com?q={searchTerms}") - ) - ), - userSelectedSearchEngineId = null, - userSelectedSearchEngineName = null, - regionDefaultSearchEngineId = "google", - customSearchEngines = emptyList(), - hiddenSearchEngines = emptyList(), - additionalAvailableSearchEngines = emptyList(), - additionalSearchEngines = emptyList(), - regionSearchEnginesOrder = listOf("google") - )).joinBlocking() + setupGoogleSearchEngine() val historyState = listOf( HistoryItem("firefox", "https://mozilla.org"), @@ -410,4 +389,26 @@ class HistoryMetadataMiddlewareTest { store.dispatch(TabListAction.RemoveTabsAction(listOf(otherTab.id, yetAnotherTab.id))).joinBlocking() verify { service wasNot Called } } + + private fun setupGoogleSearchEngine() { + store.dispatch(SearchAction.SetSearchEnginesAction( + regionSearchEngines = listOf( + SearchEngine( + id = "google", + name = "Google", + icon = mock(), + type = SearchEngine.Type.BUNDLED, + resultUrls = listOf("https://google.com?q={searchTerms}") + ) + ), + userSelectedSearchEngineId = null, + userSelectedSearchEngineName = null, + regionDefaultSearchEngineId = "google", + customSearchEngines = emptyList(), + hiddenSearchEngines = emptyList(), + additionalAvailableSearchEngines = emptyList(), + additionalSearchEngines = emptyList(), + regionSearchEnginesOrder = listOf("google") + )).joinBlocking() + } }