Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

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

Merged
merged 2 commits into from
Oct 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions app/metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down Expand Up @@ -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: |
Expand Down
77 changes: 77 additions & 0 deletions app/src/androidTest/java/org/mozilla/fenix/ui/BookmarksTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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"))
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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<BookmarkNode>, BookmarkRemoveType) -> Unit,
private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit,
private val showTabTray: () -> Unit,
private val warnLargeOpenAll: (Int, () -> Unit) -> Unit,
private val settings: Settings,
) : BookmarkController {

Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -158,6 +165,53 @@ class DefaultBookmarkController(
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) {
deleteBookmarkNodes(nodes, removeType)
}
Expand Down
Loading