Skip to content

Conversation

@mdebbar
Copy link
Contributor

@mdebbar mdebbar commented May 15, 2023

Remove the following APIs and export them directly from dart:ui_web:

  • urlStrategy getter and setter
  • HashUrlStrategy
  • PlatformLocation and BrowserPlatformLocation (keep the façades for non-web platforms)

Depends on flutter/engine#42043
Depends on flutter/engine#42252

Part of #126831

@mdebbar mdebbar requested a review from eyebrowsoffire May 15, 2023 16:40
Copy link
Contributor

@eyebrowsoffire eyebrowsoffire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

auto-submit bot pushed a commit to flutter/engine that referenced this pull request May 24, 2023
In order to use the exported `PlatformLocation` cross-platform, we need to remove any JS types from the interface. Namely, `DomEventListener`.

Required by flutter/flutter#126851
auto-submit bot pushed a commit to flutter/engine that referenced this pull request May 26, 2023
Up to this point, whenever users use the `PathUrlStrategy`, they would be using `flutter_web_plugins`'s copy of `BrowserPlatformLocation` which has a [different implementation for `getBaseHref()`](https://github.com/flutter/flutter/blob/9376a2ac6003740c357688a376870dbe84a88883/packages/flutter_web_plugins/lib/src/navigation/url_strategy.dart#L235). So we never hit the [implementation in the engine](https://github.com/flutter/engine/blob/eed12f36f5956a37fb4ba13f1b670a5d41c37edb/lib/web_ui/lib/ui_web/src/ui_web/navigation/platform_location.dart#L138), meaning the bug was invisible.

Now that we are [removing](flutter/flutter#126851) `BrowserPlatformLocation` from `flutter_web_plugins`, we are hitting the engine's implementation which contains the bug being solved in this PR.
@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label May 31, 2023
@auto-submit auto-submit bot merged commit 7eecf88 into flutter:master May 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 3, 2023
auto-submit bot pushed a commit that referenced this pull request Jun 6, 2023
- Use `Uri.parse()` to extract pathname.
- Remove unused code from `utils.dart`.
- Add test for URL encoding.

(need to wait for #126851 to make it into Google3)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants