diff --git a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt index 502236c7b..a3591d9b0 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt @@ -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() diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt index 0cfecbeca..3c2db512d 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt @@ -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 diff --git a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt index cfd540a19..a9bf0a3d7 100644 --- a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt @@ -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(