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

Closes #21659: Add SERPs to history search groups

Browse Source
upstream-sync
Christian Sadilek 3 years ago
parent 8b8fb4f00d
commit 76bb0c3b37
2 changed files with 91 additions and 30 deletions
Show Stats Download Patch File Download Diff File

60
app/src/main/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddleware.kt
Unescape Escape View File

@ -14,6 +14,7 @@ import mozilla.components.browser.state.selector.findNormalTab
import mozilla.components.browser.state.selector.findTab
import mozilla.components.browser.state.selector.selectedNormalTab
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.SearchState
import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.feature.search.ext.parseSearchTerms
import mozilla.components.lib.state.Middleware
@ -141,40 +142,48 @@ class HistoryMetadataMiddleware(
}
}
private fun createHistoryMetadata(context: MiddlewareContext<BrowserState, BrowserAction>, tab: TabSessionState) {
@Suppress("ComplexMethod")
private fun createHistoryMetadata(
context: MiddlewareContext<BrowserState, BrowserAction>,
tab: TabSessionState
) {
val tabParent = tab.getParent(context.store)
val previousUrlIndex = tab.content.history.currentIndex - 1
val tabMetadataHasSearchTerms = !tab.historyMetadata?.searchTerm.isNullOrBlank()
// Obtain search terms and referrer url either from tab parent, from the history stack, or
// from the tab itself.
// At a high level, there are two main cases here - 1) either the tab was opened as a 'new tab'
// via the search results page, or 2) a page was opened in the same tab as the search results page.
// Details about the New Tab case:
// - we obtain search terms via tab's parent (the search results page)
// - however, it's possible that parent changed (e.g. user navigated away from the search
// results page).
// - our approach below is to capture search terms from the parent within the tab.historyMetadata
// state on the first load of the tab, and then rely on this data for subsequent page loads on that tab.
// - this way, once a tab becomes part of the search group, it won't leave this search group
// unless a direct navigation event happens.
//
// At a high level, there are two main cases here:
// 1) The tab was opened as a 'new tab' via the search engine results page (SERP). In this
// case we obtain search terms via the tab's parent (the search results page). However, it's
// possible that the parent changed (e.g. user navigated away from the search results page).
// Our approach below is to capture search terms from the parent within the
// tab.historyMetadata state on the first load of the tab, and then rely on this data for
// subsequent page loads on that tab. This way, once a tab becomes part of the search group,
// it won't leave this group unless a direct navigation event happens.
//
// 2) A page was opened in the same tab as the search results page (navigated to via content).
val (searchTerm, referrerUrl) = when {
// Loading page opened in a New Tab for the first time.
// Page was opened in a new tab. Look for search terms in the parent tab.
tabParent != null && !tabMetadataHasSearchTerms -> {
val searchTerms = tabParent.content.searchTerms.takeUnless { it.isEmpty() }
?: context.state.search.parseSearchTerms(tabParent.content.url)
val searchTerms = findSearchTerms(tabParent, context.state.search)
searchTerms to tabParent.content.url
}
// We only want to inspect the previous url in history if the user navigated via
// web content i.e., they followed a link, not if the user navigated directly via
// toolbar.
// Page was navigated to via content i.e., the user followed a link. Look for search terms in tab history.
!directLoadTriggered && previousUrlIndex >= 0 -> {
// Once a tab is within the search group, only a direct load event (via the toolbar) can change that.
val previousUrl = tab.content.history.items[previousUrlIndex].uri
val (searchTerms, referrerUrl) = if (tabMetadataHasSearchTerms) {
tab.historyMetadata?.searchTerm to tab.historyMetadata?.referrerUrl
tab.historyMetadata?.searchTerm to previousUrl
} else {
val previousUrl = tab.content.history.items[previousUrlIndex].uri
context.state.search.parseSearchTerms(previousUrl) to previousUrl
// Find search terms by checking if page is a SERP or a result opened from a SERP
val searchTerms = findSearchTerms(tab, context.state.search)
if (searchTerms != null) {
searchTerms to null
} else {
context.state.search.parseSearchTerms(previousUrl) to previousUrl
}
}
if (searchTerms != null) {
@ -190,11 +199,8 @@ class HistoryMetadataMiddleware(
tabMetadataHasSearchTerms && !(directLoadTriggered && previousUrlIndex >= 0) -> {
tab.historyMetadata?.searchTerm to tab.historyMetadata?.referrerUrl
}
// We had no search terms, no history stack, and no parent.
// This would be the case for any page loaded directly via the toolbar including
// a search results page itself. For now, the original search results page is not
// part of the search group: https://github.com/mozilla-mobile/fenix/issues/21659.
else -> null to null
// In all other cases (e.g. direct load) find search terms by checking if page is a SERP
else -> findSearchTerms(tab, context.state.search) to null
}
// Sanity check to make sure we don't record a metadata record referring to itself.
@ -218,4 +224,8 @@ class HistoryMetadataMiddleware(
store.state.findTab(it)
}
}
private fun findSearchTerms(tab: TabSessionState, searchState: SearchState): String? {
return tab.content.searchTerms.takeUnless { it.isEmpty() } ?: searchState.parseSearchTerms(tab.content.url)
}
}

61
app/src/test/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddlewareTest.kt
Unescape Escape View File

@ -114,6 +114,55 @@ class HistoryMetadataMiddlewareTest {
}
}
@Test
fun `GIVEN normal tab is a search engine result page WHEN history metadata is recorded THEN search terms are provided`() {
service = TestingMetadataService()
middleware = HistoryMetadataMiddleware(service)
val tab = createTab("about:blank")
store = BrowserStore(
middleware = listOf(middleware) + EngineMiddleware.create(engine = mockk()),
initialState = BrowserState(
tabs = listOf(tab)
)
)
setupGoogleSearchEngine()
val serpUrl = "https://google.com?q=mozilla+website"
store.dispatch(EngineAction.LoadUrlAction(tab.id, serpUrl)).joinBlocking()
store.dispatch(ContentAction.UpdateUrlAction(tab.id, serpUrl)).joinBlocking()
store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, listOf(HistoryItem("Google Search", serpUrl)), currentIndex = 0)).joinBlocking()
with((service as TestingMetadataService).createdMetadata) {
assertEquals(1, this.count())
assertEquals("https://google.com?q=mozilla+website", this[0].url)
assertEquals("mozilla website", this[0].searchTerm)
assertNull(this[0].referrerUrl)
}
}
@Test
fun `GIVEN normal tab navigates to search engine result page WHEN history metadata is recorded THEN search terms are provided`() {
service = TestingMetadataService()
middleware = HistoryMetadataMiddleware(service)
val tab = createTab("https://google.com")
store = BrowserStore(
middleware = listOf(middleware) + EngineMiddleware.create(engine = mockk()),
initialState = BrowserState(
tabs = listOf(tab)
)
)
setupGoogleSearchEngine()
val serpUrl = "https://google.com?q=mozilla+website"
store.dispatch(ContentAction.UpdateUrlAction(tab.id, serpUrl)).joinBlocking()
store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, listOf(HistoryItem("Google Search", "https://google.com"), HistoryItem("Google Search", serpUrl)), currentIndex = 1)).joinBlocking()
with((service as TestingMetadataService).createdMetadata) {
assertEquals(1, this.count())
assertEquals("https://google.com?q=mozilla+website", this[0].url)
assertEquals("mozilla website", this[0].searchTerm)
assertNull(this[0].referrerUrl)
}
}
@Test
fun `GIVEN tab opened as new tab from a search page WHEN search page navigates away THEN redirecting or navigating in tab retains original search terms`() {
service = TestingMetadataService()
@ -132,7 +181,7 @@ class HistoryMetadataMiddlewareTest {
with((service as TestingMetadataService).createdMetadata) {
assertEquals(2, this.count())
assertEquals("https://google.com?q=mozilla+website", this[0].url)
assertNull(this[0].searchTerm)
assertEquals("mozilla website", this[0].searchTerm)
assertNull(this[0].referrerUrl)
assertEquals("https://google.com?url=https://mozilla.org", this[1].url)
@ -180,7 +229,7 @@ class HistoryMetadataMiddlewareTest {
assertEquals(5, this.count())
assertEquals("https://mozilla.org/manifesto", this[4].url)
assertEquals("mozilla website", this[4].searchTerm)
assertEquals("https://google.com?q=mozilla+website", this[4].referrerUrl)
assertEquals("https://mozilla.org", this[4].referrerUrl)
}
}
@ -202,7 +251,7 @@ class HistoryMetadataMiddlewareTest {
with((service as TestingMetadataService).createdMetadata) {
assertEquals(2, this.count())
assertEquals("https://google.com?q=mozilla+website", this[0].url)
assertNull(this[0].searchTerm)
assertEquals("mozilla website", this[0].searchTerm)
assertNull(this[0].referrerUrl)
assertEquals("https://google.com?url=https://mozilla.org", this[1].url)
@ -230,6 +279,7 @@ class HistoryMetadataMiddlewareTest {
// Direct load occurs on parent tab. Search terms should be cleared.
store.dispatch(EngineAction.LoadUrlAction(parentTab.id, "https://firefox.com")).joinBlocking()
store.dispatch(ContentAction.UpdateSearchTermsAction(parentTab.id, "")).joinBlocking()
store.dispatch(ContentAction.UpdateUrlAction(parentTab.id, "https://firefox.com")).joinBlocking()
store.dispatch(ContentAction.UpdateHistoryStateAction(parentTab.id, listOf(HistoryItem("Google - mozilla website", "https://google.com?q=mozilla+website"), HistoryItem("Firefox", "https://firefox.com")), 1)).joinBlocking()
with((service as TestingMetadataService).createdMetadata) {
@ -258,7 +308,7 @@ class HistoryMetadataMiddlewareTest {
with((service as TestingMetadataService).createdMetadata) {
assertEquals(2, this.count())
assertEquals("https://google.com?q=mozilla+website", this[0].url)
assertNull(this[0].searchTerm)
assertEquals("mozilla website", this[0].searchTerm)
assertNull(this[0].referrerUrl)
assertEquals("https://google.com?url=https://mozilla.org", this[1].url)
@ -286,6 +336,7 @@ class HistoryMetadataMiddlewareTest {
// Direct load occurs on parent tab. Search terms should be cleared.
store.dispatch(EngineAction.OptimizedLoadUrlTriggeredAction(parentTab.id, "https://firefox.com")).joinBlocking()
store.dispatch(ContentAction.UpdateSearchTermsAction(parentTab.id, "")).joinBlocking()
store.dispatch(ContentAction.UpdateUrlAction(parentTab.id, "https://firefox.com")).joinBlocking()
store.dispatch(ContentAction.UpdateHistoryStateAction(parentTab.id, listOf(HistoryItem("Google - mozilla website", "https://google.com?q=mozilla+website"), HistoryItem("Firefox", "https://firefox.com")), 1)).joinBlocking()
with((service as TestingMetadataService).createdMetadata) {
@ -298,7 +349,7 @@ class HistoryMetadataMiddlewareTest {
@Test
fun `GIVEN normal tab has parent WHEN url is the same THEN nothing happens`() {
val parentTab = createTab("https://mozilla.org", searchTerms = "mozilla website")
val parentTab = createTab("https://mozilla.org")
val tab = createTab("https://mozilla.org", parent = parentTab)
store.dispatch(TabListAction.AddTabAction(parentTab)).joinBlocking()
store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking()

Write Preview
Loading…
Cancel
Save