Revert "For #11404 - Create alert message when a lot of tabs are to be opened."

This reverts commit c75e331a30.

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt

@@ -7,7 +7,6 @@ package org.mozilla.fenix.library.bookmarks
 import android.content.ClipData
 import android.content.ClipboardManager
 import android.content.res.Resources
-import androidx.annotation.VisibleForTesting
 import androidx.navigation.NavController
 import androidx.navigation.NavDirections
 import kotlinx.coroutines.CoroutineScope
@@ -76,9 +75,9 @@ class DefaultBookmarkController(
     private val store: BookmarkFragmentStore,
     private val sharedViewModel: BookmarksSharedViewModel,
     private val tabsUseCases: TabsUseCases?,
-    private val loadBookmarkNode: suspend (String, Boolean) -> BookmarkNode?,
+    private val loadBookmarkNode: suspend (String) -> BookmarkNode?,
     private val showSnackbar: (String) -> Unit,
-    private val alertHeavyOpen: (Int, () -> Unit) -> Unit,
+    private val onOpenAllInTabsEmpty: () -> Unit,
     private val deleteBookmarkNodes: (Set<BookmarkNode>, BookmarkRemoveType) -> Unit,
     private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit,
     private val showTabTray: () -> Unit,
@@ -87,10 +86,6 @@ class DefaultBookmarkController(
 
     private val resources: Resources = activity.resources
 
-    @Suppress("MagicNumber")
-    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
-    internal val maxOpenBeforeWarn = 15
-
     override fun handleBookmarkChanged(item: BookmarkNode) {
         sharedViewModel.selectedFolder = item
         store.dispatch(BookmarkFragmentAction.Change(item))
@@ -113,7 +108,7 @@ class DefaultBookmarkController(
     override fun handleBookmarkExpand(folder: BookmarkNode) {
         handleAllBookmarksDeselected()
         scope.launch {
-            val node = loadBookmarkNode.invoke(folder.guid, false) ?: return@launch
+            val node = loadBookmarkNode.invoke(folder.guid) ?: return@launch
             sharedViewModel.selectedFolder = node
             store.dispatch(BookmarkFragmentAction.Change(node))
         }
@@ -166,56 +161,32 @@ class DefaultBookmarkController(
         showTabTray()
     }
 
-    private fun extractURLsFromTree(node: BookmarkNode): MutableList<String> {
+    private suspend fun recursiveBookmarkFolderOpening(folder: BookmarkNode, firstLaunch: Boolean = false) {
-        val urls = mutableListOf<String>()
+        val node = loadBookmarkNode.invoke(folder.guid) ?: return
+        if (!node.children.isNullOrEmpty()) {
+            if (firstLaunch) invokePendingDeletion.invoke()
 
-        when (node.type) {
+            node.children!!.iterator().forEach {
-            BookmarkNodeType.FOLDER -> {
+                when (it.type) {
-                // if not leaf (empty) folder
+                    BookmarkNodeType.FOLDER -> recursiveBookmarkFolderOpening(it, mode = mode)
-                if (!node.children.isNullOrEmpty()) {
+                    BookmarkNodeType.ITEM -> {
-                    node.children!!.iterator().forEach {
+                        it.url?.let { url -> tabsUseCases?.addTab?.invoke(url, private = (mode == BrowsingMode.Private)) }
-                        urls.addAll(extractURLsFromTree(it))
                     }
+                    BookmarkNodeType.SEPARATOR -> {}
+                }
+            }.also {
+                if (firstLaunch) {
+                    activity.browsingModeManager.mode = BrowsingMode.fromBoolean(mode == BrowsingMode.Private)
+                    showTabTray()
                 }
             }
-            BookmarkNodeType.ITEM -> {
+        } else if (firstLaunch) onOpenAllInTabsEmpty.invoke()
-                node.url?.let { urls.add(it) }
-            }
-            BookmarkNodeType.SEPARATOR -> {}
-        }
-
-        return urls
     }
 
     override fun handleBookmarkFolderOpening(folder: BookmarkNode, mode: BrowsingMode) {
         // potentially heavy function with a lot of bookmarks to open => use a coroutine
         scope.launch {
-            // if more than maxOpenBeforeWarn (15) elements
+            recursiveBookmarkFolderOpening(folder, true, mode)
-            val tree = loadBookmarkNode.invoke(folder.guid, true) ?: return@launch
-            val urls = extractURLsFromTree(tree)
-
-            val openAll = { load: Boolean ->
-                for (url in urls) {
-                    tabsUseCases?.addTab?.invoke(
-                        url,
-                        private = (mode == BrowsingMode.Private),
-                        startLoading = load,
-                    )
-                }
-                activity.browsingModeManager.mode =
-                    BrowsingMode.fromBoolean(mode == BrowsingMode.Private)
-                showTabTray()
-            }
-
-            // warn user if more than maxOpenBeforeWarn (15)
-            if (urls.size >= maxOpenBeforeWarn) {
-                alertHeavyOpen(urls.size) {
-                    // do not load bookmarks when more than maxOpenBeforeWarn (15)
-                    openAll(false)
-                }
-            } else {
-                openAll(true)
-            }
         }
     }
 

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt

@@ -101,7 +101,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
                 tabsUseCases = activity?.components?.useCases?.tabsUseCases,
                 loadBookmarkNode = ::loadBookmarkNode,
                 showSnackbar = ::showSnackBarWithText,
-                alertHeavyOpen = ::alertHeavyOpen,
+                onOpenAllInTabsEmpty = ::onOpenAllInTabsEmpty,
                 deleteBookmarkNodes = ::deleteMulti,
                 deleteBookmarkFolder = ::showRemoveFolderDialog,
                 showTabTray = ::showTabTray,
@@ -272,11 +272,11 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
         return bookmarkView.onBackPressed()
     }
 
-    private suspend fun loadBookmarkNode(guid: String, recursive: Boolean = false): BookmarkNode? = withContext(IO) {
+    private suspend fun loadBookmarkNode(guid: String): BookmarkNode? = withContext(IO) {
         // Only runs if the fragment is attached same as [runIfFragmentIsAttached]
         context?.let {
             requireContext().bookmarkStorage
-                .getTree(guid, recursive)
+                .getTree(guid, false)
                 ?.let { desktopFolders.withOptionalDesktopFolders(it) }
         }
     }

app/src/main/res/values/strings.xml

@@ -620,15 +620,6 @@
     <!-- Content description (not visible, for screen readers etc.): "Close button for library settings" -->
     <string name="content_description_close_button">Close</string>
 
-    <!-- Title to show in alert when a lot of tabs are to be opened -->
-    <string name="open_all_warning_title">Confirm open</string>
-    <!-- Message to show in alert when a lot of tabs are to be opened -->
-    <string name="open_all_warning_message">You are about to open %d tabs. This might slow down Firefox. Are you sure you want to continue ?</string>
-    <!-- Dialog button text for confirming open all tabs -->
-    <string name="open_all_warning_confirm">Open tabs</string>
-    <!-- Dialog button text for canceling open all tabs -->
-    <string name="open_all_warning_cancel">Cancel</string>
-
     <!-- Text to show users they have one site in the history group section of the History fragment.
     %d is a placeholder for the number of sites in the group. -->
     <string name="history_search_group_site">%d site</string>

app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt

@@ -17,11 +17,9 @@ import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.just
 import io.mockk.mockk
-import io.mockk.mockkConstructor
 import io.mockk.runs
 import io.mockk.slot
 import io.mockk.spyk
-import io.mockk.unmockkConstructor
 import io.mockk.verify
 import io.mockk.verifyOrder
 import mozilla.appservices.places.BookmarkRoot
@@ -195,7 +193,7 @@ class BookmarkControllerTest {
     fun `handleBookmarkExpand should refresh and change the active bookmark node`() = runTestOnMain {
         var loadBookmarkNodeInvoked = false
         createController(
-            loadBookmarkNode = { _: String, _: Boolean ->
+            loadBookmarkNode = {
                 loadBookmarkNodeInvoked = true
                 tree
             },
@@ -367,7 +365,7 @@ class BookmarkControllerTest {
             showTabTray = {
                 showTabTrayInvoked = true
             },
-            loadBookmarkNode = { guid: String, _: Boolean ->
+            loadBookmarkNode = {
                 fun recurseFind(item: BookmarkNode, guid: String): BookmarkNode? {
                     if (item.guid == guid) {
                         return item
@@ -381,7 +379,7 @@ class BookmarkControllerTest {
                         return null
                     }
                 }
-                recurseFind(tree, guid)
+                recurseFind(tree, it)
             },
         ).handleBookmarkFolderOpening(tree, BrowsingMode.Normal)
 
@@ -401,7 +399,7 @@ class BookmarkControllerTest {
             showTabTray = {
                 showTabTrayInvoked = true
             },
-            loadBookmarkNode = { guid: String, _: Boolean ->
+            loadBookmarkNode = {
                 fun recurseFind(item: BookmarkNode, guid: String): BookmarkNode? {
                     if (item.guid == guid) {
                         return item
@@ -415,7 +413,7 @@ class BookmarkControllerTest {
                         return null
                     }
                 }
-                recurseFind(tree, guid)
+                recurseFind(tree, it)
             },
         ).handleBookmarkFolderOpening(tree, BrowsingMode.Private)
 
@@ -429,24 +427,18 @@ class BookmarkControllerTest {
     }
 
     @Test
-    fun `handleBookmarkFolderOpening for more than maxOpenBeforeWarn items should show alert`() {
+    fun `handleBookmarkFolderOpening for an empty folder should show toast`() {
-        var alertHeavyOpenInvoked = false
+        var onOpenAllInTabsEmptyInvoked = false
-
-        mockkConstructor(DefaultBookmarkController::class)
-        every {
-            anyConstructed<DefaultBookmarkController>() getProperty "maxOpenBeforeWarn"
-        } propertyType Int::class returns 1
-
         createController(
-            alertHeavyOpen = { _: Int, _: () -> Unit -> alertHeavyOpenInvoked = true },
+            onOpenAllInTabsEmpty = {
-            loadBookmarkNode = { _: String, _: Boolean ->
+                onOpenAllInTabsEmptyInvoked = true
-                tree
             },
-        ).handleBookmarkFolderOpening(tree, BrowsingMode.Normal)
+            loadBookmarkNode = {
-
+                subfolder
-        unmockkConstructor(DefaultBookmarkController::class)
+            },
+        ).handleBookmarkFolderOpening(subfolder, BrowsingMode.Normal)
 
-        assertTrue(alertHeavyOpenInvoked)
+        assertTrue(onOpenAllInTabsEmptyInvoked)
     }
 
     @Test
@@ -517,9 +509,9 @@ class BookmarkControllerTest {
 
     @Suppress("LongParameterList")
     private fun createController(
-        loadBookmarkNode: suspend (String, Boolean) -> BookmarkNode? = { _, _ -> null },
+        loadBookmarkNode: suspend (String) -> BookmarkNode? = { _ -> null },
         showSnackbar: (String) -> Unit = { _ -> },
-        alertHeavyOpen: (Int, () -> Unit) -> Unit = { _: Int, _: () -> Unit -> },
+        onOpenAllInTabsEmpty: () -> Unit = { },
         deleteBookmarkNodes: (Set<BookmarkNode>, BookmarkRemoveType) -> Unit = { _, _ -> },
         deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit = { _ -> },
         showTabTray: () -> Unit = { },
@@ -534,7 +526,7 @@ class BookmarkControllerTest {
             tabsUseCases = tabsUseCases,
             loadBookmarkNode = loadBookmarkNode,
             showSnackbar = showSnackbar,
-            alertHeavyOpen = alertHeavyOpen,
+            onOpenAllInTabsEmpty = onOpenAllInTabsEmpty,
             deleteBookmarkNodes = deleteBookmarkNodes,
             deleteBookmarkFolder = deleteBookmarkFolder,
             showTabTray = showTabTray,

detekt-baseline.xml

@@ -547,12 +547,11 @@
     <ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleBookmarkTapped(item: BookmarkNode)</ID>
     <ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleCopyUrl(item: BookmarkNode)</ID>
     <ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)</ID>
-    <ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleBookmarkFolderOpening(folder: BookmarkNode, mode: BrowsingMode)</ID>
     <ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleRequestSync()</ID>
     <ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleSearch()</ID>
     <ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleSelectionModeSwitch()</ID>
     <ID>UndocumentedPublicFunction:BookmarkFragmentStore.kt$operator fun BookmarkNode.contains(item: BookmarkNode): Boolean</ID>
-    <ID>UndocumentedPublicFunction:BookmarkItemMenu.kt$BookmarkItemMenu$suspend fun updateMenu(itemType: BookmarkNodeType, itemId: String)</ID>
+    <ID>UndocumentedPublicFunction:BookmarkItemMenu.kt$BookmarkItemMenu$fun updateMenu(itemType: BookmarkNodeType)</ID>
     <ID>UndocumentedPublicFunction:BookmarkNodeViewHolder.kt$BookmarkNodeViewHolder$fun bind( item: BookmarkNode, mode: BookmarkFragmentState.Mode, payload: BookmarkPayload, )</ID>
     <ID>UndocumentedPublicFunction:BookmarkSearchController.kt$BookmarkSearchController$fun handleEditingCancelled()</ID>
     <ID>UndocumentedPublicFunction:BookmarkSearchController.kt$BookmarkSearchController$fun handleTextChanged(text: String)</ID>