diff --git a/app/metrics.yaml b/app/metrics.yaml index 3995b1cb4588..d3757f7e283b 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -2479,6 +2479,22 @@ bookmarks_management: metadata: tags: - Bookmarks + open_all_in_new_tabs: + type: event + description: | + A user opened all the bookmarks in a folder in new tabs. + bugs: + - https://github.com/mozilla-mobile/fenix/issues/11404 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/27138 + data_sensitivity: + - interaction + notification_emails: + - android-probes@mozilla.com + expires: 120 + metadata: + tags: + - Bookmarks open_in_private_tab: type: event description: | @@ -2523,6 +2539,22 @@ bookmarks_management: metadata: tags: - Bookmarks + open_all_in_private_tabs: + type: event + description: | + A user opened all the bookmarks in a folder in new private tabs. + bugs: + - https://github.com/mozilla-mobile/fenix/issues/11404 + data_reviews: + - https://github.com/mozilla-mobile/fenix/pull/27138 + data_sensitivity: + - interaction + notification_emails: + - android-probes@mozilla.com + expires: 120 + 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 8b704f95e43e..80a966e839a8 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt @@ -260,6 +260,83 @@ class BookmarksTest { } } + @Test + fun openAllInTabsTest() { + val webPages = listOf( + TestAssetHelper.getGenericAsset(mockWebServer, 1), + TestAssetHelper.getGenericAsset(mockWebServer, 2), + TestAssetHelper.getGenericAsset(mockWebServer, 3), + TestAssetHelper.getGenericAsset(mockWebServer, 4), + ) + + homeScreen { + }.openThreeDotMenu { + }.openBookmarks { + createFolder("root") + createFolder("sub", "root") + createFolder("empty", "root") + }.closeMenu { + } + + browserScreen { + createBookmark(webPages[0].url) + createBookmark(webPages[1].url, "root") + createBookmark(webPages[2].url, "root") + createBookmark(webPages[3].url, "sub") + }.openTabDrawer { + closeTab() + } + + browserScreen { + }.openThreeDotMenu { + }.openBookmarks { + }.openThreeDotMenu("root") { + }.clickOpenAllInTabs { + verifyTabTrayIsOpened() + verifyNormalModeSelected() + + verifyExistingOpenTabs("Test_Page_2", "Test_Page_3", "Test_Page_4") + + // Bookmark that is not under the root folder should not be opened + verifyNoExistingOpenTabs("Test_Page_1") + } + } + + @Test + fun openAllInPrivateTabsTest() { + val webPages = listOf( + TestAssetHelper.getGenericAsset(mockWebServer, 1), + TestAssetHelper.getGenericAsset(mockWebServer, 2), + ) + + homeScreen { + }.openThreeDotMenu { + }.openBookmarks { + createFolder("root") + createFolder("sub", "root") + createFolder("empty", "root") + }.closeMenu { + } + + browserScreen { + createBookmark(webPages[0].url, "root") + createBookmark(webPages[1].url, "sub") + }.openTabDrawer { + closeTab() + } + + browserScreen { + }.openThreeDotMenu { + }.openBookmarks { + }.openThreeDotMenu("root") { + }.clickOpenAllInPrivateTabs { + verifyTabTrayIsOpened() + verifyPrivateModeSelected() + + verifyExistingOpenTabs("Test_Page_1", "Test_Page_2") + } + } + @Test fun openBookmarkInPrivateTabTest() { val defaultWebPage = TestAssetHelper.getGenericAsset(mockWebServer, 1) diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BookmarksRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BookmarksRobot.kt index 2ea293c604cb..27e3b1fb40fe 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BookmarksRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BookmarksRobot.kt @@ -150,12 +150,21 @@ class BookmarksRobot { .click() } - fun createFolder(name: String) { + fun createFolder(name: String, parent: String? = null) { clickAddFolderButton() addNewFolderName(name) + if (!parent.isNullOrBlank()) { + setParentFolder(parent) + } saveNewFolder() } + fun setParentFolder(parentName: String) { + clickParentFolderSelector() + selectFolder(parentName) + navigateUp() + } + fun clickAddFolderButton() { mDevice.waitNotNull( Until.findObject(By.desc("Add folder")), diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BrowserRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BrowserRobot.kt index fb5ed4a108c9..7a5d4341e653 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BrowserRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/BrowserRobot.kt @@ -340,13 +340,20 @@ class BrowserRobot { menuSaveImage.click() } - fun createBookmark(url: Uri) { + fun createBookmark(url: Uri, folder: String? = null) { navigationToolbar { }.enterURLAndEnterToBrowser(url) { // needs to wait for the right url to load before saving a bookmark verifyUrl(url.toString()) }.openThreeDotMenu { - }.bookmarkPage { } + }.bookmarkPage { + }.takeIf { !folder.isNullOrBlank() }?.let { + it.openThreeDotMenu { + }.editBookmarkPage { + setParentFolder(folder!!) + saveEditBookmark() + } + } } fun clickLinkMatchingText(expectedText: String) = diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/TabDrawerRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/TabDrawerRobot.kt index 6f55f6bb6253..c0488b4c9ce4 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/TabDrawerRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/TabDrawerRobot.kt @@ -33,6 +33,7 @@ import androidx.test.uiautomator.Until import androidx.test.uiautomator.Until.findObject import com.google.android.material.bottomsheet.BottomSheetBehavior import junit.framework.AssertionFailedError +import junit.framework.TestCase.assertFalse import junit.framework.TestCase.assertTrue import org.hamcrest.CoreMatchers.allOf import org.hamcrest.CoreMatchers.anyOf @@ -78,6 +79,7 @@ class TabDrawerRobot { assertSyncedTabsButtonIsSelected(isSelected) fun verifyExistingOpenTabs(vararg titles: String) = assertExistingOpenTabs(*titles) + fun verifyNoExistingOpenTabs(vararg titles: String) = assertNoExistingOpenTabs(*titles) fun verifyCloseTabsButton(title: String) = assertCloseTabsButton(title) fun verifyExistingTabList() = assertExistingTabList() @@ -490,6 +492,14 @@ private fun assertExistingOpenTabs(vararg tabTitles: String) { } } +private fun assertNoExistingOpenTabs(vararg tabTitles: String) { + for (title in tabTitles) { + assertFalse( + tabItem(title).waitForExists(waitingTimeLong), + ) + } +} + private fun assertExistingTabList() { mDevice.findObject( UiSelector().resourceId("$packageName:id/tabsTray"), 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 2c13ba626bda..035aa87ffa4d 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 @@ -52,6 +52,20 @@ class ThreeDotMenuBookmarksRobot { return TabDrawerRobot.Transition() } + fun clickOpenAllInTabs(interact: TabDrawerRobot.() -> Unit): TabDrawerRobot.Transition { + openAllInTabsButton().click() + + TabDrawerRobot().interact() + 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() @@ -71,4 +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 in new tabs")) + +private fun openAllInPrivateTabsButton() = onView(withText("Open all in private tabs")) + private fun deleteButton() = onView(withText("Delete")) diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuMainRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuMainRobot.kt index 008d46869ac5..1e5f6f114923 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuMainRobot.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuMainRobot.kt @@ -207,6 +207,14 @@ class ThreeDotMenuMainRobot { return BrowserRobot.Transition() } + fun editBookmarkPage(interact: BookmarksRobot.() -> Unit): BookmarksRobot.Transition { + mDevice.waitNotNull(Until.findObject(By.text("Bookmarks")), waitingTime) + editBookmarkButton().click() + + BookmarksRobot().interact() + return BookmarksRobot.Transition() + } + fun openHelp(interact: BrowserRobot.() -> Unit): BrowserRobot.Transition { mDevice.waitNotNull(Until.findObject(By.text("Help")), waitingTime) helpButton().click() 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 4f96e26ac694..c3da16fb7451 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 @@ -7,6 +7,7 @@ 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 @@ -15,6 +16,7 @@ import mozilla.appservices.places.BookmarkRoot import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.concept.storage.BookmarkNode +import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.service.fxa.sync.SyncReason import org.mozilla.fenix.BrowserDirection @@ -27,6 +29,9 @@ import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.navigateSafe import org.mozilla.fenix.utils.Settings +@VisibleForTesting +internal const val WARN_OPEN_ALL_SIZE = 15 + /** * [BookmarkFragment] controller. * Delegated by View Interactors, handles container business logic and operates changes on it. @@ -44,6 +49,7 @@ interface BookmarkController { fun handleCopyUrl(item: BookmarkNode) fun handleBookmarkSharing(item: BookmarkNode) fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode) + fun handleOpeningFolderBookmarks(folder: BookmarkNode, mode: BrowsingMode) /** * Handle bookmark nodes deletion @@ -73,11 +79,12 @@ class DefaultBookmarkController( private val store: BookmarkFragmentStore, private val sharedViewModel: BookmarksSharedViewModel, private val tabsUseCases: TabsUseCases?, - private val loadBookmarkNode: suspend (String) -> BookmarkNode?, + private val loadBookmarkNode: suspend (String, Boolean) -> BookmarkNode?, private val showSnackbar: (String) -> Unit, private val deleteBookmarkNodes: (Set, BookmarkRemoveType) -> Unit, private val deleteBookmarkFolder: (Set) -> Unit, private val showTabTray: () -> Unit, + private val warnLargeOpenAll: (Int, () -> Unit) -> Unit, private val settings: Settings, ) : BookmarkController { @@ -105,7 +112,7 @@ class DefaultBookmarkController( override fun handleBookmarkExpand(folder: BookmarkNode) { handleAllBookmarksDeselected() scope.launch { - val node = loadBookmarkNode.invoke(folder.guid) ?: return@launch + val node = loadBookmarkNode.invoke(folder.guid, false) ?: return@launch sharedViewModel.selectedFolder = node store.dispatch(BookmarkFragmentAction.Change(node)) } @@ -158,6 +165,53 @@ class DefaultBookmarkController( showTabTray() } + private fun extractURLsFromTree(node: BookmarkNode): MutableList { + val urls = mutableListOf() + + when (node.type) { + BookmarkNodeType.FOLDER -> { + node.children?.forEach { + urls.addAll(extractURLsFromTree(it)) + } + } + BookmarkNodeType.ITEM -> { + node.url?.let { urls.add(it) } + } + BookmarkNodeType.SEPARATOR -> {} + } + + return urls + } + + override fun handleOpeningFolderBookmarks(folder: BookmarkNode, mode: BrowsingMode) { + scope.launch { + 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 maximum number of bookmarks are being opened + if (urls.size >= WARN_OPEN_ALL_SIZE) { + warnLargeOpenAll(urls.size) { + openAll(false) + } + } else { + openAll(true) + } + } + } + override fun handleBookmarkDeletion(nodes: Set, removeType: BookmarkRemoveType) { deleteBookmarkNodes(nodes, removeType) } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt index 5eb7f3898d4c..1f9ff22ce3ed 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt @@ -106,6 +106,7 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan deleteBookmarkNodes = ::deleteMulti, deleteBookmarkFolder = ::showRemoveFolderDialog, showTabTray = ::showTabTray, + warnLargeOpenAll = ::warnLargeOpenAll, settings = requireComponents.settings, ), ) @@ -272,11 +273,11 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan return bookmarkView.onBackPressed() } - private suspend fun loadBookmarkNode(guid: String): BookmarkNode? = withContext(IO) { + private suspend fun loadBookmarkNode(guid: String, recursive: Boolean = false): BookmarkNode? = withContext(IO) { // Only runs if the fragment is attached same as [runIfFragmentIsAttached] context?.let { requireContext().bookmarkStorage - .getTree(guid, false) + .getTree(guid, recursive) ?.let { desktopFolders.withOptionalDesktopFolders(it) } } } @@ -293,6 +294,27 @@ class BookmarkFragment : LibraryPageFragment(), UserInteractionHan } } + private fun warnLargeOpenAll(numberOfTabs: Int, function: () -> (Unit)) { + AlertDialog.Builder(requireContext()).apply { + setTitle(String.format(context.getString(R.string.open_all_warning_title), numberOfTabs)) + setMessage(context.getString(R.string.open_all_warning_message, context.getString(R.string.app_name))) + setPositiveButton( + R.string.open_all_warning_confirm, + ) { dialog, _ -> + function() + dialog.dismiss() + } + setNegativeButton( + R.string.open_all_warning_cancel, + ) { dialog: DialogInterface, _ -> + dialog.dismiss() + } + setCancelable(false) + create() + show() + } + } + private fun deleteMulti( selected: Set, eventType: BookmarkRemoveType = BookmarkRemoveType.MULTIPLE, 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 4fec702ac7f5..a0da5d34d22a 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 @@ -80,6 +80,16 @@ class BookmarkFragmentInteractor( } } + override fun onOpenAllInNewTabs(folder: BookmarkNode) { + require(folder.type == BookmarkNodeType.FOLDER) + bookmarksController.handleOpeningFolderBookmarks(folder, BrowsingMode.Normal) + } + + override fun onOpenAllInPrivateTabs(folder: BookmarkNode) { + require(folder.type == BookmarkNodeType.FOLDER) + bookmarksController.handleOpeningFolderBookmarks(folder, BrowsingMode.Private) + } + override fun onDelete(nodes: Set) { if (nodes.find { it.type == BookmarkNodeType.SEPARATOR } != null) { throw IllegalStateException("Cannot delete separators") 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 e437490ca2de..533f1878cbcb 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 @@ -13,6 +13,7 @@ import mozilla.components.concept.menu.candidate.TextStyle import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.support.ktx.android.content.getColorFromAttr import org.mozilla.fenix.R +import org.mozilla.fenix.ext.bookmarkStorage class BookmarkItemMenu( private val context: Context, @@ -25,6 +26,8 @@ class BookmarkItemMenu( Share, OpenInNewTab, OpenInPrivateTab, + OpenAllInNewTabs, + OpenAllInPrivateTabs, Delete, ; } @@ -32,7 +35,10 @@ class BookmarkItemMenu( val menuController: MenuController by lazy { BrowserMenuController() } @VisibleForTesting - internal fun menuItems(itemType: BookmarkNodeType): List { + @SuppressWarnings("LongMethod") + internal suspend fun menuItems(itemType: BookmarkNodeType, itemId: String): List { + val hasAtLeastOneChild = !context.bookmarkStorage.getTree(itemId)?.children.isNullOrEmpty() + return listOfNotNull( if (itemType != BookmarkNodeType.SEPARATOR) { TextMenuCandidate( @@ -79,6 +85,24 @@ class BookmarkItemMenu( } else { null }, + if (hasAtLeastOneChild && itemType == BookmarkNodeType.FOLDER) { + TextMenuCandidate( + text = context.getString(R.string.bookmark_menu_open_all_in_tabs_button), + ) { + onItemTapped.invoke(Item.OpenAllInNewTabs) + } + } else { + null + }, + if (hasAtLeastOneChild && itemType == BookmarkNodeType.FOLDER) { + TextMenuCandidate( + text = context.getString(R.string.bookmark_menu_open_all_in_private_tabs_button), + ) { + onItemTapped.invoke(Item.OpenAllInPrivateTabs) + } + } else { + null + }, TextMenuCandidate( text = context.getString(R.string.bookmark_menu_delete_button), textStyle = TextStyle(color = context.getColorFromAttr(R.attr.textWarning)), @@ -88,7 +112,10 @@ class BookmarkItemMenu( ) } - fun updateMenu(itemType: BookmarkNodeType) { - menuController.submitList(menuItems(itemType)) + /** + * Update the menu items for the type of bookmark. + */ + suspend fun updateMenu(itemType: BookmarkNodeType, itemId: String) { + menuController.submitList(menuItems(itemType, itemId)) } } 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 b6cbcf292f04..5c5c018ae5e9 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 @@ -79,6 +79,20 @@ interface BookmarkViewInteractor : SelectionInteractor { */ fun onOpenInPrivateTab(item: BookmarkNode) + /** + * Opens all bookmark items in new tabs. + * + * @param folder the bookmark folder containing all items to open in new tabs + */ + fun onOpenAllInNewTabs(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 dc1d8c8b9bc7..e071ae6f4b57 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 @@ -6,6 +6,9 @@ package org.mozilla.fenix.library.bookmarks.viewholders import androidx.core.view.isVisible import androidx.recyclerview.widget.RecyclerView +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType import org.mozilla.fenix.R @@ -42,6 +45,8 @@ class BookmarkNodeViewHolder( BookmarkItemMenu.Item.Share -> interactor.onSharePressed(item) BookmarkItemMenu.Item.OpenInNewTab -> interactor.onOpenInNormalTab(item) BookmarkItemMenu.Item.OpenInPrivateTab -> interactor.onOpenInPrivateTab(item) + BookmarkItemMenu.Item.OpenAllInNewTabs -> interactor.onOpenAllInNewTabs(item) + BookmarkItemMenu.Item.OpenAllInPrivateTabs -> interactor.onOpenAllInPrivateTabs(item) BookmarkItemMenu.Item.Delete -> interactor.onDelete(setOf(item)) } } @@ -58,7 +63,10 @@ class BookmarkNodeViewHolder( containerView.urlView.isVisible = item.type == BookmarkNodeType.ITEM containerView.setSelectionInteractor(item, mode, interactor) - menu.updateMenu(item.type) + + CoroutineScope(Dispatchers.Default).launch { + menu.updateMenu(item.type, item.guid) + } // Hide menu button if this item is a root folder or is selected if (item.type == BookmarkNodeType.FOLDER && item.inRoots()) { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index df656d45239c..2ade8fc1dd20 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -615,6 +615,17 @@ Close + + Open %d tabs? + + Opening this many tabs may slow down %s while the pages are loading. Are you sure you want to continue? + + Open tabs + + Cancel + %d site @@ -837,6 +848,10 @@ Open in new tab 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 510389f5f6a3..dc81cbf2f39b 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 @@ -17,9 +17,11 @@ 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 @@ -87,6 +89,16 @@ class BookmarkControllerTest { 0, listOf(item, item, childItem, subfolder), ) + private val largeTree = BookmarkNode( + BookmarkNodeType.FOLDER, + "123", + null, + 0u, + "Mobile", + null, + 0, + List(WARN_OPEN_ALL_SIZE) { item }, + ) private val root = BookmarkNode( BookmarkNodeType.FOLDER, BookmarkRoot.Root.id, @@ -193,7 +205,7 @@ class BookmarkControllerTest { fun `handleBookmarkExpand should refresh and change the active bookmark node`() = runTestOnMain { var loadBookmarkNodeInvoked = false createController( - loadBookmarkNode = { + loadBookmarkNode = { _: String, _: Boolean -> loadBookmarkNodeInvoked = true tree }, @@ -358,6 +370,91 @@ class BookmarkControllerTest { } } + @Test + fun `WHEN handle opening folder bookmarks THEN all bookmarks in folder is opened in normal tabs`() { + var showTabTrayInvoked = false + createController( + showTabTray = { + showTabTrayInvoked = true + }, + loadBookmarkNode = { guid: String, _: Boolean -> + 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, guid) + }, + ).handleOpeningFolderBookmarks(tree, BrowsingMode.Normal) + + assertTrue(showTabTrayInvoked) + verifyOrder { + addNewTabUseCase.invoke(item.url!!, private = false) + addNewTabUseCase.invoke(item.url!!, private = false) + addNewTabUseCase.invoke(childItem.url!!, private = false) + homeActivity.browsingModeManager.mode = BrowsingMode.Normal + } + } + + @Test + fun `WHEN handle opening folder bookmarks in private tabs THEN all bookmarks in folder is opened in private tabs`() { + var showTabTrayInvoked = false + createController( + showTabTray = { + showTabTrayInvoked = true + }, + loadBookmarkNode = { guid: String, _: Boolean -> + 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, guid) + }, + ).handleOpeningFolderBookmarks(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 `WHEN handle opening folder bookmarks with more than max items THEN warning is invoked`() { + var warningInvoked = false + + mockkConstructor(DefaultBookmarkController::class) + createController( + loadBookmarkNode = { _: String, _: Boolean -> + largeTree + }, + warnLargeOpenAll = { _: Int, _: () -> Unit -> warningInvoked = true }, + ).handleOpeningFolderBookmarks(tree, BrowsingMode.Normal) + + unmockkConstructor(DefaultBookmarkController::class) + + assertTrue(warningInvoked) + } + @Test fun `handleBookmarkDeletion for an item should properly call a passed in delegate`() { var deleteBookmarkNodesInvoked = false @@ -426,11 +523,12 @@ class BookmarkControllerTest { @Suppress("LongParameterList") private fun createController( - loadBookmarkNode: suspend (String) -> BookmarkNode? = { _ -> null }, + loadBookmarkNode: suspend (String, Boolean) -> BookmarkNode? = { _, _ -> null }, showSnackbar: (String) -> Unit = { _ -> }, deleteBookmarkNodes: (Set, BookmarkRemoveType) -> Unit = { _, _ -> }, deleteBookmarkFolder: (Set) -> Unit = { _ -> }, showTabTray: () -> Unit = { }, + warnLargeOpenAll: (Int, () -> Unit) -> Unit = { _: Int, _: () -> Unit -> }, ): BookmarkController { return DefaultBookmarkController( activity = homeActivity, @@ -445,6 +543,7 @@ class BookmarkControllerTest { deleteBookmarkNodes = deleteBookmarkNodes, deleteBookmarkFolder = deleteBookmarkFolder, showTabTray = showTabTray, + warnLargeOpenAll = warnLargeOpenAll, settings = settings, ) } 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 a11357e9edae..8593cad3cc2d 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 @@ -140,6 +140,11 @@ class BookmarkFragmentInteractorTest { assertNull(BookmarksManagement.copied.testGetValue()!!.single().extra) } + @Test(expected = IllegalArgumentException::class) + fun `WHEN copying bookmark with folder THEN illegal argument exception is thrown`() { + interactor.onCopyPressed(tree) + } + @Test fun `share a bookmark item`() { interactor.onSharePressed(item) @@ -150,6 +155,11 @@ class BookmarkFragmentInteractorTest { assertNull(BookmarksManagement.shared.testGetValue()!!.single().extra) } + @Test(expected = IllegalArgumentException::class) + fun `WHEN sharing bookmark with folder THEN illegal argument exception is thrown`() { + interactor.onSharePressed(tree) + } + @Test fun `open a bookmark item in a new tab`() { interactor.onOpenInNormalTab(item) @@ -160,6 +170,11 @@ class BookmarkFragmentInteractorTest { assertNull(BookmarksManagement.openInNewTab.testGetValue()!!.single().extra) } + @Test(expected = IllegalArgumentException::class) + fun `WHEN open bookmark with folder THEN illegal argument exception is thrown`() { + interactor.onOpenInNormalTab(tree) + } + @Test fun `open a bookmark item in a private tab`() { interactor.onOpenInPrivateTab(item) @@ -170,6 +185,39 @@ class BookmarkFragmentInteractorTest { assertNull(BookmarksManagement.openInPrivateTab.testGetValue()!!.single().extra) } + @Test(expected = IllegalArgumentException::class) + fun `WHEN open bookmark in private with folder THEN illegal argument exception is thrown`() { + interactor.onOpenInPrivateTab(tree) + } + + @Test + fun `WHEN open all bookmarks THEN call handle opening folder bookmarks`() { + interactor.onOpenAllInNewTabs(tree) + + verify { + bookmarkController.handleOpeningFolderBookmarks(tree, BrowsingMode.Normal) + } + } + + @Test(expected = IllegalArgumentException::class) + fun `WHEN open all bookmarks with single item THEN illegal argument exception is thrown`() { + interactor.onOpenAllInNewTabs(item) + } + + @Test + fun `WHEN open all bookmarks in private tabs THEN call handle opening folder bookmarks with private mode`() { + interactor.onOpenAllInPrivateTabs(tree) + + verify { + bookmarkController.handleOpeningFolderBookmarks(tree, BrowsingMode.Private) + } + } + + @Test(expected = IllegalArgumentException::class) + fun `WHEN open all bookmarks in private with single item THEN illegal argument exception is thrown`() { + interactor.onOpenAllInPrivateTabs(item) + } + @Test fun `delete a bookmark item`() { interactor.onDelete(setOf(item)) 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 f251132bf516..f16d3cef4239 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 @@ -6,15 +6,23 @@ package org.mozilla.fenix.library.bookmarks import android.content.Context import androidx.appcompat.view.ContextThemeWrapper +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import kotlinx.coroutines.runBlocking +import mozilla.components.concept.menu.candidate.TextMenuCandidate import mozilla.components.concept.menu.candidate.TextStyle import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.support.ktx.android.content.getColorFromAttr import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.R +import org.mozilla.fenix.ext.bookmarkStorage import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.library.bookmarks.BookmarkItemMenu.Item @@ -34,37 +42,78 @@ class BookmarkItemMenuTest { } @Test - fun `delete item has special styling`() { - val deleteItem = menu.menuItems(BookmarkNodeType.SEPARATOR).last() - assertEquals("Delete", deleteItem.text) + fun `delete item has special styling`() = runBlocking { + var deleteItem: TextMenuCandidate? = null + mockkStatic("org.mozilla.fenix.ext.BookmarkNodeKt") { + every { any().bookmarkStorage } returns mockk(relaxed = true) + + deleteItem = menu.menuItems(BookmarkNodeType.SEPARATOR, "").last() + } + assertNotNull(deleteItem) + assertEquals("Delete", deleteItem!!.text) assertEquals( TextStyle(color = context.getColorFromAttr(R.attr.textWarning)), - deleteItem.textStyle, + deleteItem!!.textStyle, ) - deleteItem.onClick() + 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 + fun `edit item appears for folders`() = runBlocking { + // empty folder + var emptyFolderItems: List? = null + mockkStatic("org.mozilla.fenix.ext.BookmarkNodeKt") { + every { any().bookmarkStorage } returns mockk(relaxed = true) + + emptyFolderItems = menu.menuItems(BookmarkNodeType.FOLDER, "") + } + assertNotNull(emptyFolderItems) + assertEquals(2, emptyFolderItems!!.size) + + // not empty + var folderItems: List? = null + mockkStatic("org.mozilla.fenix.ext.BookmarkNodeKt") { + coEvery { any().bookmarkStorage.getTree("")?.children } returns listOf(mockk(relaxed = true)) + + folderItems = menu.menuItems(BookmarkNodeType.FOLDER, "") + } + assertNotNull(folderItems) + assertEquals(4, folderItems!!.size) + + val (edit, openAll, openAllPrivate, delete) = folderItems!! assertEquals("Edit", edit.text) - edit.onClick() + assertEquals("Open all in new tabs", openAll.text) + assertEquals("Open all in private tabs", openAllPrivate.text) + assertEquals("Delete", delete.text) + edit.onClick() assertEquals(Item.Edit, lastItemTapped) - assertEquals("Delete", delete.text) + + openAll.onClick() + assertEquals(Item.OpenAllInNewTabs, lastItemTapped) + + openAllPrivate.onClick() + assertEquals(Item.OpenAllInPrivateTabs, lastItemTapped) + + delete.onClick() + assertEquals(Item.Delete, lastItemTapped) } @Test - fun `all item appears for sites`() { - val siteItems = menu.menuItems(BookmarkNodeType.ITEM) - assertEquals(6, siteItems.size) - val (edit, copy, share, openInNewTab, openInPrivateTab, delete) = siteItems + fun `all item appears for sites`() = runBlocking { + var siteItems: List? = null + mockkStatic("org.mozilla.fenix.ext.BookmarkNodeKt") { + every { any().bookmarkStorage } returns mockk(relaxed = true) + + siteItems = menu.menuItems(BookmarkNodeType.ITEM, "") + } + assertNotNull(siteItems) + assertEquals(6, siteItems!!.size) + val (edit, copy, share, openInNewTab, openInPrivateTab, delete) = siteItems!! assertEquals("Edit", edit.text) assertEquals("Copy", copy.text) diff --git a/detekt-baseline.xml b/detekt-baseline.xml index 41a921e1ed3d..82d166a5fa41 100644 --- a/detekt-baseline.xml +++ b/detekt-baseline.xml @@ -543,11 +543,11 @@ UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleBookmarkTapped(item: BookmarkNode) UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleCopyUrl(item: BookmarkNode) UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode) + UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleOpeningFolderBookmarks(folder: BookmarkNode, mode: BrowsingMode) UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleRequestSync() UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleSearch() UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleSelectionModeSwitch() UndocumentedPublicFunction:BookmarkFragmentStore.kt$operator fun BookmarkNode.contains(item: BookmarkNode): Boolean - UndocumentedPublicFunction:BookmarkItemMenu.kt$BookmarkItemMenu$fun updateMenu(itemType: BookmarkNodeType) UndocumentedPublicFunction:BookmarkNodeViewHolder.kt$BookmarkNodeViewHolder$fun bind( item: BookmarkNode, mode: BookmarkFragmentState.Mode, payload: BookmarkPayload, ) UndocumentedPublicFunction:BookmarkSearchController.kt$BookmarkSearchController$fun handleEditingCancelled() UndocumentedPublicFunction:BookmarkSearchController.kt$BookmarkSearchController$fun handleTextChanged(text: String)