Bug 1840721 - For top sites we should prefer switching to open tabs vs opening a duplicate tab

fenix/118.0
t-p-white 11 months ago committed by mergify[bot]
parent 321dd0e532
commit b657d3a71c

@ -54,20 +54,20 @@ class DefaultRecentBookmarksController(
) : RecentBookmarksController {
override fun handleBookmarkClicked(bookmark: RecentBookmark) {
val bookmarkTab = browserStore.state.tabs.firstOrNull {
val existingTabForBookmark = browserStore.state.tabs.firstOrNull {
it.content.url == bookmark.url
}
if (bookmarkTab != null) {
selectTabUseCase.invoke(bookmarkTab.id)
navController.navigate(R.id.browserFragment)
} else {
if (existingTabForBookmark == null) {
activity.openToBrowserAndLoad(
searchTermOrURL = bookmark.url!!,
newTab = true,
from = BrowserDirection.FromHome,
flags = EngineSession.LoadUrlFlags.select(ALLOW_JAVASCRIPT_URL),
)
} else {
selectTabUseCase.invoke(existingTabForBookmark.id)
navController.navigate(R.id.browserFragment)
}
RecentBookmarks.bookmarkClicked.add()

@ -346,8 +346,6 @@ class DefaultSessionControlController(
}
override fun handleSelectTopSite(topSite: TopSite, position: Int) {
TopSites.openInNewTab.record(NoExtras())
when (topSite) {
is TopSite.Default -> TopSites.openDefault.record(NoExtras())
is TopSite.Frecent -> TopSites.openFrecency.record(NoExtras())
@ -376,16 +374,31 @@ class DefaultSessionControlController(
)
}
val tabId = addTabUseCase.invoke(
url = appendSearchAttributionToUrlIfNeeded(topSite.url),
selectTab = true,
startLoading = true,
)
val existingTabForUrl = when (topSite) {
is TopSite.Frecent, is TopSite.Pinned -> {
store.state.tabs.firstOrNull { topSite.url == it.content.url }
}
else -> null
}
if (settings.openNextTabInDesktopMode) {
activity.handleRequestDesktopMode(tabId)
if (existingTabForUrl == null) {
TopSites.openInNewTab.record(NoExtras())
val tabId = addTabUseCase.invoke(
url = appendSearchAttributionToUrlIfNeeded(topSite.url),
selectTab = true,
startLoading = true,
)
if (settings.openNextTabInDesktopMode) {
activity.handleRequestDesktopMode(tabId)
}
activity.openToBrowser(BrowserDirection.FromHome)
} else {
selectTabUseCase.invoke(existingTabForUrl.id)
navController.navigate(R.id.browserFragment)
}
activity.openToBrowser(BrowserDirection.FromHome)
}
@VisibleForTesting

@ -473,6 +473,175 @@ class DefaultSessionControlControllerTest {
verify { activity.openToBrowser(BrowserDirection.FromHome) }
}
@Test
fun `GIVEN existing tab for url WHEN Default TopSite selected THEN open new tab`() {
val url = "mozilla.org"
val existingTabForUrl = createTab(url = url)
store = BrowserStore(
BrowserState(
tabs = listOf(existingTabForUrl),
search = SearchState(
regionSearchEngines = listOf(searchEngine),
),
),
)
val topSite = TopSite.Default(
id = 1L,
title = "Mozilla",
url = url,
createdAt = 0,
)
val controller = spyk(createController())
every { controller.getAvailableSearchEngines() } returns listOf(searchEngine)
controller.handleSelectTopSite(topSite, position = 0)
assertNotNull(TopSites.openInNewTab.testGetValue())
assertEquals(1, TopSites.openInNewTab.testGetValue()!!.size)
assertNull(TopSites.openInNewTab.testGetValue()!!.single().extra)
assertNotNull(TopSites.openDefault.testGetValue())
assertEquals(1, TopSites.openDefault.testGetValue()!!.size)
assertNull(TopSites.openDefault.testGetValue()!!.single().extra)
verify {
tabsUseCases.addTab.invoke(
url = topSite.url,
selectTab = true,
startLoading = true,
)
}
verify { activity.openToBrowser(BrowserDirection.FromHome) }
}
@Test
fun `GIVEN existing tab for url WHEN Provided TopSite selected THEN open new tab`() {
val url = "mozilla.org"
val existingTabForUrl = createTab(url = url)
store = BrowserStore(
BrowserState(
tabs = listOf(existingTabForUrl),
search = SearchState(
regionSearchEngines = listOf(searchEngine),
),
),
)
val topSite = TopSite.Provided(
id = 1L,
title = "Mozilla",
url = url,
clickUrl = "",
imageUrl = "",
impressionUrl = "",
createdAt = 0,
)
val position = 0
val controller = spyk(createController())
every { controller.getAvailableSearchEngines() } returns listOf(searchEngine)
controller.handleSelectTopSite(topSite, position)
assertNotNull(TopSites.openInNewTab.testGetValue())
assertEquals(1, TopSites.openInNewTab.testGetValue()!!.size)
assertNull(TopSites.openInNewTab.testGetValue()!!.single().extra)
assertNotNull(TopSites.openContileTopSite.testGetValue())
assertEquals(1, TopSites.openContileTopSite.testGetValue()!!.size)
assertNull(TopSites.openContileTopSite.testGetValue()!!.single().extra)
verify {
tabsUseCases.addTab.invoke(
url = topSite.url,
selectTab = true,
startLoading = true,
)
}
verify { controller.submitTopSitesImpressionPing(topSite, position) }
verify { activity.openToBrowser(BrowserDirection.FromHome) }
}
@Test
fun `GIVEN existing tab for url WHEN Frecent TopSite selected THEN navigate to tab`() {
val url = "mozilla.org"
val existingTabForUrl = createTab(url = url)
store = BrowserStore(
BrowserState(
tabs = listOf(existingTabForUrl),
search = SearchState(
regionSearchEngines = listOf(searchEngine),
),
),
)
val topSite = TopSite.Frecent(
id = 1L,
title = "Mozilla",
url = url,
createdAt = 0,
)
val controller = spyk(createController())
every { controller.getAvailableSearchEngines() } returns listOf(searchEngine)
controller.handleSelectTopSite(topSite, position = 0)
assertNull(TopSites.openInNewTab.testGetValue())
assertNotNull(TopSites.openFrecency.testGetValue())
assertEquals(1, TopSites.openFrecency.testGetValue()!!.size)
assertNull(TopSites.openFrecency.testGetValue()!!.single().extra)
verify {
selectTabUseCase.invoke(existingTabForUrl.id)
navController.navigate(R.id.browserFragment)
}
}
@Test
fun `GIVEN existing tab for url WHEN Pinned TopSite selected THEN navigate to tab`() {
val url = "mozilla.org"
val existingTabForUrl = createTab(url = url)
store = BrowserStore(
BrowserState(
tabs = listOf(existingTabForUrl),
search = SearchState(
regionSearchEngines = listOf(searchEngine),
),
),
)
val topSite = TopSite.Pinned(
id = 1L,
title = "Mozilla",
url = url,
createdAt = 0,
)
val controller = spyk(createController())
every { controller.getAvailableSearchEngines() } returns listOf(searchEngine)
controller.handleSelectTopSite(topSite, position = 0)
assertNull(TopSites.openInNewTab.testGetValue())
assertNotNull(TopSites.openPinned.testGetValue())
assertEquals(1, TopSites.openPinned.testGetValue()!!.size)
assertNull(TopSites.openPinned.testGetValue()!!.single().extra)
verify {
selectTabUseCase.invoke(existingTabForUrl.id)
navController.navigate(R.id.browserFragment)
}
}
@Test
fun handleSelectGoogleDefaultTopSiteUS() {
val topSite = TopSite.Default(

Loading…
Cancel
Save