mirror of
https://github.com/fork-maintainers/iceraven-browser
synced 2024-11-19 09:25:34 +00:00
[fenix] For https://github.com/mozilla-mobile/fenix/issues/15413: Use proper url when bookmarking a page.
This commit is contained in:
parent
0f70a8d4fc
commit
51018be388
@ -321,7 +321,11 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
|
||||
browserAnimator = browserAnimator,
|
||||
customTabSession = customTabSessionId?.let { sessionManager.findSessionById(it) },
|
||||
openInFenixIntent = openInFenixIntent,
|
||||
bookmarkTapped = { viewLifecycleOwner.lifecycleScope.launch { bookmarkTapped(it) } },
|
||||
bookmarkTapped = { url: String, title: String ->
|
||||
viewLifecycleOwner.lifecycleScope.launch {
|
||||
bookmarkTapped(url, title)
|
||||
}
|
||||
},
|
||||
scope = viewLifecycleOwner.lifecycleScope,
|
||||
tabCollectionStorage = requireComponents.core.tabCollectionStorage,
|
||||
topSitesStorage = requireComponents.core.topSitesStorage,
|
||||
@ -1052,10 +1056,10 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun bookmarkTapped(session: Session) = withContext(IO) {
|
||||
private suspend fun bookmarkTapped(sessionUrl: String, sessionTitle: String) = withContext(IO) {
|
||||
val bookmarksStorage = requireComponents.core.bookmarksStorage
|
||||
val existing =
|
||||
bookmarksStorage.getBookmarksWithUrl(session.url).firstOrNull { it.url == session.url }
|
||||
bookmarksStorage.getBookmarksWithUrl(sessionUrl).firstOrNull { it.url == sessionUrl }
|
||||
if (existing != null) {
|
||||
// Bookmark exists, go to edit fragment
|
||||
withContext(Main) {
|
||||
@ -1068,8 +1072,8 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler,
|
||||
// Save bookmark, then go to edit fragment
|
||||
val guid = bookmarksStorage.addItem(
|
||||
BookmarkRoot.Mobile.id,
|
||||
url = session.url,
|
||||
title = session.title,
|
||||
url = sessionUrl,
|
||||
title = sessionTitle,
|
||||
position = null
|
||||
)
|
||||
|
||||
|
@ -65,7 +65,7 @@ class DefaultBrowserToolbarMenuController(
|
||||
private val swipeRefresh: SwipeRefreshLayout,
|
||||
private val customTabSession: Session?,
|
||||
private val openInFenixIntent: Intent,
|
||||
private val bookmarkTapped: (Session) -> Unit,
|
||||
private val bookmarkTapped: (String, String) -> Unit,
|
||||
private val scope: CoroutineScope,
|
||||
private val tabCollectionStorage: TabCollectionStorage,
|
||||
private val topSitesStorage: DefaultTopSitesStorage,
|
||||
@ -273,7 +273,7 @@ class DefaultBrowserToolbarMenuController(
|
||||
}
|
||||
ToolbarMenu.Item.Bookmark -> {
|
||||
sessionManager.selectedSession?.let {
|
||||
bookmarkTapped(it)
|
||||
getProperUrl(it)?.let { url -> bookmarkTapped(url, it.title) }
|
||||
}
|
||||
}
|
||||
ToolbarMenu.Item.Bookmarks -> browserAnimator.captureEngineViewAndDrawStatically {
|
||||
|
@ -73,7 +73,7 @@ class DefaultBrowserToolbarMenuControllerTest {
|
||||
@RelaxedMockK private lateinit var activity: HomeActivity
|
||||
@RelaxedMockK private lateinit var navController: NavController
|
||||
@RelaxedMockK private lateinit var findInPageLauncher: () -> Unit
|
||||
@RelaxedMockK private lateinit var bookmarkTapped: (Session) -> Unit
|
||||
@RelaxedMockK private lateinit var bookmarkTapped: (String, String) -> Unit
|
||||
@RelaxedMockK private lateinit var sessionManager: SessionManager
|
||||
@RelaxedMockK private lateinit var currentSession: Session
|
||||
@RelaxedMockK private lateinit var openInFenixIntent: Intent
|
||||
@ -224,14 +224,48 @@ class DefaultBrowserToolbarMenuControllerTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun handleToolbarBookmarkPress() = runBlockingTest {
|
||||
fun handleToolbarBookmarkPressWithReaderModeInactive() = runBlockingTest {
|
||||
val item = ToolbarMenu.Item.Bookmark
|
||||
val title = "Mozilla"
|
||||
val readerUrl = "moz-extension://1234"
|
||||
val readerTab = createTab(
|
||||
url = readerUrl,
|
||||
readerState = ReaderState(active = false, activeUrl = "https://1234.org"),
|
||||
title = title
|
||||
)
|
||||
browserStore =
|
||||
BrowserStore(BrowserState(tabs = listOf(readerTab), selectedTabId = readerTab.id))
|
||||
every { currentSession.id } returns readerTab.id
|
||||
every { currentSession.title } returns title
|
||||
every { currentSession.url } returns "https://mozilla.org"
|
||||
|
||||
val controller = createController(scope = this)
|
||||
controller.handleToolbarItemInteraction(item)
|
||||
|
||||
verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.BOOKMARK)) }
|
||||
verify { bookmarkTapped(currentSession) }
|
||||
verify { bookmarkTapped("https://mozilla.org", title) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun handleToolbarBookmarkPressWithReaderModeActive() = runBlockingTest {
|
||||
val item = ToolbarMenu.Item.Bookmark
|
||||
val title = "Mozilla"
|
||||
val readerUrl = "moz-extension://1234"
|
||||
val readerTab = createTab(
|
||||
url = readerUrl,
|
||||
readerState = ReaderState(active = true, activeUrl = "https://mozilla.org"),
|
||||
title = title
|
||||
)
|
||||
browserStore = BrowserStore(BrowserState(tabs = listOf(readerTab), selectedTabId = readerTab.id))
|
||||
every { currentSession.id } returns readerTab.id
|
||||
every { currentSession.title } returns title
|
||||
every { currentSession.url } returns readerUrl
|
||||
|
||||
val controller = createController(scope = this)
|
||||
controller.handleToolbarItemInteraction(item)
|
||||
|
||||
verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.BOOKMARK)) }
|
||||
verify { bookmarkTapped("https://mozilla.org", title) }
|
||||
}
|
||||
|
||||
@Test
|
||||
|
Loading…
Reference in New Issue
Block a user