Closes #823 & Closes #835: Refactors openToBrowserAndLoad to include fromFragment (#833)

app/src/main/java/org/mozilla/fenix/HomeActivity.kt

@@ -24,6 +24,9 @@ import mozilla.components.support.ktx.kotlin.isUrl
 import mozilla.components.support.ktx.kotlin.toNormalizedUrl
 import mozilla.components.support.utils.SafeIntent
 import org.mozilla.fenix.ext.components
+import org.mozilla.fenix.home.HomeFragmentDirections
+import org.mozilla.fenix.search.SearchFragmentDirections
+import org.mozilla.fenix.settings.SettingsFragmentDirections
 
 @SuppressWarnings("TooManyFunctions")
 open class HomeActivity : AppCompatActivity() {
@@ -103,18 +106,29 @@ open class HomeActivity : AppCompatActivity() {
 
     private fun handleOpenedFromExternalSource() {
         intent?.putExtra(OPEN_TO_BROWSER, false)
-        openToBrowser(SafeIntent(intent).getStringExtra(IntentProcessor.ACTIVE_SESSION_ID))
+        openToBrowser(SafeIntent(intent).getStringExtra(IntentProcessor.ACTIVE_SESSION_ID), BrowserDirection.FromGlobal)
     }
 
-    // Since we must always call load after navigation, we only directly expose the load when coupled with open.
+    fun openToBrowserAndLoad(text: String, sessionId: String? = null, from: BrowserDirection) {
-    fun openToBrowserAndLoad(text: String, sessionId: String? = null) {
+        openToBrowser(sessionId, from)
-        openToBrowser(sessionId)
         load(text, sessionId)
     }
 
-    fun openToBrowser(sessionId: String?) {
+    fun openToBrowser(sessionId: String?, from: BrowserDirection) {
         val host = supportFragmentManager.findFragmentById(R.id.container) as NavHostFragment
-        val directions = NavGraphDirections.actionGlobalBrowser(sessionId)
+
+        val directions = when (from) {
+            BrowserDirection.FromGlobal -> {
+                NavGraphDirections.actionGlobalBrowser(sessionId).apply {
+                    host.navController.popBackStack()
+                }
+            }
+            BrowserDirection.FromHome -> HomeFragmentDirections.actionHomeFragmentToBrowserFragment(sessionId)
+            BrowserDirection.FromSearch -> SearchFragmentDirections.actionSearchFragmentToBrowserFragment(sessionId)
+            BrowserDirection.FromSettings ->
+                SettingsFragmentDirections.actionSettingsFragmentToBrowserFragment(sessionId)
+        }
+
         host.navController.navigate(directions)
     }
 
@@ -147,3 +161,7 @@ open class HomeActivity : AppCompatActivity() {
         const val OPEN_TO_BROWSER = "open_to_browser"
     }
 }
+
+enum class BrowserDirection {
+    FromGlobal, FromHome, FromSearch, FromSettings
+}

app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt

@@ -28,6 +28,7 @@ import mozilla.components.feature.session.bundling.SessionBundleStorage
 import org.mozilla.fenix.BrowsingModeManager
 import org.mozilla.fenix.DefaultThemeManager
 import org.mozilla.fenix.HomeActivity
+import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.R
 import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.home.sessions.ArchivedSession
@@ -257,7 +258,8 @@ class HomeFragment : Fragment() {
             override fun onClick(widget: View?) {
                 requireComponents.useCases.tabsUseCases.addPrivateTab
                     .invoke(SupportUtils.getSumoURLForTopic(context!!, SupportUtils.SumoTopic.PRIVATE_BROWSING_MYTHS))
-                (activity as HomeActivity).openToBrowser(requireComponents.core.sessionManager.selectedSession?.id)
+                (activity as HomeActivity).openToBrowser(requireComponents.core.sessionManager.selectedSession?.id,
+                    BrowserDirection.FromHome)
             }
         }
         val textWithLink = SpannableString(descriptionText).apply {
@@ -354,7 +356,6 @@ class HomeFragment : Fragment() {
         const val toolbarPaddingDp = 12f
         const val firstKeyTriggerFrame = 55
         const val secondKeyTriggerFrame = 90
-
         const val temporaryNumberOfSessions = 25
     }
 }

app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt

@@ -11,13 +11,10 @@ import android.view.View
 import android.view.ViewGroup
 import androidx.appcompat.app.AppCompatActivity
 import androidx.fragment.app.Fragment
-import androidx.navigation.Navigation
 import kotlinx.android.synthetic.main.fragment_search.view.*
-import mozilla.components.browser.session.Session
 import mozilla.components.feature.search.SearchUseCases
 import mozilla.components.feature.session.SessionUseCases
-import mozilla.components.support.ktx.kotlin.isUrl
+import org.mozilla.fenix.BrowserDirection
-import mozilla.components.support.ktx.kotlin.toNormalizedUrl
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.toolbar.SearchAction
@@ -87,8 +84,8 @@ class SearchFragment : Fragment() {
                 when (it) {
                     is SearchAction.UrlCommitted -> {
                         if (it.url.isNotBlank()) {
-                            transitionToBrowser()
+                            (activity as HomeActivity).openToBrowserAndLoad(it.url, it.session,
-                            load(it.url)
+                                BrowserDirection.FromSearch)
                         }
                     }
                     is SearchAction.TextChanged -> {
@@ -102,11 +99,11 @@ class SearchFragment : Fragment() {
                 when (it) {
                     is AwesomeBarAction.URLTapped -> {
                         getSessionUseCase(requireContext(), sessionId == null).invoke(it.url)
-                        transitionToBrowser()
+                        (activity as HomeActivity).openToBrowser(sessionId, BrowserDirection.FromSearch)
                     }
                     is AwesomeBarAction.SearchTermsTapped -> {
                         getSearchUseCase(requireContext(), sessionId == null).invoke(it.searchTerms)
-                        transitionToBrowser()
+                        (activity as HomeActivity).openToBrowser(sessionId, BrowserDirection.FromSearch)
                     }
                 }
             }
@@ -133,42 +130,4 @@ class SearchFragment : Fragment() {
             false -> context.components.useCases.tabsUseCases.addTab
         }
     }
-
-    // Issue: https://github.com/mozilla-mobile/fenix/issues/626
-    // Currently we were kind of forcing all this logic through the Toolbar Feature.
-    // But since we cannot actually load a page without an available GeckoSession
-    // we have to wait until after we navigate to call the use case.
-    // We should move this logic into a place that makes more sense.
-    private fun load(text: String) {
-        val sessionId = SearchFragmentArgs.fromBundle(arguments!!).sessionId
-        val isPrivate = (activity as HomeActivity).browsingModeManager.isPrivate
-
-        val loadUrlUseCase = if (sessionId == null) {
-            if (isPrivate) {
-                requireComponents.useCases.tabsUseCases.addPrivateTab
-            } else {
-                requireComponents.useCases.tabsUseCases.addTab
-            }
-        } else requireComponents.useCases.sessionUseCases.loadUrl
-
-        val searchUseCase: (String) -> Unit = { searchTerms ->
-            if (sessionId == null) {
-                requireComponents.useCases.searchUseCases.newTabSearch
-                    .invoke(searchTerms, Session.Source.USER_ENTERED, true, isPrivate)
-            } else requireComponents.useCases.searchUseCases.defaultSearch.invoke(searchTerms)
-        }
-
-        if (text.isUrl()) {
-            loadUrlUseCase.invoke(text.toNormalizedUrl())
-        } else {
-            searchUseCase.invoke(text)
-        }
-    }
-
-    private fun transitionToBrowser() {
-        val sessionId = SearchFragmentArgs.fromBundle(arguments!!).sessionId
-        val directions = SearchFragmentDirections.actionSearchFragmentToBrowserFragment(sessionId)
-
-        Navigation.findNavController(view!!.search_layout).navigate(directions)
-    }
 }

app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt

@@ -30,9 +30,11 @@ import mozilla.components.service.fxa.Profile
 import mozilla.components.support.ktx.android.graphics.toDataUri
 import org.mozilla.fenix.BuildConfig
 import org.mozilla.fenix.FenixApplication
+import org.mozilla.fenix.HomeActivity
+import org.mozilla.fenix.BrowserDirection
+import org.mozilla.fenix.R
 import org.mozilla.fenix.ext.getPreferenceKey
 import org.mozilla.fenix.ext.requireComponents
-import org.mozilla.fenix.R
 import org.mozilla.fenix.R.string.pref_key_leakcanary
 import org.mozilla.fenix.R.string.pref_key_feedback
 import org.mozilla.fenix.R.string.pref_key_help
@@ -129,16 +131,13 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse
     private fun getClickListenerForSignIn(): OnPreferenceClickListener {
         return OnPreferenceClickListener {
             requireComponents.services.accountsAuthFeature.beginAuthentication()
-            // TODO the "back button" behaviour here is pretty poor. The sign-in web content populates session history,
+            // TODO The sign-in web content populates session history,
             // so pressing "back" after signing in won't take us back into the settings screen, but rather up the
             // session history stack.
             // We could auto-close this tab once we get to the end of the authentication process?
             // Via an interceptor, perhaps.
             view?.let {
-                Navigation.findNavController(it)
+                (activity as HomeActivity).openToBrowser(null, BrowserDirection.FromHome)
-                    .navigate(
-                        SettingsFragmentDirections.actionGlobalBrowser(null)
-                    )
             }
             true
         }
@@ -225,8 +224,7 @@ class SettingsFragment : PreferenceFragmentCompat(), CoroutineScope, AccountObse
     private fun navigateToSettingsArticle() {
         val newSession = requireComponents.core.sessionManager.selectedSession?.id
         view?.let {
-            Navigation.findNavController(it)
+            (activity as HomeActivity).openToBrowser(newSession, BrowserDirection.FromSettings)
-                .navigate(SettingsFragmentDirections.actionGlobalBrowser(newSession))
         }
     }