[Android] Fix custom WebViewClient being overridden by MAUI handler mapper - #34426
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34426Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34426" |
9074806 to
2486d91
Compare
|
@kubaflo , Addressed Test case failures. Please let me know if you have any concerns. |
There was a problem hiding this comment.
Pull request overview
Fixes an Android handler lifecycle issue in .NET MAUI where WebViewHandler’s property mapper would overwrite a custom WebViewClient set by apps in ConnectHandler, preventing reliable ShouldOverrideUrlLoading interception.
Changes:
- Remove
MapWebViewClient/MapWebChromeClientfrom the Android property mapper and initialize default clients once during platform view creation. - Update Android
DisconnectHandlerto disconnect/dispose stored client instances instead of casting from the platform view. - Add a new HostApp repro page + UITest coverage for issue #34392 to validate custom
WebViewClientbehavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txt | Records removal of Android WebViewHandler mapper APIs (but currently conflicts with shipped API state). |
| src/Core/src/Handlers/WebView/WebViewHandler.cs | Removes Android mapper entries that were reassigning clients after ConnectHandler. |
| src/Core/src/Handlers/WebView/WebViewHandler.Android.cs | Creates default MauiWebViewClient/MauiWebChromeClient in CreatePlatformView() and adjusts disconnect/disposal behavior. |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34392.cs | Adds an Android-only UITest to validate ShouldOverrideUrlLoading gets called. |
| src/Controls/tests/TestCases.HostApp/MauiProgram.cs | Registers the custom handler used for the repro/test. |
| src/Controls/tests/TestCases.HostApp/Issues/Issue34392.cs | Adds the HostApp issue page + custom WebView/handler/client used by the test. |
Comments suppressed due to low confidence (1)
src/Core/src/Handlers/WebView/WebViewHandler.Android.cs:97
- The PR removes
MapWebViewClient/MapWebChromeClientmethods, but these APIs are already listed insrc/Core/src/PublicAPI/net-android/PublicAPI.Shipped.txt. Removing shipped public APIs is a breaking change and will also cause PublicAPI analyzer failures unless handled as a shipped removal. Prefer keeping these methods (even if they’re no longer used by the mapper) and/or marking them obsolete, rather than deleting them outright.
public static void MapSource(IWebViewHandler handler, IWebView webView)
{
ProcessSourceWhenReady(handler, webView);
}
public static void MapUserAgent(IWebViewHandler handler, IWebView webView)
{
handler.PlatformView.UpdateUserAgent(webView);
}
public static void MapWebViewSettings(IWebViewHandler handler, IWebView webView)
{
handler.PlatformView.UpdateSettings(webView, true, true);
}
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check copilot's suggestions and the ai's summary?
|
@kubaflo , Addressed AI summary. |
kubaflo
left a comment
There was a problem hiding this comment.
Could you please resolve conflicts?
596bd28 to
6e9af5b
Compare
|
@kubaflo , Rebased and resolved the conflicts. Please let me know if you have any concern. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Tests Failure Analysis
Test Failure Review: Insufficient data - click to expandOverall verdict: Insufficient data — all three PR pipeline builds (
Coverage: 138 checks · 126 passing · 12 failing · 0 pending · 11 inaccessible · 1 unmapped · 0 unexplained build legs · 0 unaccounted failing checks · 2 aborted failing checks · 0 canceled-build checks · 4 device-test unverified · 0 unattributed · 0 regressed-vs-base. Deterministic ceiling: Insufficient data — 11 failing check(s) could not be inspected (AzDO build/logs inaccessible). Builds (this PR): maui-pr 1494709 · maui-pr-devicetests 1494711 · maui-pr-uitests 1494710. Base sampling (net11.0, 0 recent builds): none available. Recommended actionA human should re-check the AzDO builds once they are accessible (the 404s suggest transient/permission issues); re-run |
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the suggestions?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
ℹ️ The review agent did not produce a full summary on this run (an infrastructure issue on the CI agent), but the deep UI tests completed — their results are below. Re-comment
/reviewfor a fresh full review.
drop-deep-uitests artifact before concluding.
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
ViewBaseTests |
118/119 ✓ | — |
WebView |
49/50 (app crashed; 1 couldn't complete) | — |
⚠️ WebView — app crashed mid-run; 1 test could not complete
The HostApp crashed during this category (a test failed in TearDown with investigate as possible crash); every following OneTimeSetUp then timed out waiting for the test gallery to reappear. This can be an emulator/infrastructure flake OR a regression introduced by this PR — review the screenshots and logcat in the ui-diagnostics folder of the artifact before concluding.
The app was expected to be running still, investigate as possible crash
TearDown : The app was expected to be running still, investigate as possible crash
at UITest.Appium.NUnit.UITestBase.UITestBaseTearDown() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 159
at UITest.Appium.NUnit.UITestBase.TestTearDown() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 45
--TearDown
at UITest.Appium.NUnit.UITestBase.UITestBaseTearDown() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 159
at UITest.Appium.NUnit.UITestBase.TestTearDown() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 45
1) at UITest.Appium.NUnit.UITestBase.UITestBaseTearDown() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 159
at UITest.Appium.NUnit.UITestBase.TestTearDown() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 45
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Root Cause
The MAUI handler lifecycle calls
ConnectHandlerbefore the property mappers run. When a developer sets a customWebViewClientinConnectHandler, the subsequent execution ofMapWebViewClient(triggered by the mapper pipeline) would callplatformView.SetWebViewClient(new MauiWebViewClient(...)), silently discarding the custom client. SinceMapWebViewClientandMapWebChromeClientserve no purpose beyond initial setup — they are not responding to any virtual view property change — registering them in the mapper was incorrect. Moving client creation toCreatePlatformView()and removing them from the mapper pipeline ensures the default clients are set once, beforeConnectHandlerruns, allowing developers to override them reliably.Description of Change
On Android,
WebViewHandlerregisteredMapWebViewClientandMapWebChromeClientas property mapper entries. These mappers created newMauiWebViewClient/MauiWebChromeClientinstances and set them on the platform view every time they ran — afterConnectHandlerwas called. This meant any customWebViewClientset by users inConnectHandler(e.g., to intercept navigation viaShouldOverrideUrlLoading) would be silently replaced by MAUI's default client.Changes:
MapWebViewClientandMapWebChromeClientmapper methods entirelyCreatePlatformView()and stored as private fields (_webViewClient,_webChromeClient)DisconnectHandleruses the stored field references directly forDisconnect()andDispose(), rather than casting from the platform viewWebViewClientinConnectHandlerwithout MAUI replacing itIssues Fixed
Fixes #34392