-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Android] Fix the Setting Content of ContentView through style would crash on parent change #29931
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
/azp run MAUI-UITests-public |
Azure Pipelines successfully started running 1 pipeline(s). |
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 prevents IllegalStateException
crashes on Android (and aligns iOS behavior) when setting a ContentView
’s content via a style by detaching any existing parent before re-adding the view.
- Detach and remove old platform views before adding new ones in both
ContentViewHandler
andBorderHandler
for Android and iOS - Introduce a UI test in Shared.Tests and HostApp to reproduce and guard against regressions for Issue #11812
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/Core/src/Handlers/ContentView/ContentViewHandler.iOS.cs | Call RemoveFromSuperview before AddSubview |
src/Core/src/Handlers/ContentView/ContentViewHandler.Android.cs | Call RemoveFromParent before AddView |
src/Core/src/Handlers/Border/BorderHandler.iOS.cs | Call RemoveFromSuperview before adding scroll or content |
src/Core/src/Handlers/Border/BorderHandler.Android.cs | Call RemoveFromParent before AddView |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue11812.cs | Add a UI test tapping the content-change button |
src/Controls/tests/TestCases.HostApp/Issues/Issue11812.cs | Host app scenario wiring up the styled ContentView change button |
Comments suppressed due to low confidence (3)
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue11812.cs:1
- The conditional compilation symbol TEST_FAILS_ON_WINDOWS may prevent this test from running on Android/iOS. Consider using a platform-specific symbol (e.g., WINDOWS) or inverting the condition to exclude only Windows builds, ensuring the test executes on supported platforms.
#if TEST_FAILS_ON_WINDOWS // test fails on windows , see https://github.com/dotnet/maui/issues/29930
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue11812.cs:15
- [nitpick] The test category is set to UITestCategories.Border, which doesn't match ContentView behavior. Consider creating or using a more appropriate category (e.g., UITestCategories.ContentView or UITestCategories.Issues).
[Category(UITestCategories.Border)]
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue11812.cs:18
- The test currently only checks that tapping the button does not crash. To fully validate the fix, add an assertion such as
App.WaitForElement("SubContent")
to confirm the styled child label appears after the dynamic content change.
App.WaitForElement("ChangeInnerContent");
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 of the issue
IllegalStateException
on Android when the view is added to a new parent without being removed from the old one.Description of Change
IllegalStateException
.Issues Fixed
Fixes #11812
Windows Fix PR : #30047
Tested the behaviour in the following platforms
Screenshot
ExceptionOfContentView.mov
FixedContentViewissue.mov