diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt index c9df30ee2..a5b4b7de7 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt @@ -232,7 +232,10 @@ class DefaultSessionControlController( } override fun handleCollectionShareTabsClicked(collection: TabCollection) { - showShareFragment(collection.tabs.map { ShareData(url = it.url, title = it.title) }) + showShareFragment( + collection.title, + collection.tabs.map { ShareData(url = it.url, title = it.title) } + ) metrics.track(Event.CollectionShared) } @@ -366,8 +369,9 @@ class DefaultSessionControlController( showTabTrayCollectionCreation() } - private fun showShareFragment(data: List) { + private fun showShareFragment(shareSubject: String, data: List) { val directions = HomeFragmentDirections.actionGlobalShareFragment( + shareSubject = shareSubject, data = data.toTypedArray() ) navController.nav(R.id.homeFragment, directions) diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareController.kt b/app/src/main/java/org/mozilla/fenix/share/ShareController.kt index 0f768cd9e..2c1f44888 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareController.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareController.kt @@ -55,6 +55,7 @@ interface ShareController { * Default behavior of [ShareController]. Other implementations are possible. * * @param context [Context] used for various Android interactions. + * @param shareSubject desired message subject used when sharing through 3rd party apps, like email clients. * @param shareData the list of [ShareData]s that can be shared. * @param sendTabUseCases instance of [SendTabUseCases] which allows sending tabs to account devices. * @param snackbar - instance of [FenixSnackbar] for displaying styled snackbars @@ -64,6 +65,7 @@ interface ShareController { @Suppress("TooManyFunctions") class DefaultShareController( private val context: Context, + private val shareSubject: String?, private val shareData: List, private val sendTabUseCases: SendTabUseCases, private val snackbar: FenixSnackbar, @@ -90,7 +92,7 @@ class DefaultShareController( val intent = Intent(ACTION_SEND).apply { putExtra(EXTRA_TEXT, getShareText()) - putExtra(EXTRA_SUBJECT, shareData.map { it.title }.joinToString(", ")) + putExtra(EXTRA_SUBJECT, getShareSubject()) type = "text/plain" flags = FLAG_ACTIVITY_NEW_TASK setClassName(app.packageName, app.activityName) @@ -189,6 +191,9 @@ class DefaultShareController( } } + @VisibleForTesting + internal fun getShareSubject() = shareSubject ?: shareData.map { it.title }.joinToString(", ") + // Navigation between app fragments uses ShareTab as arguments. SendTabUseCases uses TabData. @VisibleForTesting internal fun List.toTabData() = map { data -> diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt b/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt index 005a8b8fe..cd190d564 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareFragment.kt @@ -67,6 +67,7 @@ class ShareFragment : AppCompatDialogFragment() { shareInteractor = ShareInteractor( DefaultShareController( context = requireContext(), + shareSubject = args.shareSubject, shareData = shareData, snackbar = FenixSnackbar.make( view = requireActivity().getRootView()!!, diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml index c8bdb42d3..a142bf363 100644 --- a/app/src/main/res/navigation/nav_graph.xml +++ b/app/src/main/res/navigation/nav_graph.xml @@ -677,6 +677,11 @@ android:defaultValue="null" app:argType="string" app:nullable="true" /> + { every { tabs } returns emptyList() + every { title } returns "" } controller.handleCollectionShareTabsClicked(collection) diff --git a/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt b/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt index bdea82de0..ace841db6 100644 --- a/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt @@ -48,6 +48,7 @@ class ShareControllerTest { // Need a valid context to retrieve Strings for example, but we also need it to return our "metrics" private val context: Context = spyk(testContext) private val metrics: MetricController = mockk(relaxed = true) + private val shareSubject = "shareSubject" private val shareData = listOf( ShareData(url = "url0", title = "title0"), ShareData(url = "url1", title = "title1") @@ -65,7 +66,7 @@ class ShareControllerTest { private val dismiss = mockk<(ShareController.Result) -> Unit>(relaxed = true) private val recentAppStorage = mockk(relaxed = true) private val controller = DefaultShareController( - context, shareData, sendTabUseCases, snackbar, navController, + context, shareSubject, shareData, sendTabUseCases, snackbar, navController, recentAppStorage, testCoroutineScope, dismiss ) @@ -91,8 +92,8 @@ class ShareControllerTest { // needed for capturing the actual Intent used the `slot` one doesn't have this flag so we // need to use an Activity Context. val activityContext: Context = mockk() - val testController = DefaultShareController(activityContext, shareData, mockk(), mockk(), mockk(), - recentAppStorage, testCoroutineScope, dismiss) + val testController = DefaultShareController(activityContext, shareSubject, shareData, mockk(), + mockk(), mockk(), recentAppStorage, testCoroutineScope, dismiss) every { activityContext.startActivity(capture(shareIntent)) } just Runs every { recentAppStorage.updateRecentApp(appShareOption.activityName) } just Runs @@ -101,6 +102,7 @@ class ShareControllerTest { // Check that the Intent used for querying apps has the expected structure assertTrue(shareIntent.isCaptured) assertEquals(Intent.ACTION_SEND, shareIntent.captured.action) + assertEquals(shareSubject, shareIntent.captured.extras!![Intent.EXTRA_SUBJECT]) assertEquals(textToShare, shareIntent.captured.extras!![Intent.EXTRA_TEXT]) assertEquals("text/plain", shareIntent.captured.type) assertEquals(Intent.FLAG_ACTIVITY_NEW_TASK, shareIntent.captured.flags) @@ -124,8 +126,8 @@ class ShareControllerTest { // needed for capturing the actual Intent used the `slot` one doesn't have this flag so we // need to use an Activity Context. val activityContext: Context = mockk() - val testController = DefaultShareController(activityContext, shareData, mockk(), snackbar, - mockk(), mockk(), testCoroutineScope, dismiss) + val testController = DefaultShareController(activityContext, shareSubject, shareData, mockk(), + snackbar, mockk(), mockk(), testCoroutineScope, dismiss) every { activityContext.startActivity(capture(shareIntent)) } throws SecurityException() every { activityContext.getString(R.string.share_error_snackbar) } returns "Cannot share to this app" @@ -247,6 +249,7 @@ class ShareControllerTest { fun `getSuccessMessage should return different strings depending on the number of shared tabs`() { val controllerWithOneSharedTab = DefaultShareController( context, + shareSubject, listOf(ShareData(url = "url0", title = "title0")), mockk(), mockk(), @@ -280,7 +283,7 @@ class ShareControllerTest { ShareData(url = "url1") ) val controller = DefaultShareController( - context, shareData, sendTabUseCases, snackbar, navController, + context, shareSubject, shareData, sendTabUseCases, snackbar, navController, recentAppStorage, testCoroutineScope, dismiss ) @@ -288,6 +291,21 @@ class ShareControllerTest { assertEquals(expectedShareText, controller.getShareText()) } + @Test + fun `getShareSubject will return "shareSubject" if that is non null`() { + assertEquals(shareSubject, controller.getShareSubject()) + } + + @Test + fun `getShareSubject will return a concatenation of tab titles if "shareSubject" is null`() { + val controller = DefaultShareController( + context, null, shareData, sendTabUseCases, snackbar, navController, + recentAppStorage, testCoroutineScope, dismiss + ) + + assertEquals("title0, title1", controller.getShareSubject()) + } + @Test fun `ShareTab#toTabData maps a list of ShareTab to a TabData list`() { var tabData: List