-
Notifications
You must be signed in to change notification settings - Fork 2k
[Windows]Fix ContentPresenter Throws System.ArgumentException When Dynamically Assigning RefreshView or ScrollView Content #36430
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
Changes from 7 commits
c2b226d
832f997
49d188c
fad4715
0d42687
97a2789
d310443
7d8dc70
6aa094e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| [Issue(IssueTracker.Github, 36298, "[Windows] ContentPresenter throws ArgumentException when dynamically switching RefreshView or ScrollView content.", PlatformAffected.UWP)] | ||
| public class Issue36298 : ContentPage | ||
| { | ||
| ContentView _contentHolder; | ||
| View _view1; | ||
| View _view2; | ||
|
|
||
| public Issue36298() | ||
| { | ||
| _view1 = new ContentView | ||
| { | ||
| Content = new RefreshView | ||
| { | ||
| Content = new Label { Text = "View 1 - RefreshView" } | ||
| } | ||
| }; | ||
|
|
||
| _view2 = new ContentView | ||
| { | ||
| Content = new ScrollView | ||
| { | ||
| Content = new Label { Text = "View 2 - ScrollView" } | ||
| } | ||
| }; | ||
|
|
||
| _contentHolder = new ContentView | ||
| { | ||
| Content = _view1 | ||
| }; | ||
|
|
||
| var switchToView2Button = new Button | ||
| { | ||
| Text = "Switch to View 2", | ||
| AutomationId = "SwitchToView2" | ||
| }; | ||
| switchToView2Button.Clicked += (_, _) => _contentHolder.Content = _view2; | ||
|
|
||
| var switchToView1Button = new Button | ||
| { | ||
| Text = "Switch to View 1", | ||
| AutomationId = "SwitchToView1" | ||
| }; | ||
| switchToView1Button.Clicked += (_, _) => _contentHolder.Content = _view1; | ||
|
|
||
| var successLabel = new Label | ||
| { | ||
| Text = "Waiting", | ||
| AutomationId = "SuccessLabel" | ||
| }; | ||
|
|
||
| switchToView2Button.Clicked += (_, _) => successLabel.Text = "View2"; | ||
| switchToView1Button.Clicked += (_, _) => successLabel.Text = "Success"; | ||
|
|
||
| Content = new VerticalStackLayout | ||
| { | ||
| Spacing = 10, | ||
| Padding = new Thickness(20), | ||
| Children = | ||
| { | ||
| switchToView2Button, | ||
| switchToView1Button, | ||
| _contentHolder, | ||
| successLabel | ||
| } | ||
| }; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
|
|
||
| public class Issue36298 : _IssuesUITest | ||
| { | ||
| public Issue36298(TestDevice device) : base(device) { } | ||
|
|
||
| public override string Issue => "[Windows] ContentPresenter throws ArgumentException when dynamically switching RefreshView or ScrollView content."; | ||
|
|
||
| [Test] | ||
| [Category(UITestCategories.Layout)] | ||
| public void SwitchingContentPresenterContentShouldNotCrash() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
[major] Regression Prevention and Test Coverage — This PR changes both |
||
| { | ||
| // Wait for the page to load showing View 1 | ||
| App.WaitForElement("SwitchToView2"); | ||
|
|
||
| // Switch to View 2 | ||
| App.Tap("SwitchToView2"); | ||
| App.WaitForElement("SwitchToView1"); | ||
|
|
||
|
Comment on lines
+20
to
+23
|
||
| // Switch back to View 1 — this triggered the ArgumentException before the fix | ||
|
devanathan-vaithiyanathan marked this conversation as resolved.
|
||
| App.Tap("SwitchToView1"); | ||
|
|
||
| // Label is updated to "Success" only if the switch completed without crashing. | ||
| // WaitForElement("SuccessLabel") alone is insufficient because the label | ||
| // is present from page load; we must verify the text was actually updated. | ||
| App.WaitForElement("SuccessLabel"); | ||
| Assert.That(App.FindElement("SuccessLabel").GetText(), Is.EqualTo("Success")); | ||
|
devanathan-vaithiyanathan marked this conversation as resolved.
Outdated
|
||
| } | ||
| } | ||
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.
[major] Regression test coverage — The regression title and production fix target
ContentPresenter/IContentViewreparenting, but the sample host here is aContentView. That exercises a different setup than the reported ContentPresenter/templated-content scenario, and the direct ContentView swap can succeed without proving the ContentPresenter crash is fixed. Please make the repro use aContentPresenter(for example via a control template) and verify that it fails on the old handler code and passes with this fix.