[go_router] Fixes an issue where android back button pops wrong page. (#7348)

fixes flutter/flutter#141906

Author: chunhtai

packages/go_router/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 14.2.5
+
+- Fixes an issue where android back button pops pages in the wrong order.
+
 ## 14.2.4
 
 - Updates minimum supported SDK version to Flutter 3.19/Dart 3.3.

packages/go_router/lib/src/delegate.dart

@@ -52,21 +52,7 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
 
   @override
   Future<bool> popRoute() async {
-    NavigatorState? state = navigatorKey.currentState;
-    if (state == null) {
-      return false;
-    }
-    if (!state.canPop()) {
-      state = null;
-    }
-    RouteMatchBase walker = currentConfiguration.matches.last;
-    while (walker is ShellRouteMatch) {
-      if (walker.navigatorKey.currentState?.canPop() ?? false) {
-        state = walker.navigatorKey.currentState;
-      }
-      walker = walker.matches.last;
-    }
-    assert(walker is RouteMatch);
+    final NavigatorState? state = _findCurrentNavigator();
     if (state != null) {
       return state.maybePop();
     }
@@ -75,7 +61,8 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
     if (lastRoute.onExit != null && navigatorKey.currentContext != null) {
       return !(await lastRoute.onExit!(
         navigatorKey.currentContext!,
-        walker.buildState(_configuration, currentConfiguration),
+        currentConfiguration.last
+            .buildState(_configuration, currentConfiguration),
       ));
     }
     return false;
@@ -98,21 +85,33 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList>
 
   /// Pops the top-most route.
   void pop<T extends Object?>([T? result]) {
+    final NavigatorState? state = _findCurrentNavigator();
+    if (state == null) {
+      throw GoError('There is nothing to pop');
+    }
+    state.pop(result);
+  }
+
+  NavigatorState? _findCurrentNavigator() {
     NavigatorState? state;
     if (navigatorKey.currentState?.canPop() ?? false) {
       state = navigatorKey.currentState;
     }
     RouteMatchBase walker = currentConfiguration.matches.last;
     while (walker is ShellRouteMatch) {
-      if (walker.navigatorKey.currentState?.canPop() ?? false) {
+      final NavigatorState potentialCandidate =
+          walker.navigatorKey.currentState!;
+      if (!ModalRoute.of(potentialCandidate.context)!.isCurrent) {
+        // There is a pageless route on top of the shell route. it needs to be
+        // popped first.
+        break;
+      }
+      if (potentialCandidate.canPop()) {
         state = walker.navigatorKey.currentState;
       }
       walker = walker.matches.last;
     }
-    if (state == null) {
-      throw GoError('There is nothing to pop');
-    }
-    state.pop(result);
+    return state;
   }
 
   void _debugAssertMatchListNotEmpty() {

packages/go_router/lib/src/router.dart

@@ -199,7 +199,7 @@ class GoRouter implements RouterConfig<RouteMatchList> {
     setLogging(enabled: debugLogDiagnostics);
     WidgetsFlutterBinding.ensureInitialized();
 
-    navigatorKey ??= GlobalKey<NavigatorState>();
+    navigatorKey ??= GlobalKey<NavigatorState>(debugLabel: 'root');
 
     _routingConfig.addListener(_handleRoutingConfigChanged);
     configuration = RouteConfiguration(

packages/go_router/pubspec.yaml

@@ -1,7 +1,7 @@
 name: go_router
 description: A declarative router for Flutter based on Navigation 2 supporting
   deep linking, data-driven routes and more
-version: 14.2.4
+version: 14.2.5
 repository: https://github.com/flutter/packages/tree/main/packages/go_router
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22
 

packages/go_router/test/go_router_test.dart

@@ -398,6 +398,67 @@ void main() {
       expect(find.byKey(settings), findsOneWidget);
     });
 
+    testWidgets('android back button pop in correct order',
+        (WidgetTester tester) async {
+      // Regression test for https://github.com/flutter/flutter/issues/141906.
+      final List<RouteBase> routes = <RouteBase>[
+        GoRoute(
+            path: '/',
+            builder: (_, __) => const Text('home'),
+            routes: <RouteBase>[
+              ShellRoute(
+                builder: (
+                  BuildContext context,
+                  GoRouterState state,
+                  Widget child,
+                ) {
+                  return Column(
+                    children: <Widget>[
+                      const Text('shell'),
+                      child,
+                    ],
+                  );
+                },
+                routes: <GoRoute>[
+                  GoRoute(
+                    path: 'page',
+                    builder: (BuildContext context, __) {
+                      return TextButton(
+                        onPressed: () {
+                          Navigator.of(context, rootNavigator: true).push(
+                            MaterialPageRoute<void>(
+                                builder: (BuildContext context) {
+                              return const Text('pageless');
+                            }),
+                          );
+                        },
+                        child: const Text('page'),
+                      );
+                    },
+                  ),
+                ],
+              ),
+            ]),
+      ];
+      final GoRouter router =
+          await createRouter(routes, tester, initialLocation: '/page');
+      expect(find.text('shell'), findsOneWidget);
+      expect(find.text('page'), findsOneWidget);
+
+      await tester.tap(find.text('page'));
+      await tester.pumpAndSettle();
+      expect(find.text('shell'), findsNothing);
+      expect(find.text('page'), findsNothing);
+      expect(find.text('pageless'), findsOneWidget);
+
+      final bool result = await router.routerDelegate.popRoute();
+      expect(result, isTrue);
+      await tester.pumpAndSettle();
+      expect(find.text('shell'), findsOneWidget);
+      expect(find.text('page'), findsOneWidget);
+      expect(find.text('pageless'), findsNothing);
+    });
+
     testWidgets('can correctly pop stacks of repeated pages',
         (WidgetTester tester) async {
       // Regression test for https://github.com/flutter/flutter/issues/#132229.