[web] Popping a nameless route should preserve the correct route name #173652

mdebbar · 2025-08-12T21:08:05Z

gemini-code-assist

Code Review

This pull request addresses an issue where popping a nameless route on the web did not preserve the URL of the previous named route. The fix involves introducing _currentRouteName to correctly track and restore the route name. The implementation is solid, simplifying the existing logic and adding a specific test case to validate the correction. The changes are well-executed and effectively resolve the bug.

chunhtai

LGTM, just some minor comment

chunhtai · 2025-08-13T16:14:19Z

engine/src/flutter/lib/web_ui/lib/src/engine/navigation/history.dart

    final String path = currentPath;
-    if (!_isFlutterEntry(domWindow.history.state)) {
+    _currentRouteName = path;


_currentRouteName = currentPath

chunhtai · 2025-08-13T16:17:30Z

engine/src/flutter/lib/web_ui/lib/src/engine/navigation/history.dart

@@ -303,11 +313,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;


i think routeName will never be null given since routeUpdated method has been deprecated for ages.

Do you think it's safe to remove the entire routeUpdated handler:

flutter/engine/src/flutter/lib/web_ui/lib/src/engine/window.dart

Lines 628 to 632 in 1eb2f68

case 'routeUpdated': // deprecated

assert(arguments != null);

await _useSingleEntryBrowserHistory();

browserHistory.setRouteName(arguments!.tryString('routeName'));

return true;

yes, I think so, it has not been used since we have introduced Router. I forgot to clean it up

Ok, I'll do this cleanup in a follow up PR.

chunhtai · 2025-08-13T16:26:25Z

engine/src/flutter/lib/web_ui/test/engine/history_test.dart

@@ -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.


not something need to be address in this pr, routeUpdated is not used by framework for a long time now

chunhtai · 2025-08-13T16:28:52Z

engine/src/flutter/lib/web_ui/test/engine/history_test.dart

+      await implicitView.debugInitializeHistory(strategy, useSingle: true);
+
+      // Go to a named route.
+      await routeUpdated('/named-route');


let's use routeInformationUpdated for accuracy

…route name (flutter/flutter#173652)

flutter/flutter@34c2a3b...f4334d2 2025-08-14 engine-flutter-autoroll@skia.org Roll Dart SDK from 9b4691f35139 to 214a7f829913 (2 revisions) (flutter/flutter#173769) 2025-08-14 engine-flutter-autoroll@skia.org Roll Skia from b3e86773dae1 to dca5f05fee87 (4 revisions) (flutter/flutter#173763) 2025-08-14 engine-flutter-autoroll@skia.org Roll Dart SDK from 73153bdc1459 to 9b4691f35139 (3 revisions) (flutter/flutter#173755) 2025-08-14 engine-flutter-autoroll@skia.org Roll Skia from 5852eddfd404 to b3e86773dae1 (1 revision) (flutter/flutter#173750) 2025-08-14 azat24680@gmail.com Allow empty initial time when using text input mode in showTimePicker dialog (flutter/flutter#172847) 2025-08-13 engine-flutter-autoroll@skia.org Roll Skia from 525e2bf80559 to 5852eddfd404 (2 revisions) (flutter/flutter#173740) 2025-08-13 mdebbar@google.com [web] Popping a nameless route should preserve the correct route name (flutter/flutter#173652) 2025-08-13 ahmedsameha1@gmail.com Make sure that a ChoiceChip doesn't crash in 0x0 environment (flutter/flutter#173322) 2025-08-13 bkonyi@google.com [ Tool ] Fix missing import for widget_preview.dart (flutter/flutter#173731) 2025-08-13 engine-flutter-autoroll@skia.org Roll Skia from f7fdda3cd0e6 to 525e2bf80559 (7 revisions) (flutter/flutter#173727) 2025-08-13 matanlurey@users.noreply.github.com Do not include `:unittests` unless `enable_unittests` (flutter/flutter#173729) 2025-08-13 engine-flutter-autoroll@skia.org Roll Packages from 08a9b2c to 6cb9113 (1 revision) (flutter/flutter#173726) 2025-08-13 56157868+albinpk@users.noreply.github.com fix: selected date decorator renders outside PageView in `DatePickerDialog` dialog (flutter/flutter#171718) 2025-08-13 bkonyi@google.com [ Widget Preview ] Add `--machine` mode (flutter/flutter#173654) 2025-08-13 ahmedsameha1@gmail.com Make sure that a Chip doesn't crash in 0x0 environment (flutter/flutter#173245) 2025-08-13 returnymgstokh@icloud.com feat: Cupertino sheet implement upward stretch on full sheet (flutter/flutter#168547) 2025-08-13 131446187+TheLastFlame@users.noreply.github.com Fix visual overlap of transparent routes barrier when using FadeForwardsPageTransitionsBuilder (flutter/flutter#167032) 2025-08-13 32538273+ValentinVignal@users.noreply.github.com Fix `ChipThemeData` lerp for `BorderSide` (flutter/flutter#173160) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC louisehsu@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…flutter#173652) Fixes flutter#173356

This is a follow up to #173652 Closes #50836

…flutter#173652) Fixes flutter#173356

…3782) This is a follow up to flutter#173652 Closes flutter#50836

…r#9807) flutter/flutter@34c2a3b...f4334d2 2025-08-14 engine-flutter-autoroll@skia.org Roll Dart SDK from 9b4691f35139 to 214a7f829913 (2 revisions) (flutter/flutter#173769) 2025-08-14 engine-flutter-autoroll@skia.org Roll Skia from b3e86773dae1 to dca5f05fee87 (4 revisions) (flutter/flutter#173763) 2025-08-14 engine-flutter-autoroll@skia.org Roll Dart SDK from 73153bdc1459 to 9b4691f35139 (3 revisions) (flutter/flutter#173755) 2025-08-14 engine-flutter-autoroll@skia.org Roll Skia from 5852eddfd404 to b3e86773dae1 (1 revision) (flutter/flutter#173750) 2025-08-14 azat24680@gmail.com Allow empty initial time when using text input mode in showTimePicker dialog (flutter/flutter#172847) 2025-08-13 engine-flutter-autoroll@skia.org Roll Skia from 525e2bf80559 to 5852eddfd404 (2 revisions) (flutter/flutter#173740) 2025-08-13 mdebbar@google.com [web] Popping a nameless route should preserve the correct route name (flutter/flutter#173652) 2025-08-13 ahmedsameha1@gmail.com Make sure that a ChoiceChip doesn't crash in 0x0 environment (flutter/flutter#173322) 2025-08-13 bkonyi@google.com [ Tool ] Fix missing import for widget_preview.dart (flutter/flutter#173731) 2025-08-13 engine-flutter-autoroll@skia.org Roll Skia from f7fdda3cd0e6 to 525e2bf80559 (7 revisions) (flutter/flutter#173727) 2025-08-13 matanlurey@users.noreply.github.com Do not include `:unittests` unless `enable_unittests` (flutter/flutter#173729) 2025-08-13 engine-flutter-autoroll@skia.org Roll Packages from 08a9b2c to 6cb9113 (1 revision) (flutter/flutter#173726) 2025-08-13 56157868+albinpk@users.noreply.github.com fix: selected date decorator renders outside PageView in `DatePickerDialog` dialog (flutter/flutter#171718) 2025-08-13 bkonyi@google.com [ Widget Preview ] Add `--machine` mode (flutter/flutter#173654) 2025-08-13 ahmedsameha1@gmail.com Make sure that a Chip doesn't crash in 0x0 environment (flutter/flutter#173245) 2025-08-13 returnymgstokh@icloud.com feat: Cupertino sheet implement upward stretch on full sheet (flutter/flutter#168547) 2025-08-13 131446187+TheLastFlame@users.noreply.github.com Fix visual overlap of transparent routes barrier when using FadeForwardsPageTransitionsBuilder (flutter/flutter#167032) 2025-08-13 32538273+ValentinVignal@users.noreply.github.com Fix `ChipThemeData` lerp for `BorderSide` (flutter/flutter#173160) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC louisehsu@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…3782) This is a follow up to flutter#173652 Closes flutter#50836

[web] Popping a nameless route should preserve the correct route name

b2723b8

mdebbar requested a review from chunhtai August 12, 2025 21:08

github-actions bot added engine flutter/engine repository. See also e: labels. platform-web Web applications specifically labels Aug 12, 2025

gemini-code-assist bot reviewed Aug 12, 2025

View reviewed changes

chunhtai reviewed Aug 13, 2025

View reviewed changes

chunhtai approved these changes Aug 13, 2025

View reviewed changes

inline variable

4ace6b0

mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 13, 2025

auto-submit bot added this pull request to the merge queue Aug 13, 2025

Merged via the queue into flutter:master with commit f83d8cf Aug 14, 2025
180 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 14, 2025

engine-flutter-autoroll mentioned this pull request Aug 14, 2025

Roll Flutter from 34c2a3b158b2 to c932faf75472 (17 revisions) flutter/packages#9806

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025

f83d8cfd3 [web] Popping a nameless route should preserve the correct …

1cf3974

…route name (flutter/flutter#173652)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025

f83d8cfd3 [web] Popping a nameless route should preserve the correct …

8d5938a

…route name (flutter/flutter#173652)

engine-flutter-autoroll mentioned this pull request Aug 14, 2025

Roll Flutter from 34c2a3b158b2 to f4334d27934b (18 revisions) flutter/packages#9807

Merged

mdebbar deleted the history_nameless_route branch August 14, 2025 16:41

mdebbar mentioned this pull request Aug 14, 2025

[web] Cleanup usages of deprecated routeUpdated message #173782

Merged

SydneyBao pushed a commit to SydneyBao/flutter that referenced this pull request Aug 14, 2025

[web] Popping a nameless route should preserve the correct route name (…

cb6a15a

…flutter#173652) Fixes flutter#173356

github-merge-queue bot pushed a commit that referenced this pull request Aug 14, 2025

[web] Cleanup usages of deprecated routeUpdated message (#173782)

62904ba

This is a follow up to #173652 Closes #50836

Piinks mentioned this pull request Aug 15, 2025

Flutter Notable Commits - 2025 #161460

Open

ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Aug 19, 2025

[web] Popping a nameless route should preserve the correct route name (…

32e1d07

…flutter#173652) Fixes flutter#173356

ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Aug 19, 2025

[web] Cleanup usages of deprecated routeUpdated message (flutter#17…

c4bf640

…3782) This is a follow up to flutter#173652 Closes flutter#50836

SydneyBao pushed a commit to SydneyBao/flutter that referenced this pull request Aug 22, 2025

[web] Cleanup usages of deprecated routeUpdated message (flutter#17…

89fd9a5

…3782) This is a follow up to flutter#173652 Closes flutter#50836

SydneyBao pushed a commit to SydneyBao/flutter that referenced this pull request Aug 22, 2025

[web] Cleanup usages of deprecated routeUpdated message (flutter#17…

4376eaf

…3782) This is a follow up to flutter#173652 Closes flutter#50836

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[web] Popping a nameless route should preserve the correct route name #173652

[web] Popping a nameless route should preserve the correct route name #173652

mdebbar commented Aug 12, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

chunhtai left a comment

Uh oh!

chunhtai Aug 13, 2025

Uh oh!

chunhtai Aug 13, 2025

Uh oh!

mdebbar Aug 13, 2025

Uh oh!

chunhtai Aug 13, 2025

Uh oh!

mdebbar Aug 13, 2025

Uh oh!

chunhtai Aug 13, 2025

Uh oh!

chunhtai Aug 13, 2025

Uh oh!

Uh oh!

Uh oh!

	case 'routeUpdated': // deprecated
	assert(arguments != null);
	await _useSingleEntryBrowserHistory();
	browserHistory.setRouteName(arguments!.tryString('routeName'));
	return true;

[web] Popping a nameless route should preserve the correct route name #173652

[web] Popping a nameless route should preserve the correct route name #173652

Conversation

mdebbar commented Aug 12, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

chunhtai Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

chunhtai Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

mdebbar Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

chunhtai Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

mdebbar Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

chunhtai Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

chunhtai Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!