-
Notifications
You must be signed in to change notification settings - Fork 405
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
Fix for #2407 - MapHandler Throws InvalidSystemOperationExcetption #2409
base: main
Are you sure you want to change the base?
Conversation
@@ -179,6 179,8 @@ static async Task CallJSMethod(FrameworkElement platformWebView, string script) | |||
{ | |||
if (platformWebView is WebView2 webView2) | |||
{ | |||
await webView2.EnsureCoreWebView2Async(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we only need to call it once. lets move it to CreatePlatformView
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @VladislavAntonyuk
The method should be awaited.
Moving to CreatePlatformView would require modification of that method to async.
Was unsure of side effects.
In the remarks for the call....
https://learn.microsoft.com/en-us/dotnet/api/microsoft.web.webview2.wpf.webview2.ensurecorewebview2async
there is no issue calling this mutiple times, and potentially a change in the view might need to call this again, therefore I placed it here.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's no issue calling EnsureCoreWebView2Async
multiple times, I'm cool with keeping it in CallJSMethod
.
We could call EnsureCoreWebView2Async
in CreatePlatformView()
, but we'd need to include a fire-and-forget async void
, and I prefer to avoid this given that calling EnsureCoreWebView2Async
from CallJSMethod
works.
Just for fun, here's how we would modify CreatePlatformView
:
protected override FrameworkElement CreatePlatformView()
{
if (string.IsNullOrEmpty(MapsKey))
{
throw new InvalidOperationException("You need to specify a Bing Maps Key");
}
var mapPage = GetMapHtmlPage(MapsKey);
var webView = new MauiWebView(new WebViewHandler());
webView.NavigationCompleted = HandleWebViewNavigationCompleted;
webView.WebMessageReceived = WebViewWebMessageReceived;
webView.LoadHtml(mapPage, null);
InitializeMap(webView);
return webView;
// Use async-void to properly use async/await to initialize the map from a non-async method
static async void InitializeMap(MauiWebView webView2)
{
await webView2.EnsureCoreWebView2Async();
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mikelor!
I'm cool merging this as long as @VladislavAntonyuk approves.
FYI - I added a commit renaming CallJSMethod
to TryCallJSMethod
since it is possible for the method call to be unsuccessful. That said, I couldn't replicate a scenario where it does fail, so I didn't modify any of the calling methods. But, the updated method syntax may help us fix a future bug!
I also updated CommunityToolkit.Maui.csproj
to avoid emitting compiler-generated code to help us avoid Windows Long Path issues, matching CommunityToolkit.Maui.Sample.csproj
.
I'm sorry. I don't see the difference between the original code and this one except for the renamed method and waiting for a script to execute. How does the code fix the problem? |
Fixes #2407
Description of Change
Explicitly trigger the initialization of the control's CoreWebView2.Call EnsureCoreWebView2Async() in the MapHandler's CallJSMethod() before trying to execute script.
No automated tests have been created.
Linked Issues
PR Checklist
approved
(bug) orChampioned
(feature/proposal)main
at time of PRAdditional information