From 047b712ffe8e584c98c1d3a5f1c318aad57301f3 Mon Sep 17 00:00:00 2001 From: mcarare Date: Mon, 26 Oct 2020 16:37:38 +0200 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/15379: Use proper url when sharing. --- .../fenix/browser/BaseBrowserFragment.kt | 3 +- .../toolbar/BrowserToolbarMenuController.kt | 18 +++++- ...DefaultBrowserToolbarMenuControllerTest.kt | 63 ++++++++++++++++--- 3 files changed, 73 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 5323c35f08..797fc143d9 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -316,7 +316,8 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Session bookmarkTapped = { viewLifecycleOwner.lifecycleScope.launch { bookmarkTapped(it) } }, scope = viewLifecycleOwner.lifecycleScope, tabCollectionStorage = requireComponents.core.tabCollectionStorage, - topSitesStorage = requireComponents.core.topSitesStorage + topSitesStorage = requireComponents.core.topSitesStorage, + browserStore = store ) _browserInteractor = BrowserInteractor( diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt index 4fd69b08e0..9c0734c373 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt @@ -17,6 +17,8 @@ import kotlinx.coroutines.launch import mozilla.appservices.places.BookmarkRoot import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager +import mozilla.components.browser.state.selector.findTab +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.EngineSession.LoadUrlFlags import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.feature.session.SessionFeature @@ -66,7 +68,8 @@ class DefaultBrowserToolbarMenuController( private val bookmarkTapped: (Session) -> Unit, private val scope: CoroutineScope, private val tabCollectionStorage: TabCollectionStorage, - private val topSitesStorage: DefaultTopSitesStorage + private val topSitesStorage: DefaultTopSitesStorage, + private val browserStore: BrowserStore ) : BrowserToolbarMenuController { private val currentSession @@ -184,7 +187,7 @@ class DefaultBrowserToolbarMenuController( val directions = NavGraphDirections.actionGlobalShareFragment( data = arrayOf( ShareData( - url = currentSession?.url, + url = getProperUrl(currentSession), title = currentSession?.title ) ), @@ -295,6 +298,17 @@ class DefaultBrowserToolbarMenuController( } } + private fun getProperUrl(currentSession: Session?): String? { + return currentSession?.id?.let { + val currentTab = browserStore.state.findTab(it) + if (currentTab?.readerState?.active == true) { + currentTab.readerState.activeUrl + } else { + currentSession.url + } + } + } + @Suppress("ComplexMethod") private fun trackToolbarItemInteraction(item: ToolbarMenu.Item) { val eventItem = when (item) { diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt index 89e1a295ab..4ed3263176 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt @@ -26,6 +26,10 @@ import kotlinx.coroutines.test.runBlockingTest import mozilla.appservices.places.BookmarkRoot import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.ReaderState +import mozilla.components.browser.state.state.createTab +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.prompt.ShareData import mozilla.components.feature.search.SearchUseCases @@ -85,6 +89,7 @@ class DefaultBrowserToolbarMenuControllerTest { @MockK private lateinit var sessionFeatureWrapper: ViewBoundFeatureWrapper @RelaxedMockK private lateinit var sessionFeature: SessionFeature @RelaxedMockK private lateinit var topSitesStorage: DefaultTopSitesStorage + @RelaxedMockK private lateinit var browserStore: BrowserStore @Before fun setUp() { @@ -334,11 +339,19 @@ class DefaultBrowserToolbarMenuControllerTest { } @Test - fun handleToolbarSharePress() = runBlockingTest { + fun handleToolbarSharePressWithReaderModeInactive() = runBlockingTest { val item = ToolbarMenu.Item.Share - + 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" - every { currentSession.title } returns "Mozilla" val controller = createController(scope = this) controller.handleToolbarItemInteraction(item) @@ -346,10 +359,43 @@ class DefaultBrowserToolbarMenuControllerTest { verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.SHARE)) } verify { navController.navigate( - directionsEq(NavGraphDirections.actionGlobalShareFragment( - data = arrayOf(ShareData(url = "https://mozilla.org", title = "Mozilla")), - showPage = true - )) + directionsEq( + NavGraphDirections.actionGlobalShareFragment( + data = arrayOf(ShareData(url = "https://mozilla.org", title = "Mozilla")), + showPage = true + ) + ) + ) + } + } + + @Test + fun handleToolbarSharePressWithReaderModeActive() = runBlockingTest { + val item = ToolbarMenu.Item.Share + 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.SHARE)) } + verify { + navController.navigate( + directionsEq( + NavGraphDirections.actionGlobalShareFragment( + data = arrayOf(ShareData(url = "https://mozilla.org", title = "Mozilla")), + showPage = true + ) + ) ) } } @@ -491,7 +537,8 @@ class DefaultBrowserToolbarMenuControllerTest { readerModeController = readerModeController, sessionManager = sessionManager, sessionFeature = sessionFeatureWrapper, - topSitesStorage = topSitesStorage + topSitesStorage = topSitesStorage, + browserStore = browserStore ).apply { ioScope = scope }