For #11404 - Add 'Open all' options in bookmarks

* Add "Open all in new tabs" options in bookmarks
* Add "Open all in private tabs" options in bookmarks
* Add metrics tracking if the usage of "Open all in..." in bookmarks

Co-authored-by: Pg <pg.developper.fr@gmail.com>
pull/543/head
Roger Yang 2 years ago committed by mergify[bot]
parent adb9cfbb24
commit 489548b0de

@ -2479,6 +2479,22 @@ bookmarks_management:
metadata: metadata:
tags: tags:
- Bookmarks - 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: open_in_private_tab:
type: event type: event
description: | description: |
@ -2523,6 +2539,22 @@ bookmarks_management:
metadata: metadata:
tags: tags:
- Bookmarks - 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: edited:
type: event type: event
description: | description: |

@ -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 @Test
fun openBookmarkInPrivateTabTest() { fun openBookmarkInPrivateTabTest() {
val defaultWebPage = TestAssetHelper.getGenericAsset(mockWebServer, 1) val defaultWebPage = TestAssetHelper.getGenericAsset(mockWebServer, 1)

@ -150,12 +150,21 @@ class BookmarksRobot {
.click() .click()
} }
fun createFolder(name: String) { fun createFolder(name: String, parent: String? = null) {
clickAddFolderButton() clickAddFolderButton()
addNewFolderName(name) addNewFolderName(name)
if (!parent.isNullOrBlank()) {
setParentFolder(parent)
}
saveNewFolder() saveNewFolder()
} }
fun setParentFolder(parentName: String) {
clickParentFolderSelector()
selectFolder(parentName)
navigateUp()
}
fun clickAddFolderButton() { fun clickAddFolderButton() {
mDevice.waitNotNull( mDevice.waitNotNull(
Until.findObject(By.desc("Add folder")), Until.findObject(By.desc("Add folder")),

@ -340,13 +340,20 @@ class BrowserRobot {
menuSaveImage.click() menuSaveImage.click()
} }
fun createBookmark(url: Uri) { fun createBookmark(url: Uri, folder: String? = null) {
navigationToolbar { navigationToolbar {
}.enterURLAndEnterToBrowser(url) { }.enterURLAndEnterToBrowser(url) {
// needs to wait for the right url to load before saving a bookmark // needs to wait for the right url to load before saving a bookmark
verifyUrl(url.toString()) verifyUrl(url.toString())
}.openThreeDotMenu { }.openThreeDotMenu {
}.bookmarkPage { } }.bookmarkPage {
}.takeIf { !folder.isNullOrBlank() }?.let {
it.openThreeDotMenu {
}.editBookmarkPage {
setParentFolder(folder!!)
saveEditBookmark()
}
}
} }
fun clickLinkMatchingText(expectedText: String) = fun clickLinkMatchingText(expectedText: String) =

@ -33,6 +33,7 @@ import androidx.test.uiautomator.Until
import androidx.test.uiautomator.Until.findObject import androidx.test.uiautomator.Until.findObject
import com.google.android.material.bottomsheet.BottomSheetBehavior import com.google.android.material.bottomsheet.BottomSheetBehavior
import junit.framework.AssertionFailedError import junit.framework.AssertionFailedError
import junit.framework.TestCase.assertFalse
import junit.framework.TestCase.assertTrue import junit.framework.TestCase.assertTrue
import org.hamcrest.CoreMatchers.allOf import org.hamcrest.CoreMatchers.allOf
import org.hamcrest.CoreMatchers.anyOf import org.hamcrest.CoreMatchers.anyOf
@ -78,6 +79,7 @@ class TabDrawerRobot {
assertSyncedTabsButtonIsSelected(isSelected) assertSyncedTabsButtonIsSelected(isSelected)
fun verifyExistingOpenTabs(vararg titles: String) = assertExistingOpenTabs(*titles) fun verifyExistingOpenTabs(vararg titles: String) = assertExistingOpenTabs(*titles)
fun verifyNoExistingOpenTabs(vararg titles: String) = assertNoExistingOpenTabs(*titles)
fun verifyCloseTabsButton(title: String) = assertCloseTabsButton(title) fun verifyCloseTabsButton(title: String) = assertCloseTabsButton(title)
fun verifyExistingTabList() = assertExistingTabList() 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() { private fun assertExistingTabList() {
mDevice.findObject( mDevice.findObject(
UiSelector().resourceId("$packageName:id/tabsTray"), UiSelector().resourceId("$packageName:id/tabsTray"),

@ -52,6 +52,20 @@ class ThreeDotMenuBookmarksRobot {
return TabDrawerRobot.Transition() 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 { fun clickDelete(interact: BookmarksRobot.() -> Unit): BookmarksRobot.Transition {
deleteButton().click() 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 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")) private fun deleteButton() = onView(withText("Delete"))

@ -207,6 +207,14 @@ class ThreeDotMenuMainRobot {
return BrowserRobot.Transition() 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 { fun openHelp(interact: BrowserRobot.() -> Unit): BrowserRobot.Transition {
mDevice.waitNotNull(Until.findObject(By.text("Help")), waitingTime) mDevice.waitNotNull(Until.findObject(By.text("Help")), waitingTime)
helpButton().click() helpButton().click()

@ -7,6 +7,7 @@ package org.mozilla.fenix.library.bookmarks
import android.content.ClipData import android.content.ClipData
import android.content.ClipboardManager import android.content.ClipboardManager
import android.content.res.Resources import android.content.res.Resources
import androidx.annotation.VisibleForTesting
import androidx.navigation.NavController import androidx.navigation.NavController
import androidx.navigation.NavDirections import androidx.navigation.NavDirections
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
@ -15,6 +16,7 @@ import mozilla.appservices.places.BookmarkRoot
import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.concept.engine.prompt.ShareData
import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNode
import mozilla.components.concept.storage.BookmarkNodeType
import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.feature.tabs.TabsUseCases
import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.service.fxa.sync.SyncReason
import org.mozilla.fenix.BrowserDirection 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.ext.navigateSafe
import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.utils.Settings
@VisibleForTesting
internal const val WARN_OPEN_ALL_SIZE = 15
/** /**
* [BookmarkFragment] controller. * [BookmarkFragment] controller.
* Delegated by View Interactors, handles container business logic and operates changes on it. * Delegated by View Interactors, handles container business logic and operates changes on it.
@ -44,6 +49,7 @@ interface BookmarkController {
fun handleCopyUrl(item: BookmarkNode) fun handleCopyUrl(item: BookmarkNode)
fun handleBookmarkSharing(item: BookmarkNode) fun handleBookmarkSharing(item: BookmarkNode)
fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode) fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)
fun handleOpeningFolderBookmarks(folder: BookmarkNode, mode: BrowsingMode)
/** /**
* Handle bookmark nodes deletion * Handle bookmark nodes deletion
@ -73,11 +79,12 @@ class DefaultBookmarkController(
private val store: BookmarkFragmentStore, private val store: BookmarkFragmentStore,
private val sharedViewModel: BookmarksSharedViewModel, private val sharedViewModel: BookmarksSharedViewModel,
private val tabsUseCases: TabsUseCases?, private val tabsUseCases: TabsUseCases?,
private val loadBookmarkNode: suspend (String) -> BookmarkNode?, private val loadBookmarkNode: suspend (String, Boolean) -> BookmarkNode?,
private val showSnackbar: (String) -> Unit, private val showSnackbar: (String) -> Unit,
private val deleteBookmarkNodes: (Set<BookmarkNode>, BookmarkRemoveType) -> Unit, private val deleteBookmarkNodes: (Set<BookmarkNode>, BookmarkRemoveType) -> Unit,
private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit, private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit,
private val showTabTray: () -> Unit, private val showTabTray: () -> Unit,
private val warnLargeOpenAll: (Int, () -> Unit) -> Unit,
private val settings: Settings, private val settings: Settings,
) : BookmarkController { ) : BookmarkController {
@ -105,7 +112,7 @@ class DefaultBookmarkController(
override fun handleBookmarkExpand(folder: BookmarkNode) { override fun handleBookmarkExpand(folder: BookmarkNode) {
handleAllBookmarksDeselected() handleAllBookmarksDeselected()
scope.launch { scope.launch {
val node = loadBookmarkNode.invoke(folder.guid) ?: return@launch val node = loadBookmarkNode.invoke(folder.guid, false) ?: return@launch
sharedViewModel.selectedFolder = node sharedViewModel.selectedFolder = node
store.dispatch(BookmarkFragmentAction.Change(node)) store.dispatch(BookmarkFragmentAction.Change(node))
} }
@ -158,6 +165,53 @@ class DefaultBookmarkController(
showTabTray() showTabTray()
} }
private fun extractURLsFromTree(node: BookmarkNode): MutableList<String> {
val urls = mutableListOf<String>()
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<BookmarkNode>, removeType: BookmarkRemoveType) { override fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, removeType: BookmarkRemoveType) {
deleteBookmarkNodes(nodes, removeType) deleteBookmarkNodes(nodes, removeType)
} }

@ -106,6 +106,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
deleteBookmarkNodes = ::deleteMulti, deleteBookmarkNodes = ::deleteMulti,
deleteBookmarkFolder = ::showRemoveFolderDialog, deleteBookmarkFolder = ::showRemoveFolderDialog,
showTabTray = ::showTabTray, showTabTray = ::showTabTray,
warnLargeOpenAll = ::warnLargeOpenAll,
settings = requireComponents.settings, settings = requireComponents.settings,
), ),
) )
@ -272,11 +273,11 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
return bookmarkView.onBackPressed() 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] // Only runs if the fragment is attached same as [runIfFragmentIsAttached]
context?.let { context?.let {
requireContext().bookmarkStorage requireContext().bookmarkStorage
.getTree(guid, false) .getTree(guid, recursive)
?.let { desktopFolders.withOptionalDesktopFolders(it) } ?.let { desktopFolders.withOptionalDesktopFolders(it) }
} }
} }
@ -293,6 +294,27 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), 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( private fun deleteMulti(
selected: Set<BookmarkNode>, selected: Set<BookmarkNode>,
eventType: BookmarkRemoveType = BookmarkRemoveType.MULTIPLE, eventType: BookmarkRemoveType = BookmarkRemoveType.MULTIPLE,

@ -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<BookmarkNode>) { override fun onDelete(nodes: Set<BookmarkNode>) {
if (nodes.find { it.type == BookmarkNodeType.SEPARATOR } != null) { if (nodes.find { it.type == BookmarkNodeType.SEPARATOR } != null) {
throw IllegalStateException("Cannot delete separators") throw IllegalStateException("Cannot delete separators")

@ -13,6 +13,7 @@ import mozilla.components.concept.menu.candidate.TextStyle
import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.concept.storage.BookmarkNodeType
import mozilla.components.support.ktx.android.content.getColorFromAttr import mozilla.components.support.ktx.android.content.getColorFromAttr
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.ext.bookmarkStorage
class BookmarkItemMenu( class BookmarkItemMenu(
private val context: Context, private val context: Context,
@ -25,6 +26,8 @@ class BookmarkItemMenu(
Share, Share,
OpenInNewTab, OpenInNewTab,
OpenInPrivateTab, OpenInPrivateTab,
OpenAllInNewTabs,
OpenAllInPrivateTabs,
Delete, Delete,
; ;
} }
@ -32,7 +35,10 @@ class BookmarkItemMenu(
val menuController: MenuController by lazy { BrowserMenuController() } val menuController: MenuController by lazy { BrowserMenuController() }
@VisibleForTesting @VisibleForTesting
internal fun menuItems(itemType: BookmarkNodeType): List<TextMenuCandidate> { @SuppressWarnings("LongMethod")
internal suspend fun menuItems(itemType: BookmarkNodeType, itemId: String): List<TextMenuCandidate> {
val hasAtLeastOneChild = !context.bookmarkStorage.getTree(itemId)?.children.isNullOrEmpty()
return listOfNotNull( return listOfNotNull(
if (itemType != BookmarkNodeType.SEPARATOR) { if (itemType != BookmarkNodeType.SEPARATOR) {
TextMenuCandidate( TextMenuCandidate(
@ -79,6 +85,24 @@ class BookmarkItemMenu(
} else { } else {
null 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( TextMenuCandidate(
text = context.getString(R.string.bookmark_menu_delete_button), text = context.getString(R.string.bookmark_menu_delete_button),
textStyle = TextStyle(color = context.getColorFromAttr(R.attr.textWarning)), 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))
} }
} }

@ -79,6 +79,20 @@ interface BookmarkViewInteractor : SelectionInteractor<BookmarkNode> {
*/ */
fun onOpenInPrivateTab(item: BookmarkNode) 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. * Deletes a set of bookmark nodes.
* *

@ -6,6 +6,9 @@ package org.mozilla.fenix.library.bookmarks.viewholders
import androidx.core.view.isVisible import androidx.core.view.isVisible
import androidx.recyclerview.widget.RecyclerView 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.BookmarkNode
import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.concept.storage.BookmarkNodeType
import org.mozilla.fenix.R import org.mozilla.fenix.R
@ -42,6 +45,8 @@ class BookmarkNodeViewHolder(
BookmarkItemMenu.Item.Share -> interactor.onSharePressed(item) BookmarkItemMenu.Item.Share -> interactor.onSharePressed(item)
BookmarkItemMenu.Item.OpenInNewTab -> interactor.onOpenInNormalTab(item) BookmarkItemMenu.Item.OpenInNewTab -> interactor.onOpenInNormalTab(item)
BookmarkItemMenu.Item.OpenInPrivateTab -> interactor.onOpenInPrivateTab(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)) BookmarkItemMenu.Item.Delete -> interactor.onDelete(setOf(item))
} }
} }
@ -58,7 +63,10 @@ class BookmarkNodeViewHolder(
containerView.urlView.isVisible = item.type == BookmarkNodeType.ITEM containerView.urlView.isVisible = item.type == BookmarkNodeType.ITEM
containerView.setSelectionInteractor(item, mode, interactor) 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 // Hide menu button if this item is a root folder or is selected
if (item.type == BookmarkNodeType.FOLDER && item.inRoots()) { if (item.type == BookmarkNodeType.FOLDER && item.inRoots()) {

@ -615,6 +615,17 @@
<!-- Content description (not visible, for screen readers etc.): "Close button for library settings" --> <!-- Content description (not visible, for screen readers etc.): "Close button for library settings" -->
<string name="content_description_close_button">Close</string> <string name="content_description_close_button">Close</string>
<!-- Title to show in alert when a lot of tabs are to be opened
%d is a placeholder for the number of tabs that will be opened -->
<string name="open_all_warning_title">Open %d tabs?</string>
<!-- Message to warn users that a large number of tabs will be opened
%s will be replaced by app name. -->
<string name="open_all_warning_message">Opening this many tabs may slow down %s while the pages are loading. 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. <!-- 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. --> %d is a placeholder for the number of sites in the group. -->
<string name="history_search_group_site">%d site</string> <string name="history_search_group_site">%d site</string>
@ -837,6 +848,10 @@
<string name="bookmark_menu_open_in_new_tab_button">Open in new tab</string> <string name="bookmark_menu_open_in_new_tab_button">Open in new tab</string>
<!-- Bookmark overflow menu open in private tab button --> <!-- Bookmark overflow menu open in private tab button -->
<string name="bookmark_menu_open_in_private_tab_button">Open in private tab</string> <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_private_tabs_button">Open all in private tabs</string>
<!-- Bookmark overflow menu delete button --> <!-- Bookmark overflow menu delete button -->
<string name="bookmark_menu_delete_button">Delete</string> <string name="bookmark_menu_delete_button">Delete</string>
<!--Bookmark overflow menu save button --> <!--Bookmark overflow menu save button -->

@ -17,9 +17,11 @@ import io.mockk.coVerify
import io.mockk.every import io.mockk.every
import io.mockk.just import io.mockk.just
import io.mockk.mockk import io.mockk.mockk
import io.mockk.mockkConstructor
import io.mockk.runs import io.mockk.runs
import io.mockk.slot import io.mockk.slot
import io.mockk.spyk import io.mockk.spyk
import io.mockk.unmockkConstructor
import io.mockk.verify import io.mockk.verify
import io.mockk.verifyOrder import io.mockk.verifyOrder
import mozilla.appservices.places.BookmarkRoot import mozilla.appservices.places.BookmarkRoot
@ -87,6 +89,16 @@ class BookmarkControllerTest {
0, 0,
listOf(item, item, childItem, subfolder), 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( private val root = BookmarkNode(
BookmarkNodeType.FOLDER, BookmarkNodeType.FOLDER,
BookmarkRoot.Root.id, BookmarkRoot.Root.id,
@ -193,7 +205,7 @@ class BookmarkControllerTest {
fun `handleBookmarkExpand should refresh and change the active bookmark node`() = runTestOnMain { fun `handleBookmarkExpand should refresh and change the active bookmark node`() = runTestOnMain {
var loadBookmarkNodeInvoked = false var loadBookmarkNodeInvoked = false
createController( createController(
loadBookmarkNode = { loadBookmarkNode = { _: String, _: Boolean ->
loadBookmarkNodeInvoked = true loadBookmarkNodeInvoked = true
tree 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 @Test
fun `handleBookmarkDeletion for an item should properly call a passed in delegate`() { fun `handleBookmarkDeletion for an item should properly call a passed in delegate`() {
var deleteBookmarkNodesInvoked = false var deleteBookmarkNodesInvoked = false
@ -426,11 +523,12 @@ class BookmarkControllerTest {
@Suppress("LongParameterList") @Suppress("LongParameterList")
private fun createController( private fun createController(
loadBookmarkNode: suspend (String) -> BookmarkNode? = { _ -> null }, loadBookmarkNode: suspend (String, Boolean) -> BookmarkNode? = { _, _ -> null },
showSnackbar: (String) -> Unit = { _ -> }, showSnackbar: (String) -> Unit = { _ -> },
deleteBookmarkNodes: (Set<BookmarkNode>, BookmarkRemoveType) -> Unit = { _, _ -> }, deleteBookmarkNodes: (Set<BookmarkNode>, BookmarkRemoveType) -> Unit = { _, _ -> },
deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit = { _ -> }, deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit = { _ -> },
showTabTray: () -> Unit = { }, showTabTray: () -> Unit = { },
warnLargeOpenAll: (Int, () -> Unit) -> Unit = { _: Int, _: () -> Unit -> },
): BookmarkController { ): BookmarkController {
return DefaultBookmarkController( return DefaultBookmarkController(
activity = homeActivity, activity = homeActivity,
@ -445,6 +543,7 @@ class BookmarkControllerTest {
deleteBookmarkNodes = deleteBookmarkNodes, deleteBookmarkNodes = deleteBookmarkNodes,
deleteBookmarkFolder = deleteBookmarkFolder, deleteBookmarkFolder = deleteBookmarkFolder,
showTabTray = showTabTray, showTabTray = showTabTray,
warnLargeOpenAll = warnLargeOpenAll,
settings = settings, settings = settings,
) )
} }

@ -140,6 +140,11 @@ class BookmarkFragmentInteractorTest {
assertNull(BookmarksManagement.copied.testGetValue()!!.single().extra) 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 @Test
fun `share a bookmark item`() { fun `share a bookmark item`() {
interactor.onSharePressed(item) interactor.onSharePressed(item)
@ -150,6 +155,11 @@ class BookmarkFragmentInteractorTest {
assertNull(BookmarksManagement.shared.testGetValue()!!.single().extra) 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 @Test
fun `open a bookmark item in a new tab`() { fun `open a bookmark item in a new tab`() {
interactor.onOpenInNormalTab(item) interactor.onOpenInNormalTab(item)
@ -160,6 +170,11 @@ class BookmarkFragmentInteractorTest {
assertNull(BookmarksManagement.openInNewTab.testGetValue()!!.single().extra) 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 @Test
fun `open a bookmark item in a private tab`() { fun `open a bookmark item in a private tab`() {
interactor.onOpenInPrivateTab(item) interactor.onOpenInPrivateTab(item)
@ -170,6 +185,39 @@ class BookmarkFragmentInteractorTest {
assertNull(BookmarksManagement.openInPrivateTab.testGetValue()!!.single().extra) 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 @Test
fun `delete a bookmark item`() { fun `delete a bookmark item`() {
interactor.onDelete(setOf(item)) interactor.onDelete(setOf(item))

@ -6,15 +6,23 @@ package org.mozilla.fenix.library.bookmarks
import android.content.Context import android.content.Context
import androidx.appcompat.view.ContextThemeWrapper 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.menu.candidate.TextStyle
import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.concept.storage.BookmarkNodeType
import mozilla.components.support.ktx.android.content.getColorFromAttr import mozilla.components.support.ktx.android.content.getColorFromAttr
import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.robolectric.testContext
import org.junit.Assert.assertEquals import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Before import org.junit.Before
import org.junit.Test import org.junit.Test
import org.junit.runner.RunWith import org.junit.runner.RunWith
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.ext.bookmarkStorage
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
import org.mozilla.fenix.library.bookmarks.BookmarkItemMenu.Item import org.mozilla.fenix.library.bookmarks.BookmarkItemMenu.Item
@ -34,37 +42,78 @@ class BookmarkItemMenuTest {
} }
@Test @Test
fun `delete item has special styling`() { fun `delete item has special styling`() = runBlocking {
val deleteItem = menu.menuItems(BookmarkNodeType.SEPARATOR).last() var deleteItem: TextMenuCandidate? = null
assertEquals("Delete", deleteItem.text) mockkStatic("org.mozilla.fenix.ext.BookmarkNodeKt") {
every { any<Context>().bookmarkStorage } returns mockk(relaxed = true)
deleteItem = menu.menuItems(BookmarkNodeType.SEPARATOR, "").last()
}
assertNotNull(deleteItem)
assertEquals("Delete", deleteItem!!.text)
assertEquals( assertEquals(
TextStyle(color = context.getColorFromAttr(R.attr.textWarning)), TextStyle(color = context.getColorFromAttr(R.attr.textWarning)),
deleteItem.textStyle, deleteItem!!.textStyle,
) )
deleteItem.onClick() deleteItem!!.onClick()
assertEquals(Item.Delete, lastItemTapped) assertEquals(Item.Delete, lastItemTapped)
} }
@Test @Test
fun `edit item appears for folders`() { fun `edit item appears for folders`() = runBlocking {
val folderItems = menu.menuItems(BookmarkNodeType.FOLDER) // empty folder
assertEquals(2, folderItems.size) var emptyFolderItems: List<TextMenuCandidate>? = null
val (edit, delete) = folderItems mockkStatic("org.mozilla.fenix.ext.BookmarkNodeKt") {
every { any<Context>().bookmarkStorage } returns mockk(relaxed = true)
emptyFolderItems = menu.menuItems(BookmarkNodeType.FOLDER, "")
}
assertNotNull(emptyFolderItems)
assertEquals(2, emptyFolderItems!!.size)
// not empty
var folderItems: List<TextMenuCandidate>? = null
mockkStatic("org.mozilla.fenix.ext.BookmarkNodeKt") {
coEvery { any<Context>().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) 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(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 @Test
fun `all item appears for sites`() { fun `all item appears for sites`() = runBlocking {
val siteItems = menu.menuItems(BookmarkNodeType.ITEM) var siteItems: List<TextMenuCandidate>? = null
assertEquals(6, siteItems.size) mockkStatic("org.mozilla.fenix.ext.BookmarkNodeKt") {
val (edit, copy, share, openInNewTab, openInPrivateTab, delete) = siteItems every { any<Context>().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("Edit", edit.text)
assertEquals("Copy", copy.text) assertEquals("Copy", copy.text)

@ -543,11 +543,11 @@
<ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleBookmarkTapped(item: BookmarkNode)</ID> <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 handleCopyUrl(item: BookmarkNode)</ID>
<ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)</ID> <ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)</ID>
<ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleOpeningFolderBookmarks(folder: BookmarkNode, mode: BrowsingMode)</ID>
<ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleRequestSync()</ID> <ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleRequestSync()</ID>
<ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleSearch()</ID> <ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleSearch()</ID>
<ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleSelectionModeSwitch()</ID> <ID>UndocumentedPublicFunction:BookmarkController.kt$BookmarkController$fun handleSelectionModeSwitch()</ID>
<ID>UndocumentedPublicFunction:BookmarkFragmentStore.kt$operator fun BookmarkNode.contains(item: BookmarkNode): Boolean</ID> <ID>UndocumentedPublicFunction:BookmarkFragmentStore.kt$operator fun BookmarkNode.contains(item: BookmarkNode): Boolean</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: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 handleEditingCancelled()</ID>
<ID>UndocumentedPublicFunction:BookmarkSearchController.kt$BookmarkSearchController$fun handleTextChanged(text: String)</ID> <ID>UndocumentedPublicFunction:BookmarkSearchController.kt$BookmarkSearchController$fun handleTextChanged(text: String)</ID>

Loading…
Cancel
Save