[fenix] Revert "For https://github.com/mozilla-mobile/fenix/issues/11404 - Add 'Open all in private tabs' option as requested."

This reverts commit 83ed1abdc0846e726e2fcb46d0fcb7539f1ff9c3.

app/metrics.yaml

@@ -2521,23 +2521,6 @@ bookmarks_management:
     metadata:
       tags:
         - Bookmarks
-  open_all_in_private_tabs:
-    type: event
-    description: |
-      A user opened a folder of bookmarks at once in new private tabs.
-    bugs:
-      - https://github.com/mozilla-mobile/fenix/issues/974
-      - https://github.com/mozilla-mobile/fenix/issues/11404
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/pull/21212
-    data_sensitivity:
-      - interaction
-    notification_emails:
-      - android-probes@mozilla.com
-    expires: never
-    metadata:
-      tags:
-        - Bookmarks
   edited:
     type: event
     description: |

app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt

@@ -301,54 +301,9 @@ class BookmarksTest {
             verifyExistingOpenTabs("Test_Page_1")
             verifyExistingOpenTabs("Test_Page_2")
             verifyExistingOpenTabs("Test_Page_3")
-            swipeTabRight("Test_Page_1")
-            swipeTabRight("Test_Page_2")
-            swipeTabRight("Test_Page_3")
-            // no more tabs should be presents and auto close tab tray
-            verifyTabTrayIsClosed()
-        }
-    }
-
-    @Test
-    fun openAllInPrivateTabsTest() {
-        val nbPages = 4
-        val webPages = List(nbPages) {
-            TestAssetHelper.getGenericAsset(mockWebServer, it + 1)
-        }
-
-        homeScreen {
-        }.openThreeDotMenu {
-        }.openBookmarks {
-            createFolder("root")
-            createFolder("sub", "root")
-            createFolder("empty", "root")
-        }.closeMenu {
-        }
-
-        browserScreen {
-            createBookmark(webPages[0].url, "root")
-            createBookmark(webPages[1].url, "root")
-            createBookmark(webPages[2].url, "sub")
-            // out of folder and should not be opened
-            createBookmark(webPages[3].url)
-        }.openTabDrawer {
             closeTab()
-        }
+            closeTab()
-
+            closeTab()
-        browserScreen {
-        }.openThreeDotMenu {
-        }.openBookmarks {
-        }.openThreeDotMenu("root") {
-        }.clickOpenAllInPrivateTabs {
-            verifyTabTrayIsOpened()
-            verifyPrivateModeSelected()
-
-            verifyExistingOpenTabs("Test_Page_1")
-            verifyExistingOpenTabs("Test_Page_2")
-            verifyExistingOpenTabs("Test_Page_3")
-            swipeTabRight("Test_Page_1")
-            swipeTabRight("Test_Page_2")
-            swipeTabRight("Test_Page_3")
             // no more tabs should be presents and auto close tab tray
             verifyTabTrayIsClosed()
         }

app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuBookmarksRobot.kt

@@ -59,13 +59,6 @@ class ThreeDotMenuBookmarksRobot {
             return TabDrawerRobot.Transition()
         }
 
-        fun clickOpenAllInPrivateTabs(interact: TabDrawerRobot.() -> Unit): TabDrawerRobot.Transition {
-            openAllInPrivateTabsButton().click()
-
-            TabDrawerRobot().interact()
-            return TabDrawerRobot.Transition()
-        }
-
         fun clickDelete(interact: BookmarksRobot.() -> Unit): BookmarksRobot.Transition {
             deleteButton().click()
 
@@ -85,8 +78,6 @@ private fun openInNewTabButton() = onView(withText("Open in new tab"))
 
 private fun openInPrivateTabButton() = onView(withText("Open in private tab"))
 
-private fun openAllInTabsButton() = onView(withText("Open all in new tabs"))
+private fun openAllInTabsButton() = onView(withText("Open All Bookmarks"))
-
-private fun openAllInPrivateTabsButton() = onView(withText("Open all in private tabs"))
 
 private fun deleteButton() = onView(withText("Delete"))

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

@@ -45,7 +45,7 @@ interface BookmarkController {
     fun handleCopyUrl(item: BookmarkNode)
     fun handleBookmarkSharing(item: BookmarkNode)
     fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)
-    fun handleBookmarkFolderOpening(folder: BookmarkNode, mode: BrowsingMode)
+    fun handleBookmarkFolderOpening(folder: BookmarkNode)
 
     /**
      * Handle bookmark nodes deletion
@@ -183,10 +183,10 @@ class DefaultBookmarkController(
         } else if (firstLaunch) onOpenAllInTabsEmpty.invoke()
     }
 
-    override fun handleBookmarkFolderOpening(folder: BookmarkNode, mode: BrowsingMode) {
+    override fun handleBookmarkFolderOpening(folder: BookmarkNode) {
         // potentially heavy function with a lot of bookmarks to open => use a coroutine
         scope.launch {
-            recursiveBookmarkFolderOpening(folder, true, mode)
+            recursiveBookmarkFolderOpening(folder, true)
         }
     }
 

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

@@ -81,12 +81,7 @@ class BookmarkFragmentInteractor(
 
     override fun onOpenAllInTabs(folder: BookmarkNode) {
         require(folder.type == BookmarkNodeType.FOLDER)
-        bookmarksController.handleBookmarkFolderOpening(folder, BrowsingMode.Normal)
+        bookmarksController.handleBookmarkFolderOpening(folder)
-    }
-
-    override fun onOpenAllInPrivateTabs(folder: BookmarkNode) {
-        require(folder.type == BookmarkNodeType.FOLDER)
-        bookmarksController.handleBookmarkFolderOpening(folder, BrowsingMode.Private)
     }
 
     override fun onDelete(nodes: Set<BookmarkNode>) {

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

@@ -27,7 +27,6 @@ class BookmarkItemMenu(
         OpenInNewTab,
         OpenInPrivateTab,
         OpenAllInTabs,
-        OpenAllInPrivateTabs,
         Delete,
         ;
     }
@@ -111,16 +110,6 @@ class BookmarkItemMenu(
             } else {
                 null
             },
-            if (hasAtLeastOneChild) {
-                maybeCreateMenuItem(
-                    itemType,
-                    BookmarkNodeType.FOLDER,
-                    context.getString(R.string.bookmark_menu_open_all_in_privates_button),
-                    Item.OpenAllInPrivateTabs,
-                )
-            } else {
-                null
-            },
             TextMenuCandidate(
                 text = context.getString(R.string.bookmark_menu_delete_button),
                 textStyle = TextStyle(color = context.getColorFromAttr(R.attr.textWarning)),

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

@@ -86,13 +86,6 @@ interface BookmarkViewInteractor : SelectionInteractor<BookmarkNode> {
      */
     fun onOpenAllInTabs(folder: BookmarkNode)
 
-    /**
-     * Opens all bookmark items in new private tabs.
-     *
-     * @param folder the bookmark folder containing all items to open in new private tabs
-     */
-    fun onOpenAllInPrivateTabs(folder: BookmarkNode)
-
     /**
      * Deletes a set of bookmark nodes.
      *

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

@@ -46,7 +46,6 @@ class BookmarkNodeViewHolder(
                 BookmarkItemMenu.Item.OpenInNewTab -> interactor.onOpenInNormalTab(item)
                 BookmarkItemMenu.Item.OpenInPrivateTab -> interactor.onOpenInPrivateTab(item)
                 BookmarkItemMenu.Item.OpenAllInTabs -> interactor.onOpenAllInTabs(item)
-                BookmarkItemMenu.Item.OpenAllInPrivateTabs -> interactor.onOpenAllInPrivateTabs(item)
                 BookmarkItemMenu.Item.Delete -> interactor.onDelete(setOf(item))
             }
         }

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

@@ -858,8 +858,6 @@
     <string name="bookmark_menu_open_in_private_tab_button">Open in private tab</string>
     <!-- Bookmark overflow menu open all in tabs button -->
     <string name="bookmark_menu_open_all_in_tabs_button">Open all in new tabs</string>
-    <!-- Bookmark overflow menu open all in private tabs button -->
-    <string name="bookmark_menu_open_all_in_privates_button">Open all in private tabs</string>
     <!-- Bookmark overflow menu delete button -->
     <string name="bookmark_menu_delete_button">Delete</string>
     <!--Bookmark overflow menu save button -->

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

@@ -381,7 +381,7 @@ class BookmarkControllerTest {
                 }
                 recurseFind(tree, it)
             },
-        ).handleBookmarkFolderOpening(tree, BrowsingMode.Normal)
+        ).handleBookmarkFolderOpening(tree)
 
         assertTrue(showTabTrayInvoked)
         verifyOrder {
@@ -392,40 +392,6 @@ class BookmarkControllerTest {
         }
     }
 
-    @Test
-    fun `handleBookmarkFolderOpening should open all bookmarks in private tabs`() {
-        var showTabTrayInvoked = false
-        createController(
-            showTabTray = {
-                showTabTrayInvoked = true
-            },
-            loadBookmarkNode = {
-                fun recurseFind(item: BookmarkNode, guid: String): BookmarkNode? {
-                    if (item.guid == guid) {
-                        return item
-                    } else {
-                        item.children?.iterator()?.forEach {
-                            val res = recurseFind(it, guid)
-                            if (res != null) {
-                                return res
-                            }
-                        }
-                        return null
-                    }
-                }
-                recurseFind(tree, it)
-            },
-        ).handleBookmarkFolderOpening(tree, BrowsingMode.Private)
-
-        assertTrue(showTabTrayInvoked)
-        verifyOrder {
-            addNewTabUseCase.invoke(item.url!!, private = true)
-            addNewTabUseCase.invoke(item.url!!, private = true)
-            addNewTabUseCase.invoke(childItem.url!!, private = true)
-            homeActivity.browsingModeManager.mode = BrowsingMode.Private
-        }
-    }
-
     @Test
     fun `handleBookmarkFolderOpening for an empty folder should show toast`() {
         var onOpenAllInTabsEmptyInvoked = false
@@ -436,7 +402,7 @@ class BookmarkControllerTest {
             loadBookmarkNode = {
                 subfolder
             },
-        ).handleBookmarkFolderOpening(subfolder, BrowsingMode.Normal)
+        ).handleBookmarkFolderOpening(subfolder)
 
         assertTrue(onOpenAllInTabsEmptyInvoked)
     }

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

@@ -175,16 +175,7 @@ class BookmarkFragmentInteractorTest {
         interactor.onOpenAllInTabs(tree)
 
         verify {
-            bookmarkController.handleBookmarkFolderOpening(tree, BrowsingMode.Normal)
+            bookmarkController.handleBookmarkFolderOpening(tree)
-        }
-    }
-
-    @Test
-    fun `open all bookmarks item in new private tabs`() {
-        interactor.onOpenAllInPrivateTabs(tree)
-
-        verify {
-            bookmarkController.handleBookmarkFolderOpening(tree, BrowsingMode.Private)
         }
     }
 

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

@@ -81,13 +81,12 @@ class BookmarkItemMenuTest {
             folderItems = menu.menuItems(BookmarkNodeType.FOLDER, "")
         }
         assertNotNull(folderItems)
-        assertEquals(4, folderItems!!.size)
+        assertEquals(3, folderItems!!.size)
 
-        val (edit, openAll, openAllPrivate, delete) = folderItems!!
+        val (edit, openAll, delete) = folderItems!!
 
         assertEquals("Edit", edit.text)
-        assertEquals("Open all in new tabs", openAll.text)
+        assertEquals("Open All Bookmarks", openAll.text)
-        assertEquals("Open all in private tabs", openAllPrivate.text)
         assertEquals("Delete", delete.text)
 
         edit.onClick()
@@ -96,13 +95,37 @@ class BookmarkItemMenuTest {
         openAll.onClick()
         assertEquals(Item.OpenAllInTabs, lastItemTapped)
 
-        openAllPrivate.onClick()
-        assertEquals(Item.OpenAllInPrivateTabs, lastItemTapped)
-
         delete.onClick()
         assertEquals(Item.Delete, lastItemTapped)
     }
 
+    @Test
+    fun `delete item has special styling`() {
+        val deleteItem = menu.menuItems(BookmarkNodeType.SEPARATOR).last()
+        assertEquals("Delete", deleteItem.text)
+        assertEquals(
+            TextStyle(color = context.getColorFromAttr(R.attr.textWarning)),
+            deleteItem.textStyle,
+        )
+
+        deleteItem.onClick()
+
+        assertEquals(Item.Delete, lastItemTapped)
+    }
+
+    @Test
+    fun `edit item appears for folders`() {
+        val folderItems = menu.menuItems(BookmarkNodeType.FOLDER)
+        assertEquals(2, folderItems.size)
+        val (edit, delete) = folderItems
+
+        assertEquals("Edit", edit.text)
+        edit.onClick()
+
+        assertEquals(Item.Edit, lastItemTapped)
+        assertEquals("Delete", delete.text)
+    }
+
     @Test
     fun `all item appears for sites`() = runBlocking {
         var siteItems: List<TextMenuCandidate>? = null