Sandbox::https://github.com/flutter/flutter/pull/131123

vasilich6107 · 2023-07-22T13:02:08Z

This PR fixes the issue #108614
Particularly this behaviour #108614 (comment)

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

vasilich6107 · 2023-07-22T15:03:58Z

Hi @chunhtai
This is my PR for #108614

chunhtai

LGTM, but we should have one person from web team to also take a look at this pr

vasilich6107 · 2023-08-01T17:48:39Z

Hi @mdebbar
Could you check my PR

mdebbar · 2023-08-07T14:46:18Z

The change looks good but I'm concerned that this might break existing applications that rely on a full match on the path, eg:

MaterialApp(
  initialRoute: '/',
  routes: {
    '/': (_) => HomePage(),
    '/profile': (_) => ProfilePage(),
    '/settings': (_) => SettingsPage(),
  },
)

If someone navigates to /profile or /profile#foo, they both work today. But after this PR, /profile#foo won't match with the /profile route name anymore because the hash became part of the path. @chunhtai is this a valid concern to worry about?

In order to avoid the breakage, I suggest one of two options:

Implement your own PathAndHashUrlStrategy as follows:

class PathAndHashUrlStrategy extends PathUrlStrategy {
  final BrowserPlatformLocation _platformLocation = BrowserPlatformLocation();

  @override
  String getPath() {
    return super.getPath() + _platformLocation.hash;
  }
}

void main() {
  setUrlStrategy(PathAndHashUrlStrategy());
  runApp(MyApp());
}

Make this new behavior optional by adding a includeHash boolean to the PathUrlStrategy constructor.

chunhtai · 2023-08-07T15:08:38Z

yes this seems like a possible breaking change.

seems more straight forward.

vasilich6107 · 2023-08-08T08:00:42Z

Thanks for feedback
will implement 2.

vasilich6107 · 2023-09-15T10:06:52Z

Hi @mdebbar
I added the flag.
Could you check if it looks good for you

mdebbar

Looks good to me! Thanks for the contribution!

…lutter#131123)

