Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iOS/Catalyst] Correctly call DidFinishNavigation in NavigationDelegate #20725

Merged
merged 4 commits into from Feb 22, 2024

Conversation

drasticactions
Copy link
Contributor

Description of Change

  • Mark MauiWKWebView DidFinishNavigation as Obsolete. This API is never being called due to the Navigation delegate overriding it, but since it's Public, someone may call it directly so they could be relying on that implementation. Marking it obsolete should at least point people to the actual implementation that's called.
  • Check for WebViewHandler in the MAUI Navigation Delegate and call ProcessNavigatedAsync. This invokes underlying Cookie syncing processes that were previously not being called unless you manually invoked "DidFinishNavigation" yourself on the "MauiWKWebView"

Issues Fixed

Fixes #20713

@rmarinho
Copy link
Member

Should this go to net9 ?

@drasticactions
Copy link
Contributor Author

Should this go to net9 ?

MauiWKWebView.DidFinishNavigation was never being called since it was originally introduced, unless someone outside of this project happened to call it manually (IMO that's unlikely, especially since it was intended to be an ObjC selector export and run automatically by the WebView.) Making it clear where your general overrides should be set (inside the navigation delegate) where that code is always being run should be safe for Main (and by extension, net8.0). Or at the very least, it wouldn't regress it further than it already has.

As for the Cookie code itself, as far as I can tell that was being tested (so it should be safe) but it wasn't run in the context of most user apps, unless someone happened to invoke that DidFinishNavigation themselves. This should work but it has more risk since this is a new codepath that most people wouldn't have hit before. I could see that maybe only being applied for net9.0, but I think the case can be made to have it everywhere.

@PureWeen
Copy link
Member

Failing tests unrelated

@PureWeen PureWeen merged commit 85adc13 into dotnet:main Feb 22, 2024
45 of 47 checks passed
rmarinho pushed a commit that referenced this pull request Feb 22, 2024
…te (#20725)

* Invoke ProcessNavigatedAsync for MauiWKWebView

* Add underlying API back

* Revert sandbox changes

* Use FireAndForget
rmarinho pushed a commit that referenced this pull request Feb 27, 2024
…te (#20725)

* Invoke ProcessNavigatedAsync for MauiWKWebView

* Add underlying API back

* Revert sandbox changes

* Use FireAndForget
@github-actions github-actions bot locked and limited conversation to collaborators Mar 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maui iOS WebView - MauiWKWebView DidFinishNavigation not called.
4 participants