microsoft / microsoft/microsoft-ui-xaml

RadioMenuFlyoutItem leaks stale registration when GroupName changes after IsChecked

Open
#11,098 1 comment 0 reactions 0 assignees View on GitHub
area-Menus bug team-Controls
Dominant language
C++
Stars
8.4k
Forks
942
Avg merge
2d 7h
Merged PRs (30d)
105

Description

### Describe the bug

`RadioMenuFlyoutItem` keys its internal `s_selectionMap` by `GroupName`. When `IsChecked` is set before `GroupName` — e.g. when those attributes appear in that order in XAML, or when `GroupName` is changed in code while the item is checked — the item ends up registered under both the previous (default `""`) key *and* the new key. The stale entry is never cleaned up, so an unrelated `RadioMenuFlyoutItem` whose `GroupName` happens to match the stale key (most commonly the default empty string) will incorrectly uncheck the original item when it becomes checked.

### Why is this important?

Apps that bind `IsChecked` and `GroupName` to view-model state, or that simply rely on standard XAML attribute order (`IsChecked="True" GroupName="..."`), end up with radio items that silently un-check each other across unrelated menus / groups. The behavior is non-deterministic from the app developer's perspective because it depends on XAML attribute order in the parsed source.

### Steps to reproduce the bug

1. Create a blank WinUI 3 (WinAppSDK) project.
2. Replace `MainWindow.xaml` with two `MenuFlyout`s. The first contains a `RadioMenuFlyoutItem` declared as `IsChecked="True" GroupName="MyGroup"` (in that attribute order) plus a sibling in `"MyGroup"`. The second contains a single `RadioMenuFlyoutItem` with no `GroupName` set.
3. Open the first menu (to fire `OnLoaded` on the checked item), then close it.
4. Open the second menu, then close it.
5. Programmatically set the second menu's item's `IsChecked = true`.

Minimal XAML + code-behind:

```xml

















```

```csharp
private void OnCheckC(object sender, RoutedEventArgs e)
{
ItemC.IsChecked = true;
Status.Text = $"ItemA.IsChecked = {ItemA.IsChecked} (expected: True)";
}
```

### Actual behavior

After step 5, `ItemA.IsChecked` becomes `False`, even though `ItemA` is in `"MyGroup"` and `ItemC` is in the default empty group — they should be independent.

### Expected behavior

`ItemA.IsChecked` should remain `True`. Items in different `GroupName`s should not affect one another.

### NuGet package version

1.8.260317003

### Windows version

Windows 11 (24H2): Build 26100

### Additional context

Root cause is in `RadioMenuFlyoutItem::UpdateCheckedItemInGroup` / `OnPropertyChanged` (file `controls/dev/RadioMenuFlyoutItem/RadioMenuFlyoutItem.cpp`). When `s_GroupNameProperty` changes, `m_groupName` is updated but no cleanup of any previous registration in `s_selectionMap` is performed. `OnLoaded` only inserts under the current group; it does not erase the prior entry. `OnUnloaded` then calls `SharedHelpers::EraseIfExists(*s_selectionMap, m_groupName)`, which is keyed by the *current* `GroupName`, so it can erase the wrong entry while leaving the stale one in place.

Suggested fix: track the `GroupName` actually used at registration time (separate from `m_groupName`) and, on every re-registration, remove the prior entry if it still points to `this`.

Contributor guide

Open the contributing guide

Research direction

Start in controls/dev/RadioMenuFlyoutItem/RadioMenuFlyoutItem.cpp and read UpdateCheckedItemInGroup, OnPropertyChanged, OnLoaded, and OnUnloaded. Reproduce the provided XAML ordering scenario, then verify that changing GroupName or registering an item does not leave an obsolete selection-map entry and that differently grouped items remain checked independently.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
desktop, frontend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
66/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.