flutter/flutter@b7d0e8c...8936504 2023-09-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from e1c784e3f841 to 589bde9a95c9 (16 revisions) (flutter/flutter#134998) 2023-09-19 polinach@google.com Reland Resolve breaking change of adding a method to ChangeNotifier. (flutter/flutter#134983) 2023-09-18 zanderso@users.noreply.github.com Revert "Resolve breaking change of adding a method to ChangeNotifier." (flutter/flutter#134978) 2023-09-18 polinach@google.com Resolve breaking change of adding a method to ChangeNotifier. (flutter/flutter#134953) 2023-09-18 47866232+chunhtai@users.noreply.github.com Fixes focus traversal crash if the current node can't request focus (flutter/flutter#134954) 2023-09-18 v.ditsyak@gmail.com Access to fragment in router state on page refresh (flutter/flutter#131123) 2023-09-18 goderbauer@google.com Enable private field promotion for framework (flutter/flutter#134473) 2023-09-18 sokolovskyi.konstantin@gmail.com Cover more test/widgets tests with leak tracking #6 (flutter/flutter#134884) 2023-09-18 polinach@google.com Cover more tests with leak tracking. (flutter/flutter#134805) 2023-09-18 sokolovskyi.konstantin@gmail.com Cover more test/widgets tests with leak tracking #5 (flutter/flutter#134869) 2023-09-18 15619084+vashworth@users.noreply.github.com Don't uninstall before retrying to connect during app launch (flutter/flutter#134542) 2023-09-18 xilaizhang@google.com [flutter roll] Revert "LinkedText (Linkify)" (flutter/flutter#134955) 2023-09-18 sokolovskyi.konstantin@gmail.com Fix memory leak in _DarwinViewState. (flutter/flutter#134938) 2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from be7a039c5451 to e1c784e3f841 (3 revisions) (flutter/flutter#134956) 2023-09-18 104349824+huycozy@users.noreply.github.com Migrate the "Missing frames / speed related performance issues" issue template to use Github forms (flutter/flutter#134033) 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 dit@google.com,rmistry@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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

This PR fixes the issue flutter#108614 Particularly this behaviour flutter#108614 (comment) Update editable_text.dart

…lutter#131123)

flutter/flutter@b7d0e8c...8936504 2023-09-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from e1c784e3f841 to 589bde9a95c9 (16 revisions) (flutter/flutter#134998) 2023-09-19 polinach@google.com Reland Resolve breaking change of adding a method to ChangeNotifier. (flutter/flutter#134983) 2023-09-18 zanderso@users.noreply.github.com Revert "Resolve breaking change of adding a method to ChangeNotifier." (flutter/flutter#134978) 2023-09-18 polinach@google.com Resolve breaking change of adding a method to ChangeNotifier. (flutter/flutter#134953) 2023-09-18 47866232+chunhtai@users.noreply.github.com Fixes focus traversal crash if the current node can't request focus (flutter/flutter#134954) 2023-09-18 v.ditsyak@gmail.com Access to fragment in router state on page refresh (flutter/flutter#131123) 2023-09-18 goderbauer@google.com Enable private field promotion for framework (flutter/flutter#134473) 2023-09-18 sokolovskyi.konstantin@gmail.com Cover more test/widgets tests with leak tracking flutter#6 (flutter/flutter#134884) 2023-09-18 polinach@google.com Cover more tests with leak tracking. (flutter/flutter#134805) 2023-09-18 sokolovskyi.konstantin@gmail.com Cover more test/widgets tests with leak tracking flutter#5 (flutter/flutter#134869) 2023-09-18 15619084+vashworth@users.noreply.github.com Don't uninstall before retrying to connect during app launch (flutter/flutter#134542) 2023-09-18 xilaizhang@google.com [flutter roll] Revert "LinkedText (Linkify)" (flutter/flutter#134955) 2023-09-18 sokolovskyi.konstantin@gmail.com Fix memory leak in _DarwinViewState. (flutter/flutter#134938) 2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from be7a039c5451 to e1c784e3f841 (3 revisions) (flutter/flutter#134956) 2023-09-18 104349824+huycozy@users.noreply.github.com Migrate the "Missing frames / speed related performance issues" issue template to use Github forms (flutter/flutter#134033) 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 dit@google.com,rmistry@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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

vasilich6107 force-pushed the bugfix/access_to_fragment_in_router_state_on_page_refresh branch 2 times, most recently from 2a86591 to e5e3827 Compare July 22, 2023 18:47

chunhtai self-requested a review July 24, 2023 15:36

goderbauer added the platform-web Web applications specifically label Jul 25, 2023

chunhtai approved these changes Jul 27, 2023

View reviewed changes

chunhtai requested a review from mdebbar July 27, 2023 18:15

vasilich6107 force-pushed the bugfix/access_to_fragment_in_router_state_on_page_refresh branch from e5e3827 to f29996c Compare August 3, 2023 09:09

chunhtai requested a review from yjbanov August 3, 2023 15:56

chunhtai mentioned this pull request Aug 3, 2023

[go_router] Access to fragment(/#access_token=bla_bla_bla) in router state #108614

Closed

vasilich6107 force-pushed the bugfix/access_to_fragment_in_router_state_on_page_refresh branch 3 times, most recently from 83eb873 to dfe04ea Compare September 15, 2023 09:21

Access to fragment in router state on page refresh

680a013

vasilich6107 force-pushed the bugfix/access_to_fragment_in_router_state_on_page_refresh branch from dfe04ea to 680a013 Compare September 15, 2023 09:22

mdebbar approved these changes Sep 18, 2023

View reviewed changes

mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 18, 2023

auto-submit Bot merged commit 43407cf into flutter:master Sep 18, 2023

vasilich6107 deleted the bugfix/access_to_fragment_in_router_state_on_page_refresh branch September 19, 2023 05:17

engine-flutter-autoroll mentioned this pull request Sep 19, 2023

Roll Flutter from b7d0e8c9ed70 to 893650416352 (15 revisions) flutter/packages#4947

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 19, 2023

43407cf Access to fragment in router state on page refresh (flutter/f…

5426b16

…lutter#131123)

Mairramer pushed a commit to Mairramer/flutter that referenced this pull request Oct 10, 2023

Access to fragment in router state on page refresh (flutter#131123)

1857911

This PR fixes the issue flutter#108614 Particularly this behaviour flutter#108614 (comment) Update editable_text.dart

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

43407cf Access to fragment in router state on page refresh (flutter/f…

802c831

…lutter#131123)

tomekit mentioned this pull request May 30, 2024

Route path doesn't match deeplink if it contains data after # #122058

Closed

Navigation Menu

Repository navigation

Access to fragment in router state on page refresh - #131123

Access to fragment in router state on page refresh#131123
auto-submit[bot] merged 1 commit into
flutter:masterfrom
artflutter:bugfix/access_to_fragment_in_router_state_on_page_refresh

vasilich6107 commented Jul 22, 2023

Uh oh!

vasilich6107 commented Jul 22, 2023

Uh oh!

chunhtai left a comment

Uh oh!

vasilich6107 commented Aug 1, 2023

Uh oh!

mdebbar commented Aug 7, 2023

Uh oh!

chunhtai commented Aug 7, 2023

Uh oh!

vasilich6107 commented Aug 8, 2023

Uh oh!

vasilich6107 commented Sep 15, 2023

Uh oh!

mdebbar left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

vasilich6107 commented Jul 22, 2023

Pre-launch Checklist

Uh oh!

vasilich6107 commented Jul 22, 2023

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

vasilich6107 commented Aug 1, 2023

Uh oh!

mdebbar commented Aug 7, 2023

Uh oh!

chunhtai commented Aug 7, 2023

Uh oh!

vasilich6107 commented Aug 8, 2023

Uh oh!

vasilich6107 commented Sep 15, 2023

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants