Fix: Enable VisualStateManager to set Style property dynamically#33389
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug where VisualStateManager permanently breaks when attempting to set a control's Style property during state transitions. The fix adds special handling for Style values in Setters by using IStyle.Apply() and IStyle.UnApply() methods instead of directly setting/clearing the Style property, which preserves the VSM attachment.
Changes:
- Modified
Setter.Apply()to detect Style values and callIStyle.Apply()instead ofSetValue() - Modified
Setter.UnApply()to detect Style values and callIStyle.UnApply()before clearing the property - Added comprehensive UI test with XAML page demonstrating VSM Style transitions and NUnit test validating the fix
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Controls/src/Core/Setter.cs | Added special handling for Style values in Apply/UnApply methods to preserve VSM functionality |
| src/Controls/tests/TestCases.HostApp/Issues/Issue17175.xaml | XAML test page with Button using VSM to switch between different Style resources |
| src/Controls/tests/TestCases.HostApp/Issues/Issue17175.xaml.cs | Code-behind for test page with button click handler to trigger state change |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue17175.cs | NUnit UI test verifying VSM can successfully apply Style and update Button text |
|
@StephaneDelcroix could you please have a look :) |
StephaneDelcroix
left a comment
There was a problem hiding this comment.
Multi-Model Consensus Review (5 models: claude-opus-4.6 ×2, claude-sonnet-4.6, gemini-3-pro-preview, gpt-5.3-codex)
CI Status: ✅ All 23 checks passing
Consensus Findings (2+ models agreed)
🔴 CRITICAL Setter.cs:~83 + Style.cs:~120 -- ConditionalWeakTable specificity overwrite. The Style class maintains two ConditionalWeakTable<BindableObject, object> tables (_targets and specificities), each with a single slot per (Style, BindableObject) pair. If the same Style instance is used as both the XAML Style= attribute AND inside a VSM state setter, the second IStyle.Apply call silently overwrites the first specificity entry. Downstream effects:
BasedOnChanged/OnBasedOnResourceChangediterate_targets; after the first VSM deactivation removes the target, futureDynamicResourceupdates stop propagating.- When the user later changes
button.Style,UnApplyCorereads the stale VSM specificity and clears the wrong level, leaving original XAML-set property values permanently stuck.
🟡 MODERATE Issue17175.cs -- Test only covers one-way transition. The test transitions Normal → Disabled once but does not verify: (a) transition back to Normal, (b) a second cycle (re-disable). A regression in UnApply cleanup would be invisible to this test.
🟢 MINOR Setter.cs:~117 -- ClearValue(StyleProperty, specificity) in UnApply is asymmetric with Apply (which never calls SetValue(StyleProperty, specificity)). In practice a no-op but triggers unnecessary OnBindablePropertySet with changed=false.
Test Coverage Gaps
- Round-trip state transition (Normal → Disabled → Normal) not tested
- Same
Styleinstance used in both XAML attribute and VSM setter not tested DynamicResourcestyle update after VSM transition not tested
Verdict: ⚠️ Request Changes
The core mechanism is correct and solves the stated issue. Specific asks:
- Address CWT overwrite -- either guard against dual-apply of the same Style instance or document as a known limitation
- Extend test to verify Disabled → Normal restoration and a second disable cycle
- Consider early-returning in the
UnApplyStyle branch instead of callingClearValueat a specificity that was never set
kubaflo
left a comment
There was a problem hiding this comment.
Could you please review Stephane's review?
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 33389Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 33389" |
|
@StephaneDelcroix, @kubaflo I investigated all three concerns. |
MauiBot
left a comment
There was a problem hiding this comment.
🤖 Automated review — alternative fix proposed
The expert-reviewer evaluation compared the PR fix against #4 automatically generated candidates and selected try-fix-4 as the strongest fix.
Why: try-fix-4 addresses the code review's primary ❌ error (guard too broad) by narrowing the Value is Style check to Property == StyleableElement.StyleProperty || Property == Span.StyleProperty, preventing the bypass from activating on unrelated Style-typed BindableProperties. It preserves the PR's proven IStyle.Apply/UnApply mechanism with minimal code delta (Setter.cs only, 8 added lines) and passes all iOS UI tests.
Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.
Candidate diff (`try-fix-4`)
diff --git a/src/Controls/src/Core/Setter.cs b/src/Controls/src/Core/Setter.cs
index da6c340b52..feacd1bff1 100644
--- a/src/Controls/src/Core/Setter.cs
+++ b/src/Controls/src/Core/Setter.cs
@@ -86,6 +86,10 @@ namespace Microsoft.Maui.Controls
targetObject.SetDynamicResource(Property, dynamicResource.Key, specificity);
else if (Value is IList<VisualStateGroup> visualStateGroupCollection)
targetObject.SetValue(Property, visualStateGroupCollection.Clone(), specificity);
+ else if (Value is Style style && (Property == StyleableElement.StyleProperty || Property == Span.StyleProperty))
+ // When a VSM setter targets StyleProperty, apply the style's inner setters at the VSM
+ // specificity directly. MergedStyle.SetStyle hardcodes StyleLocal, which loses VSM priority.
+ ((IStyle)style).Apply(targetObject, specificity);
else
targetObject.SetValue(Property, Value, specificity: specificity);
}
@@ -106,6 +110,14 @@ namespace Microsoft.Maui.Controls
targetObject.RemoveBinding(Property, specificity);
else if (Value is DynamicResource dynamicResource)
targetObject.RemoveDynamicResource(Property, specificity);
+ else if (Value is Style style && (Property == StyleableElement.StyleProperty || Property == Span.StyleProperty))
+ {
+ // Mirror the Apply side: clean up style setters applied with VSM specificity.
+ // Early return prevents ClearValue(StyleProperty, specificity) from accidentally
+ // triggering MergedStyle.SetStyle (SetValue was never called in Apply for this path).
+ ((IStyle)style).UnApply(targetObject);
+ return;
+ }
targetObject.ClearValue(Property, specificity);
}
kubaflo
left a comment
There was a problem hiding this comment.
Could you please review the ai's suggestions?
@kubaflo, I addressed the AI concerns |
🤖 AI Summary
📊 Review Session —
|
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🖥️ Issue17175 Issue17175 |
✅ FAIL — 1069s | ✅ PASS — 813s |
🔴 Without fix — 🖥️ Issue17175: FAIL ✅ · 1069s
(truncated to last 15,000 chars)
ls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0-android36.0/Microsoft.Maui.Maps.dll
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0-android36.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Xaml.dll
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Maps.dll
Microsoft.AspNetCore.Components.WebView.Maui -> /home/vsts/work/1/s/artifacts/bin/Microsoft.AspNetCore.Components.WebView.Maui/Debug/net10.0-android36.0/Microsoft.AspNetCore.Components.WebView.Maui.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Controls.Foldable -> /home/vsts/work/1/s/artifacts/bin/Controls.Foldable/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Foldable.dll
Controls.TestCases.HostApp -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Controls.TestCases.HostApp.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Graphics -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Essentials -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Core -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Maps.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Microsoft.AspNetCore.Components.WebView.Maui -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Maps.dll
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Xaml.dll
Controls.Foldable -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Foldable.dll
Build succeeded.
0 Warning(s)
0 Error(s)
Time Elapsed 00:09:44.88
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Determining projects to restore...
Restored /home/vsts/work/1/s/src/Controls/tests/CustomAttributes/Controls.CustomAttributes.csproj (in 1.6 sec).
Restored /home/vsts/work/1/s/src/TestUtils/src/VisualTestUtils/VisualTestUtils.csproj (in 13 ms).
Restored /home/vsts/work/1/s/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj (in 9.92 sec).
Restored /home/vsts/work/1/s/src/Controls/tests/TestCases.Android.Tests/Controls.TestCases.Android.Tests.csproj (in 11.79 sec).
Restored /home/vsts/work/1/s/src/TestUtils/src/UITest.Core/UITest.Core.csproj (in 2 ms).
Restored /home/vsts/work/1/s/src/TestUtils/src/UITest.Appium/UITest.Appium.csproj (in 3 ms).
Restored /home/vsts/work/1/s/src/TestUtils/src/UITest.NUnit/UITest.NUnit.csproj (in 499 ms).
Restored /home/vsts/work/1/s/src/TestUtils/src/UITest.Analyzers/UITest.Analyzers.csproj (in 4.11 sec).
5 of 13 projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Controls.CustomAttributes -> /home/vsts/work/1/s/artifacts/bin/Controls.CustomAttributes/Debug/net10.0/Controls.CustomAttributes.dll
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
UITest.Core -> /home/vsts/work/1/s/artifacts/bin/UITest.Core/Debug/net10.0/UITest.Core.dll
VisualTestUtils -> /home/vsts/work/1/s/artifacts/bin/VisualTestUtils/Debug/netstandard2.0/VisualTestUtils.dll
VisualTestUtils.MagickNet -> /home/vsts/work/1/s/artifacts/bin/VisualTestUtils.MagickNet/Debug/netstandard2.0/VisualTestUtils.MagickNet.dll
UITest.NUnit -> /home/vsts/work/1/s/artifacts/bin/UITest.NUnit/Debug/net10.0/UITest.NUnit.dll
UITest.Appium -> /home/vsts/work/1/s/artifacts/bin/UITest.Appium/Debug/net10.0/UITest.Appium.dll
UITest.Analyzers -> /home/vsts/work/1/s/artifacts/bin/UITest.Analyzers/Debug/netstandard2.0/UITest.Analyzers.dll
Controls.TestCases.Android.Tests -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
/home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.24] Discovering: Controls.TestCases.Android.Tests
[xUnit.net 00:00:00.74] Discovered: Controls.TestCases.Android.Tests
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
NUnit3TestExecutor discovered 1 of 1 NUnit test cases using Current Discovery mode, Non-Explicit run
�[38;5;211m[ddcddcc8]�[0m�[38;5;160m[Logcat]�[0m Logcat terminated with code 1, signal null
�[38;5;156m[4a28fa77]�[0m�[38;5;160m[Logcat]�[0m Logcat terminated with code 1, signal null
�[38;5;111m[ec01be3f]�[0m�[38;5;160m[Logcat]�[0m Logcat terminated with code 1, signal null
�[38;5;47m[8e939137]�[0m�[38;5;160m[Logcat]�[0m Logcat terminated with code 1, signal null
�[38;5;179m[baf5515b]�[0m�[38;5;160m[Logcat]�[0m Logcat terminated with code 1, signal null
�[38;5;132m[2bb347e5]�[0m�[38;5;160m[Logcat]�[0m Logcat terminated with code 1, signal null
�[38;5;181m[1cd4f92b]�[0m�[38;5;160m[Logcat]�[0m Logcat terminated with code 1, signal null
�[38;5;52m[478ede6e]�[0m�[38;5;160m[Logcat]�[0m Logcat terminated with code 1, signal null
�[38;5;25m[52dc37ed]�[0m�[38;5;160m[Logcat]�[0m Logcat terminated with code 1, signal null
�[38;5;138m[f6c61abe]�[0m�[38;5;160m[Logcat]�[0m Logcat terminated with code 1, signal null
>>>>> 05/05/2026 13:19:52 The SaveDeviceDiagnosticInfo threw an exception during Issue17175(Android).
Exception details: System.InvalidOperationException: Call InitialSetup before accessing the App property.
at UITest.Appium.NUnit.UITestContextBase.get_App() in /_/src/TestUtils/src/UITest.NUnit/UITestContextBase.cs:line 32
at UITest.Appium.NUnit.UITestBase.SaveDeviceDiagnosticInfo(String note, Boolean storeForReattachment) in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 255
TearDown failed for test fixture Microsoft.Maui.TestCases.Tests.Issues.Issue17175(Android)
OpenQA.Selenium.UnknownErrorException : An unknown server-side error occurred while processing the command. Original error: Error executing adbExec. Original error: 'Command '/usr/local/lib/android/sdk/platform-tools/adb -P 5037 -s emulator-5554 install -r --no-incremental /home/vsts/work/1/s/.appium/node_modules/appium-uiautomator2-driver/node_modules/appium-uiautomator2-server/apks/appium-uiautomator2-server-v7.4.1.apk' timed out after 20000ms'. Try to increase the 20000ms adb execution timeout represented by 'uiautomator2ServerInstallTimeout' capability
TearDown : System.InvalidOperationException : Call InitialSetup before accessing the App property.
StackTrace: at OpenQA.Selenium.WebDriver.UnpackAndThrowOnError(Response errorResponse, String commandToExecute)
at OpenQA.Selenium.WebDriver.ExecuteAsync(String driverCommandToExecute, Dictionary`2 parameters)
at OpenQA.Selenium.WebDriver.Execute(String driverCommandToExecute, Dictionary`2 parameters)
at OpenQA.Selenium.Appium.AppiumDriver.Execute(String driverCommandToExecute, Dictionary`2 parameters)
at OpenQA.Selenium.WebDriver.StartSession(ICapabilities capabilities)
at OpenQA.Selenium.WebDriver..ctor(ICommandExecutor executor, ICapabilities capabilities)
at OpenQA.Selenium.Appium.AppiumDriver..ctor(ICommandExecutor commandExecutor, ICapabilities appiumOptions)
at OpenQA.Selenium.Appium.AppiumDriver..ctor(Uri remoteAddress, ICapabilities appiumOptions, TimeSpan commandTimeout, AppiumClientConfig clientConfig)
at OpenQA.Selenium.Appium.AppiumDriver..ctor(Uri remoteAddress, ICapabilities appiumOptions, TimeSpan commandTimeout)
at OpenQA.Selenium.Appium.AppiumDriver..ctor(Uri remoteAddress, ICapabilities appiumOptions)
at OpenQA.Selenium.Appium.Android.AndroidDriver..ctor(Uri remoteAddress, DriverOptions driverOptions)
at UITest.Appium.AppiumAndroidApp..ctor(Uri remoteAddress, IConfig config) in /_/src/TestUtils/src/UITest.Appium/AppiumAndroidApp.cs:line 11
at UITest.Appium.AppiumAndroidApp.CreateAndroidApp(Uri remoteAddress, IConfig config) in /_/src/TestUtils/src/UITest.Appium/AppiumAndroidApp.cs:line 41
at UITest.Appium.AppiumServerContext.CreateUIClientContext(IConfig config) in /_/src/TestUtils/src/UITest.Appium/AppiumServerContext.cs:line 42
at UITest.Appium.NUnit.UITestContextBase.InitialSetup(IServerContext context, Boolean reset) in /_/src/TestUtils/src/UITest.NUnit/UITestContextBase.cs:line 77
at UITest.Appium.NUnit.UITestContextBase.InitialSetup(IServerContext context) in /_/src/TestUtils/src/UITest.NUnit/UITestContextBase.cs:line 55
at UITest.Appium.NUnit.UITestBase.OneTimeSetup() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 215
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
--TearDown
at UITest.Appium.NUnit.UITestContextBase.get_App() in /_/src/TestUtils/src/UITest.NUnit/UITestContextBase.cs:line 32
at UITest.Appium.NUnit.UITestBase.OneTimeTearDown() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 244
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
NUnit Adapter 4.5.0.0: Test execution complete
Failed VisualStateManagerCanSetStyleProperty [4 m 29 s]
Error Message:
OneTimeSetUp: OpenQA.Selenium.UnknownErrorException : An unknown server-side error occurred while processing the command. Original error: Error executing adbExec. Original error: 'Command '/usr/local/lib/android/sdk/platform-tools/adb -P 5037 -s emulator-5554 install -r --no-incremental /home/vsts/work/1/s/.appium/node_modules/appium-uiautomator2-driver/node_modules/appium-uiautomator2-server/apks/appium-uiautomator2-server-v7.4.1.apk' timed out after 20000ms'. Try to increase the 20000ms adb execution timeout represented by 'uiautomator2ServerInstallTimeout' capability
Stack Trace:
at OpenQA.Selenium.WebDriver.UnpackAndThrowOnError(Response errorResponse, String commandToExecute)
at OpenQA.Selenium.WebDriver.ExecuteAsync(String driverCommandToExecute, Dictionary`2 parameters)
at OpenQA.Selenium.WebDriver.Execute(String driverCommandToExecute, Dictionary`2 parameters)
at OpenQA.Selenium.Appium.AppiumDriver.Execute(String driverCommandToExecute, Dictionary`2 parameters)
at OpenQA.Selenium.WebDriver.StartSession(ICapabilities capabilities)
at OpenQA.Selenium.WebDriver..ctor(ICommandExecutor executor, ICapabilities capabilities)
at OpenQA.Selenium.Appium.AppiumDriver..ctor(ICommandExecutor commandExecutor, ICapabilities appiumOptions)
at OpenQA.Selenium.Appium.AppiumDriver..ctor(Uri remoteAddress, ICapabilities appiumOptions, TimeSpan commandTimeout, AppiumClientConfig clientConfig)
at OpenQA.Selenium.Appium.AppiumDriver..ctor(Uri remoteAddress, ICapabilities appiumOptions, TimeSpan commandTimeout)
at OpenQA.Selenium.Appium.AppiumDriver..ctor(Uri remoteAddress, ICapabilities appiumOptions)
at OpenQA.Selenium.Appium.Android.AndroidDriver..ctor(Uri remoteAddress, DriverOptions driverOptions)
at UITest.Appium.AppiumAndroidApp..ctor(Uri remoteAddress, IConfig config) in /_/src/TestUtils/src/UITest.Appium/AppiumAndroidApp.cs:line 11
at UITest.Appium.AppiumAndroidApp.CreateAndroidApp(Uri remoteAddress, IConfig config) in /_/src/TestUtils/src/UITest.Appium/AppiumAndroidApp.cs:line 41
at UITest.Appium.AppiumServerContext.CreateUIClientContext(IConfig config) in /_/src/TestUtils/src/UITest.Appium/AppiumServerContext.cs:line 42
at UITest.Appium.NUnit.UITestContextBase.InitialSetup(IServerContext context, Boolean reset) in /_/src/TestUtils/src/UITest.NUnit/UITestContextBase.cs:line 77
at UITest.Appium.NUnit.UITestContextBase.InitialSetup(IServerContext context) in /_/src/TestUtils/src/UITest.NUnit/UITestContextBase.cs:line 55
at UITest.Appium.NUnit.UITestBase.OneTimeSetup() in /_/src/TestUtils/src/UITest.NUnit/UITestBase.cs:line 215
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
Total tests: 1
Failed: 1
Test Run Failed.
Total time: 4.6921 Minutes
🟢 With fix — 🖥️ Issue17175: PASS ✅ · 813s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0-android36.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0-android36.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0-android36.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0-android36.0/Microsoft.Maui.Maps.dll
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0-android36.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Microsoft.AspNetCore.Components.WebView.Maui -> /home/vsts/work/1/s/artifacts/bin/Microsoft.AspNetCore.Components.WebView.Maui/Debug/net10.0-android36.0/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Foldable -> /home/vsts/work/1/s/artifacts/bin/Controls.Foldable/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Foldable.dll
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Xaml.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Maps.dll
Controls.TestCases.HostApp -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Controls.TestCases.HostApp.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Graphics -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Essentials -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Core -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Maps.dll
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Maps.dll
Microsoft.AspNetCore.Components.WebView.Maui -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Foldable -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Foldable.dll
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Xaml.dll
Build succeeded.
0 Warning(s)
0 Error(s)
Time Elapsed 00:10:34.07
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
Controls.CustomAttributes -> /home/vsts/work/1/s/artifacts/bin/Controls.CustomAttributes/Debug/net10.0/Controls.CustomAttributes.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.70-ci+azdo.14016908
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
UITest.Core -> /home/vsts/work/1/s/artifacts/bin/UITest.Core/Debug/net10.0/UITest.Core.dll
VisualTestUtils -> /home/vsts/work/1/s/artifacts/bin/VisualTestUtils/Debug/netstandard2.0/VisualTestUtils.dll
UITest.NUnit -> /home/vsts/work/1/s/artifacts/bin/UITest.NUnit/Debug/net10.0/UITest.NUnit.dll
VisualTestUtils.MagickNet -> /home/vsts/work/1/s/artifacts/bin/VisualTestUtils.MagickNet/Debug/netstandard2.0/VisualTestUtils.MagickNet.dll
UITest.Appium -> /home/vsts/work/1/s/artifacts/bin/UITest.Appium/Debug/net10.0/UITest.Appium.dll
UITest.Analyzers -> /home/vsts/work/1/s/artifacts/bin/UITest.Analyzers/Debug/netstandard2.0/UITest.Analyzers.dll
Controls.TestCases.Android.Tests -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
/home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.12] Discovering: Controls.TestCases.Android.Tests
[xUnit.net 00:00:00.49] Discovered: Controls.TestCases.Android.Tests
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
NUnit3TestExecutor discovered 1 of 1 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 05/05/2026 13:33:09 FixtureSetup for Issue17175(Android)
>>>>> 05/05/2026 13:33:13 VisualStateManagerCanSetStyleProperty Start
>>>>> 05/05/2026 13:33:18 VisualStateManagerCanSetStyleProperty Stop
Passed VisualStateManagerCanSetStyleProperty [4 s]
NUnit Adapter 4.5.0.0: Test execution complete
Test Run Successful.
Total tests: 1
Passed: 1
Total time: 28.9487 Seconds
📁 Fix files reverted (2 files)
eng/pipelines/ci-copilot.ymlsrc/Controls/src/Core/Setter.cs
🧪 UI Tests — Category Detection
Full UI test matrix will run (no specific categories detected from PR changes).
🔍 Regression Cross-Reference
🔍 Regression Cross-Reference
🟢 No regression risks detected. No labeled bug-fix PRs in the last 6 months touched the modified files.
🔍 Pre-Flight — Context & Validation
Issue: #17175 - Setting the Style property using the VisualStateManager within a Style resource does not work
PR: #33389 - Fix: Enable VisualStateManager to set Style property dynamically
Platforms Affected: Android, iOS, macOS, Windows (All)
Files Changed: 1 implementation (src/Controls/src/Core/Setter.cs), 3 test files
Key Findings
- When a VSM
<Setter Property="Style" Value="{StaticResource ...}"/>is used, the old code calledSetValue(StyleProperty, style), which replaced the entire Style object including itsVisualStateGroupsattached property attachment — permanently breaking VSM. - The PR fixes this by special-casing
Stylevalues inSetter.ApplyandSetter.UnApply: instead of setting/clearing the Style property, it callsIStyle.Apply(target, specificity)andIStyle.UnApply(target)to apply/remove the style's individual setters while leaving the VSM connection intact. - The
UnApplybranch has an earlyreturnto prevent the subsequentClearValue(Property, specificity)from clearing the Style property set by other means. - Tests use
IsEnabled = !IsEnabled(toggling Disabled state) to exercise Normal→Disabled→Normal cycles. - The PR has
partner/syncfusionlabel and was previously flaggeds/agent-changes-requested— now re-submitted after addressing prior reviewer concerns. - Prior AI review identified 3 issues; the author says all are addressed: (1) ClearValue guard via early return, (2) UnApply correctness, (3) multi-cycle test coverage.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: medium
Errors: 0 | Warnings: 2 | Suggestions: 2
Key code review findings:
⚠️ Setter.cs:89— New code path fires from ALLSetter.Applycallers (VSM, Style.ApplyCore, TriggerBase), not just VSM. For TriggerBase callers, this is a behavioral change: style setters now fire at trigger specificity (>StyleLocal), enabling trigger-set styles to override manual values. Additionallyelement.Styleis never updated (stale), and TargetType compatibility check is bypassed.⚠️ Setter.cs:115—_targets/specificitiestracking inconsistency when same Style instance is used in bothStyle=attribute and VSM setter.IStyle.UnApplyremoves element from_targetsentirely, leaving MergedStyle with a stale reference during transitions. Author acknowledges it self-heals; needs a code comment.- 💡 Missing trailing newlines in all 3 new test files (
Issue17175.xaml,.xaml.cs,.cs) - 💡 No unit test for core
Setter.cschange — aVisualStateManagerTestsunit test would be cheaper/faster than UI test for this core infrastructure change
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #33389 | Intercept Style-valued setters in Setter.Apply/UnApply, call IStyle.Apply/IStyle.UnApply instead of SetValue/ClearValue |
✅ PASSED (Gate) | Setter.cs + 3 test files |
Original PR |
🔬 Code Review — Deep Analysis
Code Review — PR #33389
Independent Assessment
What this changes: In Setter.Apply and Setter.UnApply, a new branch intercepts the case where a Setter targets StyleableElement.StyleProperty or Span.StyleProperty and the value is a Style. Instead of calling SetValue(StyleProperty, style, specificity) (which routes through MergedStyle and hardcodes StyleLocal specificity), it calls ((IStyle)style).Apply(targetObject, specificity) directly, bypassing MergedStyle entirely. In UnApply, IStyle.UnApply(targetObject) is called and an early return prevents ClearValue from being called (correct, since SetValue was never called on the way in).
Inferred motivation: When a VSM setter assigns a style, the intended outcome is for that style's property setters to "win" at VSM specificity, overriding manually-set values. Going through MergedStyle loses that specificity: MergedStyle.Apply always hardcodes StyleLocal (value 0x0001000000000000) for its style application, which is below ManualValueSetter specificity (0x0FFF000010FFFFFF). So style setters applied via MergedStyle can never override XAML-attribute values. Calling IStyle.Apply directly preserves the caller's specificity all the way down to each setter.
Reconciliation with PR Narrative
Author claims: "The old code set the Style property by replacing the entire Style object. This removed the original Style that contained the VisualStateGroups attached property. Once the VSM attachment is lost, the control cannot transition to other states."
Agreement: The fix is correct and addresses a real problem. The element.Style property indeed changes away from the original style when VSM calls SetValue(StyleProperty, ...) via the old path, which can have downstream effects.
Disagreement on root cause framing: The primary failure mechanism is not "the VSG attachment is lost." VisualStateGroups is an attached property on VisualStateManager, stored in the element's own bindable properties, completely independent of what StyleProperty is set to. The VSM still works after the style changes.
The real root cause (which the author correctly identifies in a later comment) is: MergedStyle.Apply hardcodes StyleLocal specificity, so styles applied by VSM via MergedStyle cannot override XAML-attribute values (which are at ManualValueSetter specificity). The test's DisabledButtonStyle setter Text="State: Disabled" is at StyleLocal but competes with Text="State: Normal" set in XAML at ManualValueSetter. StyleLocal < ManualValueSetter → style setter loses → text stays "State: Normal" instead of "State: Disabled".
Findings
⚠️ Warning — New code path fires from ALL Setter.Apply callers, not just VSM
src/Controls/src/Core/Setter.cs lines 89–94
Setter.Apply is called from:
VisualStateManager(the intended target)Style.ApplyCore(line 202 ofStyle.cs)TriggerBase(line 130 ofInteractivity/TriggerBase.cs)
When a Style or Trigger has a setter that assigns StyleProperty, the new code path fires with the caller's specificity (e.g., StyleLocal for Style.ApplyCore). This means:
StylePropertyon the element is never updated.element.Stylestill returns the outer style (or the XAML-attribute style), not the nested style._mergedStyle._styleis not updated. Any code that readselement.Styleto determine the current visual style will see a stale value.- TargetType compatibility check is bypassed.
MergedStyle.SetStylechecks!value.TargetType.IsAssignableFrom(TargetType)and logs a warning for incompatible styles. This check is now skipped.
For Style.ApplyCore callers, specificity passed through is already StyleLocal-based (same as MergedStyle would use), so the effective property values are identical. The behavioral regression is limited to: element.Style not reflecting the nested style, and the compatibility check being skipped.
For TriggerBase callers, trigger specificity (0x0FFF000020FFFFFF) > StyleLocal, so the behavioral change is more significant: trigger-set style properties now override manual values, which was not the case before.
Suggested fix or discussion: Narrow the guard to only apply when the specificity is VSM-level or higher, or add a code comment explicitly documenting that this intentionally changes Trigger and Style-nested-style semantics as well.
⚠️ Warning — _targets / specificities tracking inconsistency when same Style is used in both Style= attribute and VSM setter
src/Controls/src/Core/Setter.cs lines 89–94 and 115–121
When the same Style instance is assigned via Style="{StaticResource BaseButtonStyle}" (applied by MergedStyle at StyleLocal → adds to _targets[button] = StyleLocal) and by a VSM setter in the Normal state (new code path → overwrites _targets[button] = vsm_specificity), the two style applications share a single _targets entry.
When VSM transitions away from Normal and calls IStyle.UnApply(button), _targets.Remove(button) removes the element completely. At this point, MergedStyle still holds _style = BaseButtonStyle, and BaseButtonStyle's specificities[button] = vsm_specificity has been removed. The element is invisible to BasedOnChanged until VSM re-applies the Normal state (which re-adds it).
The author correctly notes this self-heals on the next VSM Apply. However, during the gap:
- If
BaseButtonStyle.BasedOnor its_basedOnResourcePropertydynamic resource updates, the element will silently miss the update. - If the element is detached during that gap,
MergedStyle.UnApplycallsstyle.UnApply(target)which will return early (no-op) becausespecificitiesno longer tracksbutton— leaving style-applied property values un-cleaned atStyleLocalspecificity.
Recommend adding a comment on both branches documenting this known limitation.
💡 Suggestion — Missing trailing newlines in all three new test files
Issue17175.xaml (line 91), Issue17175.xaml.cs (line 16), Issue17175.cs (line 34) all end without a trailing newline. Add \n to each.
💡 Suggestion — No unit test for the core fix in Setter.cs
The UI test (Issue17175.cs) validates the full end-to-end flow, which is valuable. But Setter.cs is shared cross-platform infrastructure. A unit test in Controls.Core.UnitTests (alongside VisualStateManagerTests.cs) would be cheaper to run in CI, provide faster feedback, and more precisely document the specificity invariant. Similar coverage exists for other VSM+style interactions (e.g., ChangingStyleContainingVSMShouldResetStateValue, VSMFromStyleAreUnApplied).
💡 Suggestion — element.Style returns stale value after VSM applies a Style; not documented
After the VSM applies a style via the new code path, element.Style still returns the original (non-VSM) style. This is an observable API behavior change that should at minimum be noted in a code comment, or in the PR description, so maintainers don't investigate "unexpected" values when debugging.
Devil's Advocate
On the "fires from all callers" concern: In Style.ApplyCore, the specificity is already StyleLocal-based (matching MergedStyle), so the only practical change for Style-in-Style is the stale element.Style and skipped compatibility check. These are genuine regressions but very narrow — styles containing Setter Property="Style" are rare. A conservative review would flag this; a pragmatic review would note it's unlikely to bite anyone.
On the _targets gap: The author's self-healing argument is correct. The most dangerous failure mode (missed BasedOnChanged) requires a dynamic BasedOn resource to update during the exact state-transition window. Realistic? Very unlikely. A dealbreaker? No. But the dead window should be documented.
On CI: All checks are green including integration, unit, and multi-platform build tests. The PR has been validated on Android, Windows, iOS, and Mac per the author.
Is the fix correct for its primary purpose? Yes. Without this PR, DisabledButtonStyle's Text="State: Disabled" setter is applied at StyleLocal specificity, which loses to the XAML Text="State: Normal" at ManualValueSetter. The test would fail. With the fix, the style setters are applied at VSM specificity (higher than Manual), and the test correctly validates the transition.
Verdict: NEEDS_DISCUSSION
Confidence: medium
Summary: The core fix is correct and addresses a real, demonstrable bug. CI is fully green. The primary concern warranting discussion is that Setter.Apply's new code path is triggered from ALL callers — including Style.ApplyCore and TriggerBase — not just VSM. This causes an unintended behavioral change for the (admittedly rare) case of a Style or Trigger that assigns the Style property: element.Style never reflects the applied style, and the MergedStyle TargetType compatibility check is bypassed. A reviewer with domain ownership (StephaneDelcroix is assigned) should explicitly confirm whether these broader semantic changes are acceptable or whether the fix should be narrowed to VSM-only callers.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #33389 | Intercept Style-valued setters in Setter.cs for ALL callers; call IStyle.Apply/UnApply directly |
✅ PASSED (Gate) | Setter.cs + 3 test files |
Original PR; blast radius includes TriggerBase/Style.ApplyCore |
| 1 | try-fix-1 | VSM-scoped helpers in VisualStateManager.cs — ApplySetterForVsm/UnApplySetterForVsm |
✅ PASS | VisualStateManager.cs |
Zero blast radius outside VSM |
| 2 | try-fix-2 | Fix in MergedStyle.cs — SetStyle reads back actual specificity from BindablePropertyContext.Values |
✅ PASS | MergedStyle.cs, StyleableElement.cs, Span.cs |
Most architecturally correct; fixes root cause |
| 3 | try-fix-3 | Fix in StyleableElement.cs — OnStyleChanged detects VSM specificity, calls IStyle.Apply/UnApply |
✅ PASS | StyleableElement.cs |
Intercepts at property-change callback level |
| 4 | try-fix-4 | Fix in Setter.cs with VSM-specificity guard — same as PR but adds specificity.IsVsm check |
✅ PASS | Setter.cs |
Narrowed PR fix; eliminates blast radius concern |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | Yes | NEW IDEA: Fix in BindableObject.SetValueActual — when PropertyType is assignable to IStyle, auto-call Apply/UnApply |
| claude-sonnet-4.6 | 2 | No | NO NEW IDEAS — all meaningful interception points covered |
Exhausted: Yes — claude-opus-4.6's new idea (BindableObject layer) was noted but not tested as it would be a very broad architectural change affecting all IStyle-typed properties
Selected Fix: Pending final comparison in Report phase
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #17175 — VSM Style setter specificity loss via MergedStyle |
| Code Review | NEEDS_DISCUSSION (medium) | 0 errors, 2 warnings (blast radius + _targets), 2 suggestions |
| Gate | ✅ PASSED | android |
| Try-Fix | ✅ COMPLETE | 4 attempts, 4 passing |
| Expert Eval | ✅ COMPLETE | pr-plus-reviewer candidate prepared |
| Report | ✅ COMPLETE |
Code Review Impact on Try-Fix
The code review's #1 warning (blast radius — all Setter.Apply callers affected, not just VSM) directly shaped all 4 try-fix attempts. Try-fix-1 eliminated the blast radius by moving to VisualStateManager.cs. Try-fix-4 addressed it by adding a specificity.IsVsm guard in Setter.cs. The expert reviewer's findings independently confirmed the blast radius concern was Critical (OnBasedOnResourceChanged reads wrong Style), not merely stylistic — leading to the pr-plus-reviewer candidate applying the IsVsm guard as the minimal, targeted fix.
Summary
PR #33389 correctly diagnoses and fixes issue #17175 (VSM style setters lose their specificity inside MergedStyle, preventing them from overriding manually-set values). The fix is validated on all four platforms. However, the implementation has two structural issues that require changes before merge: (1) the Setter.Apply intercept fires for ALL callers (not just VSM), causing OnBasedOnResourceChanged to read the wrong style for non-VSM cases — a regression; and (2) no unit test for the core behavior change. The pr-plus-reviewer candidate resolves both with a one-line specificity.IsVsm guard and two targeted unit tests.
Root Cause
SetValue(StyleProperty, style, vsmSpecificity) fires propertyChanged → _mergedStyle.Style = style → MergedStyle.Apply(target, hardcoded StyleLocal). VSM specificity is discarded at the MergedStyle layer. Style setters applied at StyleLocal cannot override XAML-attribute values at ManualValueSetter specificity. The PR's fix correctly bypasses this by calling IStyle.Apply(target, specificity) directly — but must be guarded to VSM-only callers.
Fix Quality
Winning candidate: pr-plus-reviewer
| Candidate | Result | Key Property |
|---|---|---|
pr (raw) |
✅ PASS (Gate) | Blast radius: fires for TriggerBase/Style.ApplyCore callers; Critical regression in OnBasedOnResourceChanged |
pr-plus-reviewer |
✅ PASS | + specificity.IsVsm guard eliminates blast radius + regression; + 2 unit tests |
try-fix-1 |
✅ PASS | VSM-scoped in VisualStateManager.cs; zero blast radius; no unit tests |
try-fix-2 |
✅ PASS | MergedStyle.cs root-cause fix; most architecturally precise; 3 files changed; no unit tests |
try-fix-3 |
✅ PASS | StyleableElement.cs; extra _visualStateStyle field; no unit tests |
try-fix-4 |
✅ PASS | Setter.cs + IsVsm guard; re-implements style application logic manually vs using IStyle.Apply |
Why pr-plus-reviewer wins:
- Same files changed as the PR (Setter.cs + test files) — minimal diff against base
specificity.IsVsmguard is a one-line addition per branch that eliminates both the Critical regression and the Major blast radius concern- Unit tests (
VSMStyleSetterAppliesStyleAtVsmSpecificity,VSMStyleSetterDoesNotReplaceStyleProperty) directly prove the specificity contract and that StyleProperty is not replaced - The PR's existing UI test cycles Normal→Disabled→Normal→Disabled; the unit tests add faster, more targeted coverage
- try-fix-1/2/3 are architecturally valid alternatives, but involve touching more or different files; the IsVsm guard in Setter.cs is the most surgical fix
Required changes before merge:
- Add
&& specificity.IsVsmguard on the Style intercept branch in bothSetter.ApplyandSetter.UnApply - Add unit tests in
VisualStateManagerTests.csvalidating the specificity behavior and thatStylePropertyis not replaced - Add trailing newlines to the 3 new test files
- (Optional) Document that
DynamicResource-valued Style setters in VSM are not fixed by this PR (they still hit the old path)
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 11 findings
See inline comments for details.
kubaflo
left a comment
There was a problem hiding this comment.
Looks good! Could you check the ai's suggestions before merge?
@kubaflo, I addressed the concerns |
) <!-- 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. !!!!!!! --> ### Issue Details VisualStateManager permanently breaks when attempting to set a control's Style property during state transitions. When properties like IsEnabled change, VSM automatically transitions states - but if that state contains a Style setter, the control loses all VSM functionality and can no longer respond to any state changes. ### Root Cause The old code set the Style property by replacing the entire Style object. This removed the original Style that contained the VisualStateGroups attached property. Once the VSM attachment is lost, the control cannot transition to other states - VSM is permanently broken for that control. ### Description of Change Added special handling when Setter values are Styles, using the IStyle.Apply() and IStyle.UnApply() methods. Apply() applies the Style's individual setters without replacing the Style property, keeping the VSM connection intact. UnApply() properly removes the Style's setters when the state changes, preventing conflicts between different states. Validated the behavior in the following platforms - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Issues Fixed Fixes #17175 ### Output ScreenShot |Before|After| |--|--| | <video src="https://github.com/user-attachments/assets/a5a2a482-73a2-4e84-a133-afacaf1c3872" >| <video src="https://github.com/user-attachments/assets/a1079f90-06cb-4083-8202-2517200b5346">|
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!
Issue Details
VisualStateManager permanently breaks when attempting to set a control's Style property during state transitions. When properties like IsEnabled change, VSM automatically transitions states - but if that state contains a Style setter, the control loses all VSM functionality and can no longer respond to any state changes.
Root Cause
The old code set the Style property by replacing the entire Style object. This removed the original Style that contained the VisualStateGroups attached property. Once the VSM attachment is lost, the control cannot transition to other states - VSM is permanently broken for that control.
Description of Change
Added special handling when Setter values are Styles, using the IStyle.Apply() and IStyle.UnApply() methods. Apply() applies the Style's individual setters without replacing the Style property, keeping the VSM connection intact. UnApply() properly removes the Style's setters when the state changes, preventing conflicts between different states.
Validated the behavior in the following platforms
Issues Fixed
Fixes #17175
Output ScreenShot
17175-Before.mov
17175-AfterFix.mov