ishan
/
fenix
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert "For #11404 - Add 'Open all in private tabs' option as requested." 

This reverts commit fe16a81c1d.
This commit is contained in:
Roger Yang 2022-09-21 18:07:51 -04:00 committed by mergify[bot]
parent cfa376e5d2
commit dff4c31606
12 changed files with 40 additions and 157 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
app/metrics.yaml
View File

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

2
app/src/main/res/values/strings.xml
View File

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

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

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

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

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

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

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