Skip to content

Commit 1783670

Browse files
StephaneDelcroixCopilot
StephaneDelcroix
and
Copilot
committed
Fix VisualStateGroups duplicate name crash with implicit styles (#34716)
When an implicit style (e.g., default Button style in Styles.xaml) sets VisualStateManager.VisualStateGroups with a 'CommonStates' group, and the XAML also explicitly declares VisualStateGroups with 'CommonStates', all inflators (Runtime, XamlC, SourceGen) call GetValue() + Add() on the style-populated list, causing 'VisualStateGroup Names must be unique'. Fix: VisualStateGroupList.Add() now removes any existing group with the same name before adding the new one, so explicit XAML groups replace style-set groups. This is backward-compatible since duplicate names were never valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 720a9d4 commit 1783670

4 files changed

Lines changed: 225 additions & 3 deletions

File tree

‎src/Controls/src/Core/VisualStateManager.cs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,21 @@ public void Add(VisualStateGroup item)
272272
throw new ArgumentNullException(nameof(item));
273273
}
274274

275+
// If a group with the same name already exists (e.g., set by an implicit style),
276+
// remove it so the explicitly-added group takes precedence.
277+
if (!string.IsNullOrEmpty(item.Name))
278+
{
279+
for (int i = _internalList.Count - 1; i >= 0; i--)
280+
{
281+
if (string.Equals(_internalList[i].Name, item.Name, StringComparison.Ordinal))
282+
{
283+
_internalList[i].StatesChanged -= ValidateAndNotify;
284+
_internalList.Remove(_internalList[i]);
285+
break;
286+
}
287+
}
288+
}
289+
275290
_internalList.Add(item);
276291

277292
item.StatesChanged += ValidateAndNotify;
@@ -751,7 +766,7 @@ internal static IList<VisualStateGroup> Clone(this IList<VisualStateGroup> group
751766
group.VisualElement = clone.VisualElement;
752767
clone.Add(group.Clone());
753768
}
754-
769+
755770
// Preserve specificity when cloning (issue #27202)
756771
if (groups is VisualStateGroupList sourceList)
757772
{

‎src/Controls/tests/Core.UnitTests/VisualStateManagerTests.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,16 @@ public void StateNamesMustBeUniqueWithinGroupListWhenAddingGroup()
141141
}
142142

143143
[Fact]
144-
public void GroupNamesMustBeUniqueWithinGroupList()
144+
public void GroupWithDuplicateNameReplacesExisting()
145145
{
146146
IList<VisualStateGroup> vsgs = CreateTestStateGroups();
147147
var secondGroup = new VisualStateGroup { Name = CommonStatesGroupName };
148+
secondGroup.States.Add(new VisualState { Name = NormalStateName });
148149

149-
Assert.Throws<InvalidOperationException>(() => vsgs.Add(secondGroup));
150+
// Adding a group with the same name should replace the existing one, not throw
151+
vsgs.Add(secondGroup);
152+
Assert.Single(vsgs);
153+
Assert.Same(secondGroup, vsgs[0]);
150154
}
151155

152156
[Fact]

‎src/Controls/tests/Xaml.UnitTests/Issues/Maui34716.xaml‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
3+
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
4+
x:Class="Microsoft.Maui.Controls.Xaml.UnitTests.Maui34716">
5+
<ContentPage.Resources>
6+
<Style x:Key="VsgStyle" TargetType="Button">
7+
<Setter Property="VisualStateManager.VisualStateGroups">
8+
<VisualStateGroupList>
9+
<VisualStateGroup Name="CommonStates">
10+
<VisualState Name="Normal">
11+
<VisualState.Setters>
12+
<Setter Property="BackgroundColor" Value="White" />
13+
</VisualState.Setters>
14+
</VisualState>
15+
<VisualState Name="Pressed">
16+
<VisualState.Setters>
17+
<Setter Property="BackgroundColor" Value="LightGray" />
18+
</VisualState.Setters>
19+
</VisualState>
20+
</VisualStateGroup>
21+
</VisualStateGroupList>
22+
</Setter>
23+
</Style>
24+
</ContentPage.Resources>
25+
<StackLayout>
26+
<!-- Direct VisualStateGroups on element -->
27+
<Button x:Name="button" Text="Press me">
28+
<VisualStateManager.VisualStateGroups>
29+
<VisualStateGroup Name="CommonStates">
30+
<VisualState Name="Normal">
31+
<VisualState.Setters>
32+
<Setter Property="BackgroundColor" Value="White" />
33+
</VisualState.Setters>
34+
</VisualState>
35+
<VisualState Name="Pressed">
36+
<VisualState.Setters>
37+
<Setter Property="BackgroundColor" Value="LightGray" />
38+
</VisualState.Setters>
39+
</VisualState>
40+
</VisualStateGroup>
41+
</VisualStateManager.VisualStateGroups>
42+
</Button>
43+
44+
<!-- Style-based VisualStateGroups -->
45+
<Button x:Name="button2" Text="Styled" Style="{StaticResource VsgStyle}" />
46+
47+
<!-- Two groups on same element -->
48+
<Button x:Name="button3" Text="Two groups">
49+
<VisualStateManager.VisualStateGroups>
50+
<VisualStateGroup Name="CommonStates">
51+
<VisualState Name="Normal">
52+
<VisualState.Setters>
53+
<Setter Property="BackgroundColor" Value="White" />
54+
</VisualState.Setters>
55+
</VisualState>
56+
</VisualStateGroup>
57+
<VisualStateGroup Name="FocusStates">
58+
<VisualState Name="Focused">
59+
<VisualState.Setters>
60+
<Setter Property="BackgroundColor" Value="Yellow" />
61+
</VisualState.Setters>
62+
</VisualState>
63+
</VisualStateGroup>
64+
</VisualStateManager.VisualStateGroups>
65+
</Button>
66+
67+
<!-- Explicit VisualStateGroupList -->
68+
<Button x:Name="button4" Text="Explicit list">
69+
<VisualStateManager.VisualStateGroups>
70+
<VisualStateGroupList>
71+
<VisualStateGroup Name="CommonStates">
72+
<VisualState Name="Normal">
73+
<VisualState.Setters>
74+
<Setter Property="BackgroundColor" Value="White" />
75+
</VisualState.Setters>
76+
</VisualState>
77+
</VisualStateGroup>
78+
</VisualStateGroupList>
79+
</VisualStateManager.VisualStateGroups>
80+
</Button>
81+
</StackLayout>
82+
</ContentPage>

