For #15413: Use proper url when bookmarking a page.

app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt

@@ -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,
+                url = sessionUrl,
-                title = session.title,
+                title = sessionTitle,
                 position = null
             )
 

app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt

@@ -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 {

app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt

@@ -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