-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
HybridWebView: Invoke JS methods from .NET #23769
Conversation
59f7393
to
8a11cb1
Compare
Handler?.Invoke(nameof(IHybridWebView.SendRawMessage), rawMessage); | ||
Handler?.Invoke( | ||
nameof(IHybridWebView.SendRawMessage), | ||
new HybridWebViewRawMessage |
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.
@mattleibow - this is the follow-up to a comment you had on an earlier PR. I added this 'container type' for better future-proofing.
platformView.CoreWebView2.Settings.AreDevToolsEnabled = true;//EnableWebDevTools; | ||
platformView.CoreWebView2.Settings.IsWebMessageEnabled = true; | ||
platformView.CoreWebView2.AddWebResourceRequestedFilter($"{AppOrigin}*", CoreWebView2WebResourceContext.All); | ||
PlatformView.DispatcherQueue.TryEnqueue(async () => |
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.
There are now a few places in the code where the Windows code that uses WebView2 does a "ensure WebView2 is initialized, and then execute" pattern. This is because WebView2 is the only webview we use that has an async pattern to ensure it is ready. For that reason in places where the code is synchronous but needs WebView2 to be ready, it dispatches the work item to an async task, and awaits ready-ness, and then does what it wants. This is the same pattern we did in BlazorWebView.
e1e8f8a
to
218ce12
Compare
Tested locally on Windows, Android Emulator, iOS Simulator, and MacCatalyst. Will add more automated tests now. |
654e690
to
9b1087f
Compare
Everything on CI is passing except for the |
Also fixes #6446? |
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.
Overall looks good to me! Just some small things!
src/Controls/samples/Controls.Sample/Pages/Controls/HybridWebViewPage.xaml.cs
Outdated
Show resolved
Hide resolved
src/Controls/tests/DeviceTests/Elements/HybridWebView/HybridWebViewTests.cs
Show resolved
Hide resolved
src/Controls/tests/DeviceTests/Elements/HybridWebView/HybridWebViewTests.cs
Outdated
Show resolved
Hide resolved
src/Controls/tests/DeviceTests/Elements/HybridWebView/HybridWebViewTests.cs
Outdated
Show resolved
Hide resolved
74698f4
to
793a887
Compare
|
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.
This PR is mostly good with regards to code and implementation, however it is starting to break the "rules" of the architecture and separation.
The XAML controls should not have much processing (especially platform-specific processing) and instead let the handler do it all. Also, if there is any platform specific code in the XAML controls, then it is highly likely that it should have been in Core.
In the cases where there is no implementation for commands/mappers, the XAML control must still pass the message/event/command to the handler and let the handler control its behavior. This allows for the Community Toolkit (and developers) customize the behavior.
I also see the code in the XAML layer deciding that the best way to communicate is via JSON. This may be fine, but is it the most correct? Are there cases where there may be communication in a binary form or some custom/alternate serialization. A developer may wish to use their own models and objects that are processed in a different library.
void IHybridWebView.RawMessageReceived(string rawMessage) | ||
void IHybridWebView.MessageReceived(string rawMessage) |
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.
This processing should be done in Core and fire specific methods in Controls. There is no need for XAML objects to be post processing and parsing strings.
#if WINDOWS || ANDROID || IOS || MACCATALYST | ||
public async Task<string?> InvokeJavaScriptAsync(string methodName, object?[]? paramValues, JsonTypeInfo?[]? paramJsonTypeInfos = null) |
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.
There should be almost no #if
things in Controls as it really is there for cases where it is not possible to do it any way else.
No matter if the operation is not supported, the Controls layer should always send the command to the handler where the work is done. If the handler decides it is not supported, then the handler can throw. This also allows the user to override the full operation if need be to make it work.
if (DeviceInfo.Platform != DevicePlatform.Android) | ||
{ | ||
script = WebView.EscapeJsString(script); | ||
|
||
if (DeviceInfo.Platform != DevicePlatform.WinUI) | ||
{ | ||
// Use JSON.stringify() method to converts a JavaScript value to a JSON string | ||
script = "try{JSON.stringify(eval('" + script + "'))}catch(e){'null'};"; | ||
} | ||
else | ||
{ | ||
script = "try{eval('" + script + "')}catch(e){'null'};"; | ||
} | ||
} |
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.
Most of this function, and especially this code should be in Core via a command. The handler should be doing this instead of checking the platform. Android knows it does not need this, so it does not need to have this code.
void RawMessageReceived(string rawMessage); | ||
void MessageReceived(string rawMessage); |
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.
Instead of having the processing done in Controls, it should be here with maybe an event args with the message type:
void MessageReceived(HybridWebViewMessageRecieved message);
class HybridWebViewMessageRecieved {
}
class HybridWebViewRawMessageRecieved : HybridWebViewMessageRecieved {
}
class HybridWebViewInvokeMessageRecieved : HybridWebViewMessageRecieved {
}
This allows for easy extension and possibly better handler-specific implementations instead of parsing in XAML controls.
Context: dotnet#23769 Context: dotnet/android#9300 026e046 introduced `HybridWebView`, which unfortunately introduces trimmer warnings in the `dotnet new maui` project template: > dotnet new maui > dotnet build -f net9.0-android -c Release -p:TrimMode=Full ... hellomaui succeeded with 1 warning(s) (7.9s) /_/src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.Android.cs(53,5): Trim analysis warning IL2026: Microsoft.Maui.Handlers.HybridWebViewHandler.HybridWebViewJavaScriptInterface.SendMessage(String): Using member 'Java.Interop.ExportAttribute.ExportAttribute(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. [ExportAttribute] uses dynamic features. This is due to usage of `Java.Interop.ExportAttribute`: private sealed class HybridWebViewJavaScriptInterface : Java.Lang.Object { //... [JavascriptInterface] [Export("sendMessage")] public void SendMessage(string message) `Java.Interop.ExportAttribute` makes heavy usage of unbounded System.Reflection, System.Reflection.Emit, etc. for it to be able to work. It brings in an additional assembly `Mono.Android.Export.dll` as well. It is inherently trimming unsafe, but how did it get through these tests? https://github.com/dotnet/maui/blob/08ff1246383ed4fdaef84a40d5b2ae8e6096bb56/src/TestUtils/src/Microsoft.Maui.IntegrationTests/AndroidTemplateTests.cs#L50-L61 This slipped through, unfortunately, as we had missed solving all the trimmer warnings in `Mono.Android.Export.dll`! dotnet/android#9300 aims to fix that. After dotnet/android#9300, new trimming warnings specific to .NET MAUI will surface such as the one above. But we can easily replace `[Export]` by: * Define a Java interface & create a binding for it in C#, we already have `maui.aar` setup for this. * We can simply implement the interface in C# and remove `[Export]`. Lastly, I fixed some of the defaults in `Metadata.xml`, it didn't look like we were automatically making Java interfaces `internal`. It looks like we probably made `ImageLoaderCallback` public by mistake.
Context: #23769 Context: dotnet/android#9300 026e046 introduced `HybridWebView`, which unfortunately introduces trimmer warnings in the `dotnet new maui` project template: > dotnet new maui > dotnet build -f net9.0-android -c Release -p:TrimMode=Full ... hellomaui succeeded with 1 warning(s) (7.9s) /_/src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.Android.cs(53,5): Trim analysis warning IL2026: Microsoft.Maui.Handlers.HybridWebViewHandler.HybridWebViewJavaScriptInterface.SendMessage(String): Using member 'Java.Interop.ExportAttribute.ExportAttribute(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. [ExportAttribute] uses dynamic features. This is due to usage of `Java.Interop.ExportAttribute`: private sealed class HybridWebViewJavaScriptInterface : Java.Lang.Object { //... [JavascriptInterface] [Export("sendMessage")] public void SendMessage(string message) `Java.Interop.ExportAttribute` makes heavy usage of unbounded System.Reflection, System.Reflection.Emit, etc. for it to be able to work. It brings in an additional assembly `Mono.Android.Export.dll` as well. It is inherently trimming unsafe, but how did it get through these tests? https://github.com/dotnet/maui/blob/08ff1246383ed4fdaef84a40d5b2ae8e6096bb56/src/TestUtils/src/Microsoft.Maui.IntegrationTests/AndroidTemplateTests.cs#L50-L61 This slipped through, unfortunately, as we had missed solving all the trimmer warnings in `Mono.Android.Export.dll`! dotnet/android#9300 aims to fix that. After dotnet/android#9300, new trimming warnings specific to .NET MAUI will surface such as the one above. But we can easily replace `[Export]` by: * Define a Java interface & create a binding for it in C#, we already have `maui.aar` setup for this. * We can simply implement the interface in C# and remove `[Export]`. Lastly, I fixed some of the defaults in `Metadata.xml`, it didn't look like we were automatically making Java interfaces `internal`. It looks like we probably made `ImageLoaderCallback` public by mistake.
Description of Change
Before we get started: HUGE thank you to @rbrundritt for working on a key part of this for async JavaScript method support!
Enable C# code to invoke methods running in JS in the WebView. The methods can be invoked as async, have optional return values, and optional parameters. The parameters and return values are serialized using JSON.
Example
With some simple JavaScript code like this:
And you can invoke it from C# like this:
And then with a bit more complex async JavaScript code like this:
You can invoke it from C# like this:
Issues Fixed
Fixes #22303