diff --git a/app/metrics.yaml b/app/metrics.yaml index df6fd1ae21..9fd124855f 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -2517,6 +2517,23 @@ 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: | diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt index eb643054c3..fa3e2a82cd 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt @@ -301,9 +301,54 @@ 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() } diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuBookmarksRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuBookmarksRobot.kt index 25f6ec3754..035aa87ffa 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuBookmarksRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuBookmarksRobot.kt @@ -59,6 +59,13 @@ 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() @@ -78,6 +85,8 @@ 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 Bookmarks")) +private fun openAllInTabsButton() = onView(withText("Open all in new tabs")) + +private fun openAllInPrivateTabsButton() = onView(withText("Open all in private tabs")) private fun deleteButton() = onView(withText("Delete")) 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 index 69205b9331..b46307e150 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt +++ b/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) + fun handleBookmarkFolderOpening(folder: BookmarkNode, mode: BrowsingMode) /** * Handle bookmark nodes deletion @@ -183,10 +183,10 @@ class DefaultBookmarkController( } else if (firstLaunch) onOpenAllInTabsEmpty.invoke() } - override fun handleBookmarkFolderOpening(folder: BookmarkNode) { + override fun handleBookmarkFolderOpening(folder: BookmarkNode, mode: BrowsingMode) { // potentially heavy function with a lot of bookmarks to open => use a coroutine scope.launch { - recursiveBookmarkFolderOpening(folder, true) + recursiveBookmarkFolderOpening(folder, true, mode) } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt index c4b6c86c01..bdac4350ad 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt @@ -81,7 +81,12 @@ class BookmarkFragmentInteractor( override fun onOpenAllInTabs(folder: BookmarkNode) { require(folder.type == BookmarkNodeType.FOLDER) - bookmarksController.handleBookmarkFolderOpening(folder) + bookmarksController.handleBookmarkFolderOpening(folder, BrowsingMode.Normal) + } + + override fun onOpenAllInPrivateTabs(folder: BookmarkNode) { + require(folder.type == BookmarkNodeType.FOLDER) + bookmarksController.handleBookmarkFolderOpening(folder, BrowsingMode.Private) } override fun onDelete(nodes: Set) { diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkItemMenu.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkItemMenu.kt index 2d678f7ec2..249535312e 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkItemMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkItemMenu.kt @@ -27,6 +27,7 @@ class BookmarkItemMenu( OpenInNewTab, OpenInPrivateTab, OpenAllInTabs, + OpenAllInPrivateTabs, Delete, ; } @@ -110,6 +111,16 @@ 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)), diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt index 853a20009a..c609cfdd72 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt @@ -86,6 +86,13 @@ interface BookmarkViewInteractor : SelectionInteractor { */ 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. * diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkNodeViewHolder.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkNodeViewHolder.kt index 8131c92b6c..153238c393 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkNodeViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/viewholders/BookmarkNodeViewHolder.kt @@ -46,6 +46,7 @@ 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)) } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a086667011..229221db9d 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -864,6 +864,8 @@ Open in private tab Open all in new tabs + + Open all in private tabs Delete 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 index 4a8a7c0529..535365735a 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt @@ -381,7 +381,7 @@ class BookmarkControllerTest { } recurseFind(tree, it) }, - ).handleBookmarkFolderOpening(tree) + ).handleBookmarkFolderOpening(tree, BrowsingMode.Normal) assertTrue(showTabTrayInvoked) verifyOrder { @@ -392,6 +392,40 @@ 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 @@ -402,7 +436,7 @@ class BookmarkControllerTest { loadBookmarkNode = { subfolder }, - ).handleBookmarkFolderOpening(subfolder) + ).handleBookmarkFolderOpening(subfolder, BrowsingMode.Normal) assertTrue(onOpenAllInTabsEmptyInvoked) } diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt index 8b670bf229..c5556975ff 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt @@ -175,7 +175,16 @@ class BookmarkFragmentInteractorTest { interactor.onOpenAllInTabs(tree) verify { - bookmarkController.handleBookmarkFolderOpening(tree) + bookmarkController.handleBookmarkFolderOpening(tree, BrowsingMode.Normal) + } + } + + @Test + fun `open all bookmarks item in new private tabs`() { + interactor.onOpenAllInPrivateTabs(tree) + + verify { + bookmarkController.handleBookmarkFolderOpening(tree, BrowsingMode.Private) } } diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkItemMenuTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkItemMenuTest.kt index 82a1ec5b0e..a356018983 100644 --- a/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkItemMenuTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkItemMenuTest.kt @@ -81,12 +81,13 @@ class BookmarkItemMenuTest { folderItems = menu.menuItems(BookmarkNodeType.FOLDER, "") } assertNotNull(folderItems) - assertEquals(3, folderItems!!.size) + assertEquals(4, folderItems!!.size) - val (edit, openAll, delete) = folderItems!! + val (edit, openAll, openAllPrivate, delete) = folderItems!! assertEquals("Edit", edit.text) - assertEquals("Open All Bookmarks", openAll.text) + assertEquals("Open all in new tabs", openAll.text) + assertEquals("Open all in private tabs", openAllPrivate.text) assertEquals("Delete", delete.text) edit.onClick() @@ -95,37 +96,13 @@ 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? = null