Repository navigation
PlatformRouteInformationProvider does not push new entry if query par… - #130457
Merged
Merged
Conversation
…ameter is semanticsally the same
github-actions
Bot
added
framework
goderbauer
reviewed
Jul 13, 2023
| }) : _value = initialRouteInformation; | ||
|
|
||
| static bool _equals(Uri a, Uri b) { | ||
| return a.path == b.path |
Member
There was a problem hiding this comment.
What about all the other properties a URI can have? E.g. host, scheme, etc? If the only problem is that query params are out of order, would it make sense to remove the query params from both URIs, compare that those URIs are equal and then check the querParams separately?
Contributor
Author
There was a problem hiding this comment.
host and scheme only make sense if this is coming from the engine. I can't find a reasonable use case that the state in framework can translate into host and scheme. That means the host and scheme is always empty when framework reports uri to engine.
If we compares the host and scheme here, we would send the uri back with replace: false if the path/queryparam/fragment is the same but host and scheme are different. I think that would confuse the engine.
Contributor
Author
There was a problem hiding this comment.
we could always check for the host and scheme in the future if we identify a use case
Contributor
Author
There was a problem hiding this comment.
If we compare host and scheme now, this would only affect mobile platform as the host and scheme will only be filled out if this is a deeplink.
So we would immediately send a replace: false with the same uri but empty host and scheme to engine. Which currently is a noop for non web platform. so either way wouldn't change the behavior, but if people listen to the navigation channel for some reason, they may get some confusing result.
chunhtai
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 14, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 14, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 14, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 14, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 14, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 14, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 14, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 14, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 14, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 15, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 15, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 15, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 16, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 16, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 17, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 17, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 17, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 17, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 17, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 17, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 17, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 17, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 18, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 18, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 18, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 18, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 18, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 18, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 18, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 18, 2023
LouiseHsu
pushed a commit
to LouiseHsu/flutter
that referenced
this pull request
Jul 31, 2023
flutter#130457) �ameter is semanticsally the same The URI compare does not taking into account that query parameter may or may not be encoded, or the parameters' order can be different. However, they are all semantically the same. This pr makes PlatformRouteInformationProvider to take those into account when deciding whether it should push/replace the browser history entry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ameter is semanticsally the same
The URI compare does not taking into account that query parameter may or may not be encoded, or the parameters' order can be different. However, they are all semantically the same.
This pr makes PlatformRouteInformationProvider to take those into account when deciding whether it should push/replace the browser history entry.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.