Blazor Windows Open Links in Browser with Configurability #4680
Conversation
Windows portion of #4338
| var uri = new Uri(args.Uri); | ||
| if (uri.Host != "0.0.0.0" && _externalLinkMode == ExternalLinkMode.OpenInExternalBrowser) | ||
| { | ||
| _ = Launcher.LaunchUriAsync(uri); |
There was a problem hiding this comment.
Should we log errors thrown by these tasks or do they appear somewhere?
There was a problem hiding this comment.
Just searched through, there doesn't seem to be an existing logging mechanism. Is there a standardized mechanism we could use here (or perhaps #4441 would cover it?).
| : base(webview, services, dispatcher, fileProvider, jsComponents, hostPageRelativePath) | ||
| { | ||
| _webview = webview; | ||
| _hostPageRelativePath = hostPageRelativePath; | ||
| _contentRootDir = contentRootDir; | ||
| _externalLinkMode = externalLinkMode; | ||
|
|
||
| _webview.CoreWebView2Initialized += Webview_CoreWebView2Initialized; |
There was a problem hiding this comment.
Does this need to unsubscribed when this type is disposed?
There was a problem hiding this comment.
I was under the impression the lifecycle of the WinUIWebViewManager and the associated WebView2Control would be essentially the same, hence I didn't bother with unsubscribing (given if they get disposed at the same time then there shouldn't be any stray references preventing GC). This is in line with the current async lambda we have for Blazor.Start():
maui/src/BlazorWebView/src/Maui/Windows/WinUIWebViewManager.cs
Lines 98 to 103 in b00586c
I can add it in if you think it'd be beneficial.
There was a problem hiding this comment.
Alternatively, instead of hooking the Initialized event at all, you can wait until we force initialization and then hook the events you actually want directly. Here's where we force initialization in the shared code: https://github.com/dotnet/maui/blob/main/src/BlazorWebView/src/SharedSource/WebView2WebViewManager.cs#L93
Because presumably we want exactly this logic on WinForms and WPF as well. Launching the browser will be different between WinUI and non-WinUI, but that could be abstracted via a delegate or something.
| : base(webview, services, dispatcher, fileProvider, jsComponents, hostPageRelativePath) | ||
| { | ||
| _webview = webview; | ||
| _hostPageRelativePath = hostPageRelativePath; | ||
| _contentRootDir = contentRootDir; | ||
| _externalLinkMode = externalLinkMode; | ||
|
|
||
| _webview.CoreWebView2Initialized += Webview_CoreWebView2Initialized; |
There was a problem hiding this comment.
Alternatively, instead of hooking the Initialized event at all, you can wait until we force initialization and then hook the events you actually want directly. Here's where we force initialization in the shared code: https://github.com/dotnet/maui/blob/main/src/BlazorWebView/src/SharedSource/WebView2WebViewManager.cs#L93
Because presumably we want exactly this logic on WinForms and WPF as well. Launching the browser will be different between WinUI and non-WinUI, but that could be abstracted via a delegate or something.
|
Thanks for the feedback, merged into #4645 and will address feedback there. |
* Blazor Android Open Links in Browser with Configurability * Blazor Windows Open Links in Browser with Configurability (#4680) * Blazor Windows Open Links in Browser with Configurability Windows portion of #4338 * TryCreate URI * PR Feedback (cherry picked from commit 35f637e) * OnExternalNavigationStarting * Pranav Points * Event based approach * Info -> EventArgs * iOS & Mac Catalyst * Fix WPF/Winforms Browser Start * Winforms ExternalNavigationStarting * PR Feedback * Remove ordering dependency during property mapping * @blowdart feedback * @Eilon feedback
Windows portion of #4338
Merges into: #4645