‎src/Controls/tests/Xaml.UnitTests/Issues/Maui34716.xaml.cs‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
using System;
2+
using Microsoft.Maui.Controls.Core.UnitTests;
3+
using Xunit;
4+
5+
namespace Microsoft.Maui.Controls.Xaml.UnitTests;
6+
7+
public partial class Maui34716 : ContentPage
8+
{
9+
public Maui34716() => InitializeComponent();
10+
11+
[Collection("Issue")]
12+
public class Tests : IDisposable
13+
{
14+
public Tests()
15+
{
16+
Application.SetCurrentApplication(new MockApplication());
17+
}
18+
19+
public void Dispose()
20+
{
21+
Application.Current = null;
22+
}
23+
24+
// Registers an implicit Button style with VisualStateGroups (like the default MAUI template Styles.xaml)
25+
void SetupImplicitButtonStyle()
26+
{
27+
Application.Current.Resources.Add(new Style(typeof(Button))
28+
{
29+
Setters =
30+
{
31+
new Setter
32+
{
33+
Property = VisualStateManager.VisualStateGroupsProperty,
34+
Value = new VisualStateGroupList
35+
{
36+
new VisualStateGroup
37+
{
38+
Name = "CommonStates",
39+
States =
40+
{
41+
new VisualState { Name = "Normal" },
42+
new VisualState { Name = "Pressed" },
43+
new VisualState { Name = "Disabled" }
44+
}
45+
}
46+
}
47+
}
48+
}
49+
});
50+
}
51+
52+
[Theory]
53+
[XamlInflatorData]
54+
internal void VisualStateGroupsOnElementShouldNotThrowDuplicateNames(XamlInflator inflator)
55+
{
56+
var page = new Maui34716(inflator);
57+
Assert.NotNull(page);
58+
59+
var groups = VisualStateManager.GetVisualStateGroups(page.button);
60+
Assert.Single(groups);
61+
Assert.Equal("CommonStates", groups[0].Name);
62+
Assert.Equal(2, groups[0].States.Count);
63+
}
64+
65+
[Theory]
66+
[XamlInflatorData]
67+
internal void VisualStateGroupsWithImplicitStyleShouldNotThrowDuplicateNames(XamlInflator inflator)
68+
{
69+
// This is the real bug scenario: an implicit style already sets VisualStateGroups
70+
// with "CommonStates", then the XAML also sets VisualStateGroups with "CommonStates".
71+
// The SG calls GetValue() (returning the style's list) then Add() → duplicate name → crash.
72+
SetupImplicitButtonStyle();
73+
74+
var page = new Maui34716(inflator);
75+
Assert.NotNull(page);
76+
77+
// The explicit XAML VisualStateGroups should replace the implicit style's groups
78+
var groups = VisualStateManager.GetVisualStateGroups(page.button);
79+
Assert.Single(groups);
80+
Assert.Equal("CommonStates", groups[0].Name);
81+
Assert.Equal(2, groups[0].States.Count);
82+
}
83+
84+
[Theory]
85+
[XamlInflatorData]
86+
internal void VisualStateGroupsViaStyleShouldNotThrow(XamlInflator inflator)
87+
{
88+
var page = new Maui34716(inflator);
89+
Assert.NotNull(page);
90+
91+
var groups = VisualStateManager.GetVisualStateGroups(page.button2);
92+
Assert.Single(groups);
93+
Assert.Equal("CommonStates", groups[0].Name);
94+
}
95+
96+
[Theory]
97+
[XamlInflatorData]
98+
internal void MultipleVisualStateGroupsShouldNotThrow(XamlInflator inflator)
99+
{
100+
var page = new Maui34716(inflator);
101+
Assert.NotNull(page);
102+
103+
var groups = VisualStateManager.GetVisualStateGroups(page.button3);
104+
Assert.Equal(2, groups.Count);
105+
Assert.Equal("CommonStates", groups[0].Name);
106+
Assert.Equal("FocusStates", groups[1].Name);
107+
}
108+
109+
[Theory]
110+
[XamlInflatorData]
111+
internal void ExplicitVisualStateGroupListShouldNotThrow(XamlInflator inflator)
112+
{
113+
var page = new Maui34716(inflator);
114+
Assert.NotNull(page);
115+
116+
var groups = VisualStateManager.GetVisualStateGroups(page.button4);
117+
Assert.Single(groups);
118+
Assert.Equal("CommonStates", groups[0].Name);
119+
}
120+
}
121+
}

0 commit comments

Comments
 (0)