Route store not trimming backable routes

[eliserichards]

Steps to reproduce

• Log in to the app
• Navigate to an item detail
• Delete the item, confirm delete
• See that the app navigates you back to the item list
• Use the back button: see that it takes you back to the DeleteConfirmationDialog

This is also reproducible when navigating back and forth from the item list to the hamburger menu and back again. The back button shouldn't open the menus.

Expected behavior

The RouteStore subscribes to all of the RouteActions that are dispatched, and trims the actions that are not relevant (dialogs, menus, etc):

.subscribe { routeAction ->
                when (routeAction) {
                    is RouteAction.InternalBack -> _routes.safePop()
                    else -> _routes.onNext(routeAction)
                }
                _routes.trim {
                    when (it) {
                        is DialogAction,
                        is RouteAction.AutoLockSetting,
                        is RouteAction.DialogFragment,
                        is RouteAction.SystemIntent -> true
                        else -> false
                    }
                }

Actual behavior

This .trim { ... } does not seem to be happening.

Device & build information

[Elise]

• Device: Pixel 3
• Build version: master

Notes

Attachments:

[jhugman]

1. The history stack maintained by the RouteStore is not the actual back stack. It is only there to make restoring the route state during a background/foreground cycle. Thus, the trimming of the history is not the source of our woes here.
2. The DialogFragments are shown using a FragmentTransaction in the androidx.fragment.app.FragmentManager. Stepping through the source, nothing seems to be added to the back stack history.
3. In the RoutePresenter a BackPressedCallback, which has not been enabled. It is not clear why this has been disabled. Enabling this won't fix this bug, but should be enabled to keep the histories (the replay stack, and the fragment history stack) synced.
4. The way we have implemented Dialogs is to define Actions to be dispatched in response to positive or negative buttons. Thus when a DeleteConfirmationDialog; probably the correct thing to do here would be to keep popping the history stack until we'd find the ItemList, rather than going straight to ItemList from ItemDetail.

This could be a solution.

However, looking at the log shows an easier path.

E/Lockwise: Cannot route from mozilla.lockbox:id/fragment_item_detail to mozilla.lockbox:id/fragment_item_list. This is a developer bug, fixable by adding an action to mozilla.lockbox:id/graph_main.xml and/or AppRoutePresenter

Given that the ItemList should be at the route of the history stack — both before and after the delete— we could likely avoid the more complicated solution.

Will comment, and add an action in AppRoutePresenter.

[jhugman]

closed in error.