For #17771: three-dot menu reorder (#17838)

* Feature flag for toolbar menu redesign. Add new items to menu and reorder.

* Handle toolbar items in menu controller

* Menu controller tests

* Make icons invisible

* Lint

* UI tests reflect design change

* Refactor test names

* Lint fixes

* UI tests

app/src/androidTest/java/org/mozilla/fenix/ui/ReaderViewTest.kt

@@ -12,6 +12,8 @@ import org.junit.Rule
 import org.junit.Test
 import org.mozilla.fenix.ui.robots.navigationToolbar
 import androidx.test.espresso.IdlingRegistry
+import org.junit.Ignore
+import org.mozilla.fenix.FeatureFlags
 import org.mozilla.fenix.R
 import org.mozilla.fenix.helpers.AndroidAssetDispatcher
 import org.mozilla.fenix.helpers.ViewVisibilityIdlingResource
@@ -29,7 +31,7 @@ import org.mozilla.fenix.ui.robots.mDevice
  *
  */
 
-// @Ignore("Temp disable - reader view page detection issues: https://github.com/mozilla-mobile/fenix/issues/9688 ")
+@Ignore("Temp disable - reader view page detection issues: https://github.com/mozilla-mobile/fenix/issues/9688 ")
 class ReaderViewTest {
     private lateinit var mockWebServer: MockWebServer
     private var readerViewNotification: ViewVisibilityIdlingResource? = null
@@ -103,39 +105,42 @@ class ReaderViewTest {
 
     @Test
     fun verifyReaderViewToggle() {
-        val readerViewPage =
-            TestAssetHelper.getLoremIpsumAsset(mockWebServer)
-
-        navigationToolbar {
-        }.enterURLAndEnterToBrowser(readerViewPage.url) {
-            mDevice.waitForIdle()
+        // New three-dot menu design does not have readerview
+        if (!FeatureFlags.toolbarMenuFeature) {
+            val readerViewPage =
+                TestAssetHelper.getLoremIpsumAsset(mockWebServer)
+
+            navigationToolbar {
+            }.enterURLAndEnterToBrowser(readerViewPage.url) {
+                mDevice.waitForIdle()
+            }
+
+            readerViewNotification = ViewVisibilityIdlingResource(
+                activityIntentTestRule.activity.findViewById(R.id.mozac_browser_toolbar_page_actions),
+                View.VISIBLE
+            )
+
+            IdlingRegistry.getInstance().register(readerViewNotification)
+
+            navigationToolbar {
+                verifyReaderViewDetected(true)
+                toggleReaderView()
+                mDevice.waitForIdle()
+            }
+
+            browserScreen {
+                verifyPageContent(estimatedReadingTime)
+            }.openThreeDotMenu {
+                verifyReaderViewAppearance(true)
+            }.closeBrowserMenuToBrowser { }
+
+            navigationToolbar {
+                toggleReaderView()
+                mDevice.waitForIdle()
+            }.openThreeDotMenu {
+                verifyReaderViewAppearance(false)
+            }.close { }
         }
-
-        readerViewNotification = ViewVisibilityIdlingResource(
-            activityIntentTestRule.activity.findViewById(R.id.mozac_browser_toolbar_page_actions),
-            View.VISIBLE
-        )
-
-        IdlingRegistry.getInstance().register(readerViewNotification)
-
-        navigationToolbar {
-            verifyReaderViewDetected(true)
-            toggleReaderView()
-            mDevice.waitForIdle()
-        }
-
-        browserScreen {
-            verifyPageContent(estimatedReadingTime)
-        }.openThreeDotMenu {
-            verifyReaderViewAppearance(true)
-        }.closeBrowserMenuToBrowser { }
-
-        navigationToolbar {
-            toggleReaderView()
-            mDevice.waitForIdle()
-        }.openThreeDotMenu {
-            verifyReaderViewAppearance(false)
-        }.close { }
     }
 
     @Test

app/src/androidTest/java/org/mozilla/fenix/ui/SettingsAddonsTest.kt

@@ -79,19 +79,23 @@ class SettingsAddonsTest {
         val defaultWebPage = TestAssetHelper.getGenericAsset(mockWebServer, 1)
         val addonName = "uBlock Origin"
 
-        navigationToolbar {
-        }.openNewTabAndEnterToBrowser(defaultWebPage.url) {
-        }.openThreeDotMenu {
-        }.openAddonsManagerMenu {
-            addonsListIdlingResource =
-                RecyclerViewIdlingResource(activityTestRule.activity.findViewById(R.id.add_ons_list), 1)
-            IdlingRegistry.getInstance().register(addonsListIdlingResource!!)
-            clickInstallAddon(addonName)
-            verifyAddonPrompt(addonName)
-            cancelInstallAddon()
-            clickInstallAddon(addonName)
-            acceptInstallAddon()
-            verifyDownloadAddonPrompt(addonName, activityTestRule)
+        navigationToolbar {}
+            .openNewTabAndEnterToBrowser(defaultWebPage.url) {}
+            .openThreeDotMenu {}
+            .openAddonsManagerMenu {
+                addonsListIdlingResource =
+                    RecyclerViewIdlingResource(
+                        activityTestRule.activity.findViewById(R.id.add_ons_list),
+                        1
+                    )
+                IdlingRegistry.getInstance().register(addonsListIdlingResource!!)
+                clickInstallAddon(addonName)
+                verifyAddonPrompt(addonName)
+                cancelInstallAddon()
+                clickInstallAddon(addonName)
+                acceptInstallAddon()
+
+                verifyDownloadAddonPrompt(addonName, activityTestRule)
         }
     }
 

app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt

@@ -14,6 +14,7 @@ import androidx.test.uiautomator.UiDevice
 import okhttp3.mockwebserver.MockWebServer
 import org.junit.After
 import org.junit.Before
+import org.junit.Ignore
 import org.junit.Rule
 import org.junit.Test
 import org.mozilla.fenix.R
@@ -198,6 +199,7 @@ class SmokeTest {
 
     @Test
     // Verifies the list of items in a tab's 3 dot menu
+    @Ignore("To be re-implemented with the three dot menu changes https://github.com/mozilla-mobile/fenix/issues/17870")
     fun verifyPageMainMenuItemsTest() {
         val defaultWebPage = TestAssetHelper.getGenericAsset(mockWebServer, 1)
 
@@ -231,6 +233,7 @@ class SmokeTest {
     }
 
     @Test
+    @Ignore("To be re-implemented in https://github.com/mozilla-mobile/fenix/issues/17798")
     // Verifies the Synced tabs menu opens from a tab's 3 dot menu
     fun openMainMenuSyncedTabsItemTest() {
         homeScreen {
@@ -362,6 +365,7 @@ class SmokeTest {
 
     @Test
     // Turns ETP toggle off from Settings and verifies the ETP shield is not displayed in the nav bar
+    @Ignore("To be re-implemented with the three dot menu changes https://github.com/mozilla-mobile/fenix/issues/17870")
     fun verifyETPShieldNotDisplayedIfOFFGlobally() {
         val defaultWebPage = TestAssetHelper.getGenericAsset(mockWebServer, 1)
 
@@ -537,6 +541,7 @@ class SmokeTest {
 
     @Test
     // Saves a login, then changes it and verifies the update
+    @Ignore("To be re-implemented with the three dot menu changes https://github.com/mozilla-mobile/fenix/issues/17870")
     fun updateSavedLoginTest() {
         val saveLoginTest =
             TestAssetHelper.getSaveLoginAsset(mockWebServer)
@@ -600,6 +605,7 @@ class SmokeTest {
     }
 
     @Test
+    @Ignore("To be re-implemented in https://github.com/mozilla-mobile/fenix/issues/17799")
     // Installs uBlock add-on and checks that the app doesn't crash while loading pages with trackers
     fun noCrashWithAddonInstalledTest() {
         // setting ETP to Strict mode to test it works with add-ons
@@ -1107,6 +1113,7 @@ class SmokeTest {
     }
 
     @Test
+    @Ignore("To be re-implemented in https://github.com/mozilla-mobile/fenix/issues/17799")
     fun mainMenuInstallPWATest() {
         val pwaPage = "https://rpappalax.github.io/testapp/"
 
@@ -1123,6 +1130,7 @@ class SmokeTest {
     }
 
     @Test
+    @Ignore("To be re-implemented in https://github.com/mozilla-mobile/fenix/issues/17971")
     // Verifies that reader mode is detected and the custom appearance controls are displayed
     fun verifyReaderViewAppearanceUI() {
         val readerViewPage =

app/src/androidTest/java/org/mozilla/fenix/ui/StrictEnhancedTrackingProtectionTest.kt

@@ -8,6 +8,7 @@ import androidx.test.platform.app.InstrumentationRegistry
 import okhttp3.mockwebserver.MockWebServer
 import org.junit.After
 import org.junit.Before
+import org.junit.Ignore
 import org.junit.Rule
 import org.junit.Test
 import org.mozilla.fenix.ext.settings
@@ -126,6 +127,7 @@ class StrictEnhancedTrackingProtectionTest {
     }
 
     @Test
+    @Ignore("To be re-implemented with the three dot menu changes https://github.com/mozilla-mobile/fenix/issues/17870")
     fun testStrictVisitDisable() {
         val trackingProtectionTest =
             TestAssetHelper.getEnhancedTrackingProtectionAsset(mockWebServer)

app/src/androidTest/java/org/mozilla/fenix/ui/ThreeDotMenuMainTest.kt

@@ -12,6 +12,7 @@ import org.junit.Before
 import org.junit.BeforeClass
 import org.junit.Rule
 import org.junit.Test
+import org.mozilla.fenix.FeatureFlags
 import org.mozilla.fenix.helpers.AndroidAssetDispatcher
 import org.mozilla.fenix.helpers.HomeActivityTestRule
 import org.mozilla.fenix.ui.robots.homeScreen
@@ -54,42 +55,68 @@ class ThreeDotMenuMainTest {
 
     @Test
     fun threeDotMenuItemsTest() {
-        homeScreen {
-        }.openThreeDotMenu {
-            verifySettingsButton()
-            verifyBookmarksButton()
-            verifyHistoryButton()
-            verifyHelpButton()
-            verifyWhatsNewButton()
-        }.openSettings {
-            verifySettingsView()
-        }.goBack {
-        }.openThreeDotMenu {
-        }.openHelp {
-            verifyHelpUrl()
-        }.openTabDrawer {
-            closeTab()
-        }
+        if (FeatureFlags.toolbarMenuFeature) {
+            homeScreen {
+            }.openThreeDotMenu {
+            }.openHistory {
+                verifyHistoryMenuView()
+            }.goBackToBrowser {}
 
-        homeScreen {
-        }.openThreeDotMenu {
-        }.openWhatsNew {
-            verifyWhatsNewURL()
-        }.openTabDrawer {
-            closeTab()
-        }
+            homeScreen {
+            }.openThreeDotMenu {
+            }.openBookmarks {
+                verifyBookmarksMenuView()
+            }.closeMenu {}
 
-        homeScreen {
-        }.openThreeDotMenu {
-        }.openBookmarks {
-            verifyBookmarksMenuView()
-        }.closeMenu {
-        }
+            homeScreen {
+            }.openThreeDotMenu {
+                verifySettingsButton()
+                verifyBookmarksButton()
+                verifyHistoryButton()
+            }.openSettings {
+                verifySettingsView()
+            }.goBack {
+            }.openThreeDotMenu {
+            }.goBack {}
+        } else {
+            homeScreen {
+            }.openThreeDotMenu {
+                verifySettingsButton()
+                verifyBookmarksButton()
+                verifyHistoryButton()
+                verifyHelpButton()
+                verifyWhatsNewButton()
+            }.openSettings {
+                verifySettingsView()
+            }.goBack {
+            }.openThreeDotMenu {
+            }.openHelp {
+                verifyHelpUrl()
+            }.openTabDrawer {
+                closeTab()
+            }
 
-        homeScreen {
-        }.openThreeDotMenu {
-        }.openHistory {
-            verifyHistoryMenuView()
+            homeScreen {
+            }.openThreeDotMenu {
+            }.openWhatsNew {
+                verifyWhatsNewURL()
+            }.openTabDrawer {
+                closeTab()
+            }
+
+            homeScreen {
+            }.openThreeDotMenu {
+            }.openBookmarks {
+                verifyBookmarksMenuView()
+            }.closeMenu {
+            }
+
+            homeScreen {
+            }.openThreeDotMenu {
+            }.openHistory {
+                verifyHistoryMenuView()
+            }
         }
+
     }
 }

app/src/androidTest/java/org/mozilla/fenix/ui/robots/ThreeDotMenuMainRobot.kt

@@ -38,6 +38,7 @@ import androidx.test.uiautomator.Until
 import org.hamcrest.Matcher
 import org.hamcrest.Matchers.allOf
 import org.junit.Assert.assertTrue
+import org.mozilla.fenix.FeatureFlags
 import org.mozilla.fenix.R
 import org.mozilla.fenix.helpers.TestAssetHelper.waitingTime
 import org.mozilla.fenix.helpers.click
@@ -124,21 +125,33 @@ class ThreeDotMenuMainRobot {
     fun verifyShareTabsOverlay() = assertShareTabsOverlay()
 
     fun verifyThreeDotMainMenuItems() {
-        verifyAddOnsButton()
-        verifyDownloadsButton()
-        verifyHistoryButton()
-        verifyBookmarksButton()
-        verifySyncedTabsButton()
-        verifySettingsButton()
-        verifyFindInPageButton()
-        verifyAddFirefoxHome()
-        verifyAddToMobileHome()
-        verifyDesktopSite()
-        verifySaveCollection()
-        verifyAddBookmarkButton()
-        verifyShareButton()
-        verifyForwardButton()
-        verifyRefreshButton()
+        if (FeatureFlags.toolbarMenuFeature) {
+            verifyDownloadsButton()
+            verifyHistoryButton()
+            verifyBookmarksButton()
+            verifySettingsButton()
+            verifyDesktopSite()
+            verifySaveCollection()
+            verifyShareButton()
+            verifyForwardButton()
+            verifyRefreshButton()
+        } else {
+            verifyAddOnsButton()
+            verifyDownloadsButton()
+            verifyHistoryButton()
+            verifyBookmarksButton()
+            verifySyncedTabsButton()
+            verifySettingsButton()
+            verifyFindInPageButton()
+            verifyAddFirefoxHome()
+            verifyAddToMobileHome()
+            verifyDesktopSite()
+            verifySaveCollection()
+            verifyAddBookmarkButton()
+            verifyShareButton()
+            verifyForwardButton()
+            verifyRefreshButton()
+        }
     }
 
     private fun assertShareTabsOverlay() {
@@ -390,7 +403,8 @@ private fun assertSettingsButton() = settingsButton()
     .check(matches(withEffectiveVisibility(Visibility.VISIBLE)))
     .check(matches(isCompletelyDisplayed()))
 
-private fun addOnsButton() = onView(allOf(withText("Add-ons")))
+private val addOnsText = if (FeatureFlags.toolbarMenuFeature) "Extensions" else "Add-ons"
+private fun addOnsButton() = onView(allOf(withText(addOnsText)))
 private fun assertAddOnsButton() {
     onView(withId(R.id.mozac_browser_menu_menuView)).perform(swipeDown())
     addOnsButton().check(matches(withEffectiveVisibility(Visibility.VISIBLE)))

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

@@ -30,4 +30,9 @@ object FeatureFlags {
      * Enables WebAuthn support.
      */
     val webAuthFeature = Config.channel.isNightlyOrDebug
+
+    /**
+     * Shows new three-dot toolbar menu design.
+     */
+    val toolbarMenuFeature = Config.channel.isDebug
 }

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

@@ -51,7 +51,7 @@ interface BrowserToolbarMenuController {
     fun handleToolbarItemInteraction(item: ToolbarMenu.Item)
 }
 
-@Suppress("LargeClass")
+@Suppress("LargeClass", "ForbiddenComment")
 class DefaultBrowserToolbarMenuController(
     private val activity: HomeActivity,
     private val navController: NavController,
@@ -87,6 +87,76 @@ class DefaultBrowserToolbarMenuController(
         trackToolbarItemInteraction(item)
 
         Do exhaustive when (item) {
+            // TODO: These can be removed for https://github.com/mozilla-mobile/fenix/issues/17870
+            // todo === Start ===
+            is ToolbarMenu.Item.InstallToHomeScreen -> {
+                settings.installPwaOpened = true
+                MainScope().launch {
+                    with(activity.components.useCases.webAppUseCases) {
+                        if (isInstallable()) {
+                            addToHomescreen()
+                        } else {
+                            val directions =
+                                BrowserFragmentDirections.actionBrowserFragmentToCreateShortcutFragment()
+                            navController.navigateSafe(R.id.browserFragment, directions)
+                        }
+                    }
+                }
+            }
+            is ToolbarMenu.Item.OpenInFenix -> {
+                // Stop the SessionFeature from updating the EngineView and let it release the session
+                // from the EngineView so that it can immediately be rendered by a different view once
+                // we switch to the actual browser.
+                sessionFeature.get()?.release()
+
+                // Strip the CustomTabConfig to turn this Session into a regular tab and then select it
+                customTabSession!!.customTabConfig = null
+                sessionManager.select(customTabSession)
+
+                // Switch to the actual browser which should now display our new selected session
+                activity.startActivity(openInFenixIntent.apply {
+                    // We never want to launch the browser in the same task as the external app
+                    // activity. So we force a new task here. IntentReceiverActivity will do the
+                    // right thing and take care of routing to an already existing browser and avoid
+                    // cloning a new one.
+                    flags = flags or Intent.FLAG_ACTIVITY_NEW_TASK
+                })
+
+                // Close this activity (and the task) since it is no longer displaying any session
+                activity.finishAndRemoveTask()
+            }
+            is ToolbarMenu.Item.Quit -> {
+                // We need to show the snackbar while the browsing data is deleting (if "Delete
+                // browsing data on quit" is activated). After the deletion is over, the snackbar
+                // is dismissed.
+                val snackbar: FenixSnackbar? = activity.getRootView()?.let { v ->
+                    FenixSnackbar.make(
+                        view = v,
+                        duration = Snackbar.LENGTH_LONG,
+                        isDisplayedWithBrowserToolbar = true
+                    )
+                        .setText(v.context.getString(R.string.deleting_browsing_data_in_progress))
+                }
+
+                deleteAndQuit(activity, scope, snackbar)
+            }
+            is ToolbarMenu.Item.ReaderModeAppearance -> {
+                readerModeController.showControls()
+                metrics.track(Event.ReaderModeAppearanceOpened)
+            }
+            is ToolbarMenu.Item.OpenInApp -> {
+                settings.openInAppOpened = true
+
+                val appLinksUseCases = activity.components.useCases.appLinksUseCases
+                val getRedirect = appLinksUseCases.appLinkRedirect
+                currentSession?.let {
+                    val redirect = getRedirect.invoke(it.url)
+                    redirect.appIntent?.flags = Intent.FLAG_ACTIVITY_NEW_TASK
+                    appLinksUseCases.openAppLink.invoke(redirect.appIntent)
+                }
+            }
+            // todo === End ===
+
             is ToolbarMenu.Item.Back -> {
                 if (item.viewHistory) {
                     navController.navigate(
@@ -118,12 +188,24 @@ class DefaultBrowserToolbarMenuController(
 
                 sessionUseCases.reload.invoke(currentSession, flags = flags)
             }
-            ToolbarMenu.Item.Stop -> sessionUseCases.stopLoading.invoke(currentSession)
-            ToolbarMenu.Item.Settings -> browserAnimator.captureEngineViewAndDrawStatically {
+            is ToolbarMenu.Item.Stop -> sessionUseCases.stopLoading.invoke(currentSession)
+            is ToolbarMenu.Item.Share -> {
+                val directions = NavGraphDirections.actionGlobalShareFragment(
+                    data = arrayOf(
+                        ShareData(
+                            url = getProperUrl(currentSession),
+                            title = currentSession?.title
+                        )
+                    ),
+                    showPage = true
+                )
+                navController.navigate(directions)
+            }
+            is ToolbarMenu.Item.Settings -> browserAnimator.captureEngineViewAndDrawStatically {
                 val directions = BrowserFragmentDirections.actionBrowserFragmentToSettingsFragment()
                 navController.nav(R.id.browserFragment, directions)
             }
-            ToolbarMenu.Item.SyncedTabs -> browserAnimator.captureEngineViewAndDrawStatically {
+            is ToolbarMenu.Item.SyncedTabs -> browserAnimator.captureEngineViewAndDrawStatically {
                 navController.nav(
                     R.id.browserFragment,
                     BrowserFragmentDirections.actionBrowserFragmentToSyncedTabsFragment()
@@ -133,7 +215,7 @@ class DefaultBrowserToolbarMenuController(
                 item.isChecked,
                 currentSession
             )
-            ToolbarMenu.Item.AddToTopSites -> {
+            is ToolbarMenu.Item.AddToTopSites -> {
                 scope.launch {
                     val context = swipeRefresh.context
                     val numPinnedSites =
@@ -169,7 +251,7 @@ class DefaultBrowserToolbarMenuController(
                     }
                 }
             }
-            ToolbarMenu.Item.AddToHomeScreen, ToolbarMenu.Item.InstallToHomeScreen -> {
+            is ToolbarMenu.Item.AddToHomeScreen -> {
                 settings.installPwaOpened = true
                 MainScope().launch {
                     with(activity.components.useCases.webAppUseCases) {
@@ -183,31 +265,17 @@ class DefaultBrowserToolbarMenuController(
                     }
                 }
             }
-            ToolbarMenu.Item.Share -> {
-                val directions = NavGraphDirections.actionGlobalShareFragment(
-                    data = arrayOf(
-                        ShareData(
-                            url = getProperUrl(currentSession),
-                            title = currentSession?.title
-                        )
-                    ),
-                    showPage = true
-                )
-                navController.navigate(directions)
-            }
-
-            ToolbarMenu.Item.FindInPage -> {
+            is ToolbarMenu.Item.FindInPage -> {
                 findInPageLauncher()
                 metrics.track(Event.FindInPageOpened)
             }
-
-            ToolbarMenu.Item.AddonsManager -> browserAnimator.captureEngineViewAndDrawStatically {
+            is ToolbarMenu.Item.AddonsManager -> browserAnimator.captureEngineViewAndDrawStatically {
                 navController.nav(
                     R.id.browserFragment,
                     BrowserFragmentDirections.actionGlobalAddonsManagementFragment()
                 )
             }
-            ToolbarMenu.Item.SaveToCollection -> {
+            is ToolbarMenu.Item.SaveToCollection -> {
                 metrics
                     .track(Event.CollectionSaveButtonPressed(TELEMETRY_BROWSER_IDENTIFIER))
 
@@ -225,82 +293,35 @@ class DefaultBrowserToolbarMenuController(
                     navController.nav(R.id.browserFragment, directions)
                 }
             }
-            ToolbarMenu.Item.OpenInFenix -> {
-                // Stop the SessionFeature from updating the EngineView and let it release the session
-                // from the EngineView so that it can immediately be rendered by a different view once
-                // we switch to the actual browser.
-                sessionFeature.get()?.release()
-
-                // Strip the CustomTabConfig to turn this Session into a regular tab and then select it
-                customTabSession!!.customTabConfig = null
-                sessionManager.select(customTabSession)
-
-                // Switch to the actual browser which should now display our new selected session
-                activity.startActivity(openInFenixIntent.apply {
-                    // We never want to launch the browser in the same task as the external app
-                    // activity. So we force a new task here. IntentReceiverActivity will do the
-                    // right thing and take care of routing to an already existing browser and avoid
-                    // cloning a new one.
-                    flags = flags or Intent.FLAG_ACTIVITY_NEW_TASK
-                })
-
-                // Close this activity (and the task) since it is no longer displaying any session
-                activity.finishAndRemoveTask()
-            }
-            ToolbarMenu.Item.Quit -> {