From f83d8cfd3a93aec687a162dda69505ff54479342 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Wed, 13 Aug 2025 19:53:05 -0400 Subject: [PATCH] [web] Popping a nameless route should preserve the correct route name (#173652) Fixes https://github.com/flutter/flutter/issues/173356 --- .../lib/src/engine/navigation/history.dart | 41 ++++++++++--------- .../web_ui/lib/src/engine/test_embedding.dart | 4 +- .../lib/web_ui/test/engine/history_test.dart | 37 +++++++++++++++-- 3 files changed, 57 insertions(+), 25 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/navigation/history.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/navigation/history.dart index 99d44c8a2fb..b5c8f353312 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/navigation/history.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/navigation/history.dart @@ -6,7 +6,6 @@ import 'package:meta/meta.dart'; import 'package:ui/ui.dart' as ui; import 'package:ui/ui_web/src/ui_web.dart' as ui_web; -import '../dom.dart'; import '../platform_dispatcher.dart'; import '../services/message_codec.dart'; import '../services/message_codecs.dart'; @@ -257,20 +256,30 @@ class SingleEntryBrowserHistory extends BrowserHistory { _setupStrategy(strategy); - final String path = currentPath; - if (!_isFlutterEntry(domWindow.history.state)) { + _currentRouteName = currentPath; + if (!_isFlutterEntry(currentState)) { // An entry may not have come from Flutter, for example, when the user // refreshes the page. They land directly on the "flutter" entry, so // there's no need to set up the "origin" and "flutter" entries, we can // safely assume they are already set up. _setupOriginEntry(strategy); - _setupFlutterEntry(strategy, path: path); + _setupFlutterEntry(strategy); } } @override final ui_web.UrlStrategy? urlStrategy; + /// The route name of the current page. + /// + /// This is updated whenever the framework calls `setRouteName`. This is then + /// used when the user hits the back button to pop a nameless route, to restore + /// the route name from before the nameless route was pushed. + /// + /// This is also used to track the user-provided url when they change it + /// directly in the address bar. + String _currentRouteName = '/'; + static const MethodCall _popRouteMethodCall = MethodCall('popRoute'); static const String _kFlutterTag = 'flutter'; static const String _kOriginTag = 'origin'; @@ -303,11 +312,11 @@ class SingleEntryBrowserHistory extends BrowserHistory { @override void setRouteName(String? routeName, {Object? state, bool replace = false}) { if (urlStrategy != null) { - _setupFlutterEntry(urlStrategy!, replace: true, path: routeName); + _currentRouteName = routeName ?? currentPath; + _setupFlutterEntry(urlStrategy!, replace: true); } } - String? _userProvidedRouteName; @override void onPopState(Object? state) { if (_isOriginEntry(state)) { @@ -323,17 +332,13 @@ class SingleEntryBrowserHistory extends BrowserHistory { // We get into this scenario when the user changes the url manually. It // causes a new entry to be pushed on top of our "flutter" one. When this // happens it first goes to the "else" section below where we capture the - // path into `_userProvidedRouteName` then trigger a history back which + // path into `_currentRouteName` then trigger a history back which // brings us here. - assert(_userProvidedRouteName != null); - - final String newRouteName = _userProvidedRouteName!; - _userProvidedRouteName = null; // Send a 'pushRoute' platform message so the app handles it accordingly. EnginePlatformDispatcher.instance.invokeOnPlatformMessage( 'flutter/navigation', - const JSONMethodCodec().encodeMethodCall(MethodCall('pushRoute', newRouteName)), + const JSONMethodCodec().encodeMethodCall(MethodCall('pushRoute', _currentRouteName)), (_) {}, ); } else { @@ -342,7 +347,7 @@ class SingleEntryBrowserHistory extends BrowserHistory { // example. // 1. We first capture the user's desired path. - _userProvidedRouteName = currentPath; + _currentRouteName = currentPath; // 2. Then we remove the new entry. // This will take us back to our "flutter" entry and it causes a new @@ -360,13 +365,9 @@ class SingleEntryBrowserHistory extends BrowserHistory { /// This method is used manipulate the Flutter Entry which is always the /// active entry while the Flutter app is running. - void _setupFlutterEntry(ui_web.UrlStrategy strategy, {bool replace = false, String? path}) { - path ??= currentPath; - if (replace) { - strategy.replaceState(_flutterState, 'flutter', path); - } else { - strategy.pushState(_flutterState, 'flutter', path); - } + void _setupFlutterEntry(ui_web.UrlStrategy strategy, {bool replace = false}) { + final updateState = replace ? strategy.replaceState : strategy.pushState; + updateState(_flutterState, 'flutter', _currentRouteName); } @override diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/test_embedding.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/test_embedding.dart index b33cc96cac3..cc71134d720 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/test_embedding.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/test_embedding.dart @@ -15,7 +15,7 @@ const bool _debugLogHistoryActions = false; class TestHistoryEntry { const TestHistoryEntry(this.state, this.title, this.url); - final dynamic state; + final Object? state; final String? title; final String url; @@ -45,7 +45,7 @@ class TestUrlStrategy implements ui_web.UrlStrategy { String getPath() => currentEntry.url; @override - dynamic getState() => currentEntry.state; + Object? getState() => currentEntry.state; int _currentEntryIndex; int get currentEntryIndex => _currentEntryIndex; diff --git a/engine/src/flutter/lib/web_ui/test/engine/history_test.dart b/engine/src/flutter/lib/web_ui/test/engine/history_test.dart index ba412939542..40af749fecc 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/history_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/history_test.dart @@ -203,6 +203,8 @@ void testMain() { expect(spy.messages[0].channel, 'flutter/navigation'); expect(spy.messages[0].methodName, 'popRoute'); expect(spy.messages[0].methodArguments, isNull); + // The framework responds by updating to the most current route name. + await routeUpdated('/home'); // We still have 2 entries. expect(strategy.history, hasLength(2)); expect(strategy.currentEntryIndex, 1); @@ -293,7 +295,7 @@ void testMain() { expect(spy.messages[0].methodName, 'pushRoute'); expect(spy.messages[0].methodArguments, '/page3'); spy.messages.clear(); - // 2. The framework sends a `routePushed` platform message. + // 2. The framework sends a `routeUpdated` platform message. await routeUpdated('/page3'); // 3. The history state should reflect that /page3 is currently active. expect(strategy.history, hasLength(3)); @@ -309,9 +311,9 @@ void testMain() { expect(spy.messages[0].methodName, 'popRoute'); expect(spy.messages[0].methodArguments, isNull); spy.messages.clear(); - // 2. The framework sends a `routePopped` platform message. + // 2. The framework sends a `routeUpdated` platform message. await routeUpdated('/home'); - // 3. The history state should reflect that /page1 is currently active. + // 3. The history state should reflect that /home is currently active. expect(strategy.history, hasLength(2)); expect(strategy.currentEntryIndex, 1); expect(strategy.currentEntry.state, flutterState); @@ -341,6 +343,35 @@ void testMain() { expect(strategy.currentEntry.state, flutterState); expect(strategy.currentEntry.url, '/home'); }); + + test('popping a nameless route does not change url', () async { + final TestUrlStrategy strategy = TestUrlStrategy.fromEntry( + const TestHistoryEntry(null, null, '/home'), + ); + await implicitView.debugInitializeHistory(strategy, useSingle: true); + + // Go to a named route. + await routeUpdated('/named-route'); + expect(strategy.currentEntry.url, '/named-route'); + + // Now, push a nameless route. The url shouldn't change. + // In a real app, this would be `Navigator.push(context, ...)`; + // Here, we simulate it by NOT calling `routeUpdated`. + + // Click back to pop the nameless route. + await strategy.go(-1); + + // A `popRoute` message should have been sent to the framework. + expect(spy.messages, hasLength(1)); + expect(spy.messages[0].channel, 'flutter/navigation'); + expect(spy.messages[0].methodName, 'popRoute'); + + // Because the popped route was nameless, the framework doesn't send any updated route + // information. + + // The url from before the nameless route should've been preserved. + expect(strategy.currentEntry.url, '/named-route'); + }); }); group('$MultiEntriesBrowserHistory', () {