[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

  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

      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 [email protected] Roll Dart SDK from 9b4691f35139 to 214a7f829913 (2 revisions) (flutter/flutter#173769) 2025-08-14 [email protected] Roll Skia from b3e86773dae1 to dca5f05fee87 (4 revisions) (flutter/flutter#173763) 2025-08-14 [email protected] Roll Dart SDK from 73153bdc1459 to 9b4691f35139 (3 revisions) (flutter/flutter#173755) 2025-08-14 [email protected] Roll Skia from 5852eddfd404 to b3e86773dae1 (1 revision) (flutter/flutter#173750) 2025-08-14 [email protected] Allow empty initial time when using text input mode in showTimePicker dialog (flutter/flutter#172847) 2025-08-13 [email protected] Roll Skia from 525e2bf80559 to 5852eddfd404 (2 revisions) (flutter/flutter#173740) 2025-08-13 [email protected] [web] Popping a nameless route should preserve the correct route name (flutter/flutter#173652) 2025-08-13 [email protected] Make sure that a ChoiceChip doesn't crash in 0x0 environment (flutter/flutter#173322) 2025-08-13 [email protected] [ Tool ] Fix missing import for widget_preview.dart (flutter/flutter#173731) 2025-08-13 [email protected] Roll Skia from f7fdda3cd0e6 to 525e2bf80559 (7 revisions) (flutter/flutter#173727) 2025-08-13 [email protected] Do not include `:unittests` unless `enable_unittests` (flutter/flutter#173729) 2025-08-13 [email protected] Roll Packages from 08a9b2c to 6cb9113 (1 revision) (flutter/flutter#173726) 2025-08-13 [email protected] fix: selected date decorator renders outside PageView in `DatePickerDialog` dialog (flutter/flutter#171718) 2025-08-13 [email protected] [ Widget Preview ] Add `--machine` mode (flutter/flutter#173654) 2025-08-13 [email protected] Make sure that a Chip doesn't crash in 0x0 environment (flutter/flutter#173245) 2025-08-13 [email protected] feat: Cupertino sheet implement upward stretch on full sheet (flutter/flutter#168547) 2025-08-13 [email protected] Fix visual overlap of transparent routes barrier when using FadeForwardsPageTransitionsBuilder (flutter/flutter#167032) 2025-08-13 [email protected] 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 [email protected],[email protected] 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 [email protected] Roll Dart SDK from 9b4691f35139 to 214a7f829913 (2 revisions) (flutter/flutter#173769) 2025-08-14 [email protected] Roll Skia from b3e86773dae1 to dca5f05fee87 (4 revisions) (flutter/flutter#173763) 2025-08-14 [email protected] Roll Dart SDK from 73153bdc1459 to 9b4691f35139 (3 revisions) (flutter/flutter#173755) 2025-08-14 [email protected] Roll Skia from 5852eddfd404 to b3e86773dae1 (1 revision) (flutter/flutter#173750) 2025-08-14 [email protected] Allow empty initial time when using text input mode in showTimePicker dialog (flutter/flutter#172847) 2025-08-13 [email protected] Roll Skia from 525e2bf80559 to 5852eddfd404 (2 revisions) (flutter/flutter#173740) 2025-08-13 [email protected] [web] Popping a nameless route should preserve the correct route name (flutter/flutter#173652) 2025-08-13 [email protected] Make sure that a ChoiceChip doesn't crash in 0x0 environment (flutter/flutter#173322) 2025-08-13 [email protected] [ Tool ] Fix missing import for widget_preview.dart (flutter/flutter#173731) 2025-08-13 [email protected] Roll Skia from f7fdda3cd0e6 to 525e2bf80559 (7 revisions) (flutter/flutter#173727) 2025-08-13 [email protected] Do not include `:unittests` unless `enable_unittests` (flutter/flutter#173729) 2025-08-13 [email protected] Roll Packages from 08a9b2c to 6cb9113 (1 revision) (flutter/flutter#173726) 2025-08-13 [email protected] fix: selected date decorator renders outside PageView in `DatePickerDialog` dialog (flutter/flutter#171718) 2025-08-13 [email protected] [ Widget Preview ] Add `--machine` mode (flutter/flutter#173654) 2025-08-13 [email protected] Make sure that a Chip doesn't crash in 0x0 environment (flutter/flutter#173245) 2025-08-13 [email protected] feat: Cupertino sheet implement upward stretch on full sheet (flutter/flutter#168547) 2025-08-13 [email protected] Fix visual overlap of transparent routes barrier when using FadeForwardsPageTransitionsBuilder (flutter/flutter#167032) 2025-08-13 [email protected] 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 [email protected],[email protected] 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

…flutter#173652) Fixes flutter#173356

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

…flutter#173652) Fixes flutter#173356

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

…route name (flutter/flutter#173652)

…flutter#173652) Fixes flutter#173356

…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 related. 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

mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025

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

4c5999a

…flutter#173652) Fixes flutter#173356

mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025

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

399dfa4

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

korca0220 pushed a commit to korca0220/flutter that referenced this pull request Sep 22, 2025

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

e7a5898

…flutter#173652) Fixes flutter#173356

korca0220 pushed a commit to korca0220/flutter that referenced this pull request Sep 22, 2025

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

ae999be

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

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

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

bdd8d24

…route name (flutter/flutter#173652)

lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025

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

5ef5d21

…flutter#173652) Fixes flutter#173356

lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025

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

5e03cf0

…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

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	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

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants