-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fixed the crash on iOS when setting HeightRequest on WebView inside a ScrollView with IsVisible set to false #29022
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| var width = widthConstraint; | ||
| var height = heightConstraint; | ||
| var width = size.Width; |
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.
With this changes, the next conditions like:
if (size.Width == 0)
can use directly the width variable.
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 for the feedback, I have modified the changes @jsuarezruiz
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 29022Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 29022" |
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.
Pull request overview
This PR addresses a crash on iOS when a WebView with HeightRequest is placed inside an invisible ScrollView. The fix modifies the GetDesiredSize method in WebViewHandler.iOS.cs to use the computed size dimensions instead of the constraint parameters directly, preventing infinite values from causing layout exceptions.
- Fixes iOS crash when WebView with HeightRequest is inside invisible ScrollView
- Updates GetDesiredSize to use size.Width/Height instead of widthConstraint/heightConstraint
- Adds UI test to prevent regression
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Core/src/Handlers/WebView/WebViewHandler.iOS.cs | Modified GetDesiredSize to use size dimensions instead of constraints to prevent infinite value propagation |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue26795.cs | Added NUnit UI test to verify WebView doesn't crash with HeightRequest in invisible ScrollView |
| src/Controls/tests/TestCases.HostApp/Issues/Issue26795.cs | Added test page reproducing the crash scenario with WebView inside invisible ScrollView |
Comments suppressed due to low confidence (1)
src/Core/src/Handlers/WebView/WebViewHandler.iOS.cs:126
- The logic appears incomplete. When
widthorheightis 0 and the constraint is valid and finite (not infinite, not <= 0), the code should fall back to using the constraint value.
Currently, if size.Width is 0 but widthConstraint is a valid finite value like 300, the code will leave width as 0 instead of using the constraint. This could cause incorrect layout calculations.
Consider adding an else clause to handle the case where constraints are valid:
if (width == 0)
{
if (widthConstraint <= 0 || double.IsInfinity(widthConstraint))
{
width = MinimumSize;
set = true;
}
else
{
width = widthConstraint;
set = true;
}
}And similarly for height.
if (width == 0)
{
if (widthConstraint <= 0 || double.IsInfinity(widthConstraint))
{
width = MinimumSize;
set = true;
}
}
if (height == 0)
{
if (heightConstraint <= 0 || double.IsInfinity(heightConstraint))
{
height = MinimumSize;
set = true;
}
}
| public void WebViewShouldNotCrashWithHeightRequest() | ||
| { | ||
| App.WaitForElement("Label"); | ||
| } |
Copilot
AI
Dec 9, 2025
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.
The test only verifies that the app doesn't crash, but doesn't validate the actual layout behavior. Consider adding assertions to verify:
- The WebView renders with the correct dimensions when made visible
- The HeightRequest is respected
- The layout works correctly when the ScrollView's IsVisible property is toggled
Example:
App.Tap("ToggleVisibilityButton");
App.WaitForElement("WebViewId");
var webViewRect = App.FindElement("WebViewId").GetRect();
Assert.That(webViewRect.Height, Is.GreaterThan(0));This would provide more comprehensive coverage of the layout fix beyond just crash prevention.
| var webView = new WebView | ||
| { | ||
| Source = "https://en.m.wikipedia.org/wiki", | ||
| HeightRequest = 1000, | ||
| }; |
Copilot
AI
Dec 9, 2025
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.
Consider adding an AutomationId to the WebView to enable more comprehensive UI testing of the layout behavior:
var webView = new WebView
{
Source = "https://en.m.wikipedia.org/wiki",
HeightRequest = 1000,
AutomationId = "TestWebView"
};This would allow the test to verify that the WebView is properly sized and positioned, not just that the app doesn't crash.
| var scrollView = new ScrollView | ||
| { | ||
| IsVisible = false, | ||
| Content = webView | ||
| }; |
Copilot
AI
Dec 9, 2025
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.
Consider adding a button to toggle the ScrollView's visibility to enable more comprehensive testing:
var toggleButton = new Button
{
Text = "Toggle Visibility",
AutomationId = "ToggleVisibilityButton"
};
toggleButton.Clicked += (s, e) => scrollView.IsVisible = !scrollView.IsVisible;
layout.Children.Add(toggleButton);This would allow the test to verify that the WebView layout works correctly both when initially invisible and when made visible.
Root Cause of the issue
WebViewis placed inside a ScrollView, the height constraint can become infinite. The original code was using constraints directly, which could propagate infinite values. This caused layout issues and potentialNaNexceptions.Description of Change
infinitecase for WebView in GetDesiredSize by usingsize.Widthandsize.Heightinstead of relying on potentially infinite constraints.Regressed PR
Issues Fixed
Fixes #26795
Tested the behaviour in the following platforms
Screenshot
WebViewCrash.mov
WebViewFix.mov