Close mozilla-mobile#13336: Open bookmark in current tab if browsing …

…mode is not private or previous fragment is not home
rocketsroger · Feb 23, 2022 · 31f4234 · 31f4234
1 parent d71e311
commit 31f4234
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 3 deletions.
diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt
@@ -74,10 +74,13 @@ class DefaultBookmarkController(
     }
 
     override fun handleBookmarkTapped(item: BookmarkNode) {
+        val fromHomeFragment =
+            navController.previousBackStackEntry?.destination?.id == R.id.homeFragment
+        val isPrivate = activity.browsingModeManager.mode == BrowsingMode.Private
         val flags = EngineSession.LoadUrlFlags.select(EngineSession.LoadUrlFlags.ALLOW_JAVASCRIPT_URL)
         openInNewTabAndShow(
             item.url!!,
-            true,
+            isPrivate || fromHomeFragment,
             BrowserDirection.FromBookmarks,
             activity.browsingModeManager.mode,
             flags

diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt
@@ -6,6 +6,7 @@ package org.mozilla.fenix.library.bookmarks
 
 import android.content.ClipboardManager
 import android.content.Context
+import androidx.navigation.NavBackStackEntry
 import androidx.navigation.NavController
 import androidx.navigation.NavDestination
 import androidx.navigation.NavDirections
@@ -54,6 +55,8 @@ class BookmarkControllerTest {
     private val homeActivity: HomeActivity = mockk(relaxed = true)
     private val services: Services = mockk(relaxed = true)
     private val addNewTabUseCase: TabsUseCases.AddNewTabUseCase = mockk(relaxed = true)
+    private val navBackStackEntry: NavBackStackEntry = mockk(relaxed = true)
+    private val navDestination: NavDestination = mockk(relaxed = true)
 
     private val item =
         BookmarkNode(BookmarkNodeType.ITEM, "456", "123", 0u, "Mozilla", "http://mozilla.org", 0, null)
@@ -89,6 +92,9 @@ class BookmarkControllerTest {
         every { navController.currentDestination } returns NavDestination("").apply {
             id = R.id.bookmarkFragment
         }
+        every { navController.previousBackStackEntry } returns navBackStackEntry
+        every { navBackStackEntry.destination } returns navDestination
+        every { navDestination.id } returns R.id.browserFragment
         every { bookmarkStore.dispatch(any()) } returns mockk()
         every { sharedViewModel.selectedFolder = any() } just runs
         every { tabsUseCases.addTab } returns addNewTabUseCase
@@ -110,7 +116,7 @@ class BookmarkControllerTest {
     }
 
     @Test
-    fun `handleBookmarkTapped should load the bookmark in a new tab`() {
+    fun `WHEN handleBookmarkTapped is called with BrowserFragment THEN load the bookmark in current tab`() {
         var invokePendingDeletionInvoked = false
         val flags = EngineSession.LoadUrlFlags.select(EngineSession.LoadUrlFlags.ALLOW_JAVASCRIPT_URL)
 
@@ -120,6 +126,53 @@ class BookmarkControllerTest {
             }
         ).handleBookmarkTapped(item)
 
+        assertTrue(invokePendingDeletionInvoked)
+        verify {
+            homeActivity.openToBrowserAndLoad(
+                item.url!!,
+                false,
+                BrowserDirection.FromBookmarks,
+                flags = flags
+            )
+        }
+    }
+
+    @Test
+    fun `WHEN handleBookmarkTapped is called with HomeFragment THEN load the bookmark in new tab`() {
+        var invokePendingDeletionInvoked = false
+        val flags = EngineSession.LoadUrlFlags.select(EngineSession.LoadUrlFlags.ALLOW_JAVASCRIPT_URL)
+
+        every { navDestination.id } returns R.id.homeFragment
+
+        createController(
+            invokePendingDeletion = {
+                invokePendingDeletionInvoked = true
+            }
+        ).handleBookmarkTapped(item)
+
+        assertTrue(invokePendingDeletionInvoked)
+        verify {
+            homeActivity.openToBrowserAndLoad(
+                item.url!!,
+                true,
+                BrowserDirection.FromBookmarks,
+                flags = flags
+            )
+        }
+    }
+
+    @Test
+    fun `WHEN handleBookmarkTapped is called with private browsing THEN load the bookmark in new tab`() {
+        var invokePendingDeletionInvoked = false
+        every { homeActivity.browsingModeManager.mode } returns BrowsingMode.Private
+        val flags = EngineSession.LoadUrlFlags.select(EngineSession.LoadUrlFlags.ALLOW_JAVASCRIPT_URL)
+
+        createController(
+            invokePendingDeletion = {
+                invokePendingDeletionInvoked = true
+            }
+        ).handleBookmarkTapped(item)
+
         assertTrue(invokePendingDeletionInvoked)
         verify {
             homeActivity.openToBrowserAndLoad(
@@ -290,7 +343,7 @@ class BookmarkControllerTest {
         verify {
             homeActivity.openToBrowserAndLoad(
                 item.url!!,
-                true,
+                false,
                 BrowserDirection.FromBookmarks,
                 flags = flags
             )