Skip to content

[Windows] Fix crash when clicking a tab with sub tabs in the Shell TabBar overflow menu - #39412

Merged
kubaflo merged 5 commits into
dotnet:inflight/currentfrom
Empiree:fix/windows-shell-overflow-subtabs
Oct 21, 2026
Merged

kubaflo merged 5 commits into
dotnet:inflight/currentfrom
Empiree:fix/windows-shell-overflow-subtabs

Conversation

@Empiree

@Empiree Empiree commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

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!

Description of Change

On Windows, clicking a Shell tab with more than one ShellContent in the overflow menu of the top tab bar (the ... button) crashed the app with E_BOUNDS (invalid vector subscript).

The crash is inside WinUI NavigationView. From what I see in the WinUI code, in Top mode it cant find the container of a data bound item that sits in the overflow menu, so the clicked item never gets IsSelected, only IsChildSelected, and when it expands right after the click WinUI reads past the end of the selected index path. MAUI always fills the tab bar through MenuItemsSource, so every tab with sub tabs runs into it. A plain WinUI NavigationView with MenuItemsSource and a similar item template crashes the same way.

Fixing up the selection state after the click only helps for the first click, other overflow bugs in WinUI show up after that (for example picking a sub tab from the overflow menu and closing the menu later crashes again). So now ShellItemHandler sets SelectsOnInvoked to false on overflow menu containers that have sub tabs, and clears it when the container leaves the overflow menu. MapMenuItems also updates the items that are already in the overflow menu, because a tab can get or lose sub tabs while it sits there. A click on such a tab just expands it, picking one of its sub tabs navigates like before and WinUI moves the tab into the bar. Tabs in the bar work like before.

There are four device tests. The first one opens a tab with one ShellContent from the overflow menu, then clicks the tab with sub tabs, picks Sub B and checks the navigation. The second one adds and removes a sub tab of a tab that already sits in the open overflow menu and checks the item follows. The third one moves a tab with sub tabs from the overflow menu back into the bar and checks it gets selected there like before. The fourth one applies the NavigationView template again and checks the new overflow menu behaves the same. Without the fix the click makes WinUI kill the whole test app, so the tests check SelectsOnInvoked before clicking to fail in a normal way.

Issues Fixed

Fixes #37985

WinUI cant select a data bound NavigationView item with children while it sits in the overflow menu of the top tab bar, it fails with E_BOUNDS. In the overflow menu such a tab now only expands, and the navigation happens when one of its sub tabs is picked.

Fixes dotnet#37985
@Empiree
Empiree temporarily deployed to copilot-pat-pool October 15, 2026 19:33 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 39412

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 39412"

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@Empiree
Empiree temporarily deployed to copilot-pat-pool October 15, 2026 19:33 — with GitHub Actions Inactive
@dotnet-policy-service dotnet-policy-service Bot added the community ✨ Community Contribution label Oct 15, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Hey there @@Empiree! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Hey there @Empiree! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@kubaflo

This comment has been minimized.

@github-actions github-actions Bot added the s/agent-review-in-progress AI review is currently running for this PR label Oct 16, 2026
@kubaflo

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

/review gate was not started: another review or Gate is already running for this PR.

This command is not queued. Submit it again after the current run finishes.

@kubaflo

This comment has been minimized.

@azure-pipelines

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

/review gate was not started: another review or Gate is already running for this PR.

This command is not queued. Submit it again after the current run finishes.

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert Review — 1 findings

See inline comments for details.

// a tab with sub tabs only expands, and the navigation happens when one of its sub tabs is picked.
static void OnOverflowMenuItemPrepared(ItemsRepeater sender, ItemsRepeaterElementPreparedEventArgs args)
{
if (args.Element is NavigationViewItem item && item.DataContext is NavigationViewItemViewModel { MenuItemsSource: not null })

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 AI-Generated Review (multi-model)

[major] Logic and Correctness — SelectsOnInvoked is updated only during ElementPrepared/ElementClearing, so an already-prepared overflow NavigationViewItem is not updated when its bound NavigationViewItemViewModel.MenuItemsSource changes at runtime. Concrete scenario: a tab initially has a single ShellContent (prepared with default selectable behavior) and later receives another ShellContent via ItemsCollectionChanged/MapMenuItems, making MenuItemsSource non-null without recycling the existing overflow container; clicking it in overflow still follows the select path that this fix is meant to avoid (E_BOUNDS repro path).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, MapMenuItems now updates SelectsOnInvoked of the overflow items that are already realized. Covered by TabBarOverflowMenuTabFollowsItsShellContentChanges.

MauiBot

This comment was marked as outdated.

@MauiBot MauiBot added s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-gate-passed AI verified tests catch the bug (fail without fix, pass with fix) s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) and removed s/agent-review-in-progress AI review is currently running for this PR labels Oct 16, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert Review — 2 findings

Reviewed commit 7478a41.

See inline comments for details.

static void OnOverflowMenuItemPrepared(ItemsRepeater sender, ItemsRepeaterElementPreparedEventArgs args)
{
if (args.Element is NavigationViewItem item && item.DataContext is NavigationViewItemViewModel { MenuItemsSource: not null })
item.SelectsOnInvoked = false;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[major] Logic and Correctness - Keep SelectsOnInvoked synchronized with changes to an existing overflow item's MenuItemsSource. Open overflow for an inactive tab containing two ShellContents, remove one content, then change TabBar.Title to refresh MapMenuItems while the container remains realized. SyncItems retains the existing view model, and MapMenuItems sets its MenuItemsSource to null. The template updates its children, but neither ElementPrepared nor ElementClearing runs for that property update, leaving SelectsOnInvoked=false. The now-single-content tab cannot be selected and has no children through which to navigate. Update the restriction on child-source changes as well as container preparation, and cover this transition in the regression test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the same way, the new test also checks the other direction (sub tab removed, the tab is selectable again and navigates)


// The tab only expands to show its sub tabs, the current page stays the same
Assert.True(nestedItem.IsExpanded);
Assert.NotEqual(nestedTab, shell.CurrentItem.CurrentItem);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[moderate] Regression Prevention and Test Coverage - Add a negative-case assertion for an ordinary single-content tab in the overflow menu. The fixture creates seven such tabs but never invokes one; selecting Sub B exercises a child container, not a top-level container prepared by the overflow ItemsRepeater. Setting SelectsOnInvoked=false for every top-level overflow item would therefore still satisfy this test while preventing ordinary overflow tabs from navigating. That working case is an explicit counter-check in #37985. Invoke a single-content tab from the same overflow menu and assert that Shell switches to it, so the test distinguishes selective suppression from disabling all overflow selection.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, the test now opens Tab 7 from the overflow menu first and checks the navigation.

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but could you please check the ai's suggestions?

@github-actions github-actions Bot added the s/agent-review-in-progress AI review is currently running for this PR label Oct 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Windows device-test results remain unverified for this native selection and container-recycling workaround.

No open issues 🎉

What changed in this PR

Addresses #37985 by preventing Windows Shell tabs with nested content from being selected directly in the overflow menu.

Changes:

  • Makes nested overflow tabs expand first, with navigation through their subtabs.
  • Updates selection behavior when content changes or containers leave overflow.
  • Adds three Windows device tests covering navigation, content changes, and return to the tab bar.
File Description
src/​Core/​src/​Platform/​Windows/​MauiNavigationView.cs Exposes the overflow items host internally.
src/​Controls/​tests/​DeviceTests/​Elements/​Shell/​ShellTabBarTests.Windows.cs Adds overflow regression tests.
src/​Controls/​src/​Core/​Handlers/​Shell/​ShellItemHandler.Windows.cs Tracks overflow containers and manages selection behavior.

Review effort: Balanced

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review Summary

@Empiree — new AI review results are available based on commit b8d8383.

Gate Passed Confidence Low Platform Windows


🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix

Gate Result: ✅ PASSED

Platform: WINDOWS · Base: main · Merge base: c2b1f685

Test Without Fix (expect FAIL) With Fix (expect PASS)
📱 ShellTests (TabWithSeveralShellContentsOpensFromTabBarOverflowMenu, TabBarOverflowMenuTabFollowsItsShellContentChanges, TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu) Category=Shell ✅ FAIL — 475s ✅ PASS — 351s
🔴 Without fix — 📱 ShellTests (TabWithSeveralShellContentsOpensFromTabBarOverflowMenu, TabBarOverflowMenuTabFollowsItsShellContentChanges, TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu): FAIL ✅ · 475s

(no coded error found; showing last 1200 chars)

ss the warning by specifying `x:SuppressXamlTrimWarnings=True` within the closest element. If not, the property path might be trimmed and will not be AOT compatible. [D:\a\1\s\src\Controls\src\Core\Controls.Core.csproj::TargetFramework=net10.0-windows10.0.19041.0]
Platform\Windows\TabbedPage\TabbedPageStyle.xaml(13,13): XamlCompiler warning WMC1510: Ensure the property path is trimming and AOT compatible by making use of 'Compiled Bindings (x:bind)' if possible or by specifying the 'x:DataType' directive with the respective binding data context and marking the type declaration with the 'WinRT.GeneratedBindableCustomProperty' attribute or the 'Microsoft.UI.Xaml.Data.Bindable' attribute. If you are unable to do either but can ensure the data type is attributed correctly, then you can also suppress the warning by specifying `x:SuppressXamlTrimWarnings=True` within the closest element. If not, the property path might be trimmed and will not be AOT compatible. [D:\a\1\s\src\Controls\src\Core\Controls.Core.csproj::TargetFramework=net10.0-windows10.0.19041.0]
    0 Error(s)
Time Elapsed 00:05:12.20
  Passed: 0
  Failed: 3
  Skipped: 0
  Total: 3
  Tests completed with exit code: 1
🟢 With fix — 📱 ShellTests (TabWithSeveralShellContentsOpensFromTabBarOverflowMenu, TabBarOverflowMenuTabFollowsItsShellContentChanges, TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu): PASS ✅ · 351s

(no coded error found; showing last 1200 chars)

uppress the warning by specifying `x:SuppressXamlTrimWarnings=True` within the closest element. If not, the property path might be trimmed and will not be AOT compatible. [D:\a\1\s\src\Controls\src\Core\Controls.Core.csproj::TargetFramework=net10.0-windows10.0.19041.0]
Platform\Windows\TabbedPage\TabbedPageStyle.xaml(13,13): XamlCompiler warning WMC1510: Ensure the property path is trimming and AOT compatible by making use of 'Compiled Bindings (x:bind)' if possible or by specifying the 'x:DataType' directive with the respective binding data context and marking the type declaration with the 'WinRT.GeneratedBindableCustomProperty' attribute or the 'Microsoft.UI.Xaml.Data.Bindable' attribute. If you are unable to do either but can ensure the data type is attributed correctly, then you can also suppress the warning by specifying `x:SuppressXamlTrimWarnings=True` within the closest element. If not, the property path might be trimmed and will not be AOT compatible. [D:\a\1\s\src\Controls\src\Core\Controls.Core.csproj::TargetFramework=net10.0-windows10.0.19041.0]
    0 Error(s)
Time Elapsed 00:05:00.42
  Passed: 3
  Failed: 0
  Skipped: 0
  Total: 3
  Tests completed successfully
📁 Fix files reverted (2 files)
  • src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs
  • src/Core/src/Platform/Windows/MauiNavigationView.cs

📋 Pre-Flight — Context & Validation

PR #39412 Pre-Flight

Problem

PR #39412 addresses #37985: on Windows, selecting a Shell Tab with more than one nested ShellContent from the top TabBar overflow menu crashes in WinUI with E_BOUNDS / invalid vector subscript. The failure requires both nested Shell content and an overflow-menu container; the same tab works in the visible tab bar and programmatic navigation works.

The PR identifies the WinUI failure mechanism as a data-bound top-mode NavigationView being unable to resolve the selected item's container while it is hosted in the overflow repeater. Selection and immediate expansion then leave WinUI with an invalid selected-index path.

Existing PR Approach

The current PR:

  • exposes TopNavMenuItemsOverflowHost from MauiNavigationView;
  • subscribes ShellItemHandler to that overflow ItemsRepeater;
  • sets NavigationViewItem.SelectsOnInvoked = false for overflow containers whose view model has child menu items;
  • clears the local dependency-property value when a recycled container leaves overflow;
  • refreshes realized overflow containers when MapMenuItems adds or removes sub-tabs.

This deliberately makes a parent tab in overflow expand without first selecting it; choosing a child performs navigation. The implementation changes:

  • src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs
  • src/Core/src/Platform/Windows/MauiNavigationView.cs

The PR also adds three Windows Controls device tests in:

  • src/Controls/tests/DeviceTests/Elements/Shell/ShellTabBarTests.Windows.cs

Alternative-Fix Boundary

Try-fix attempts may inspect the tests but may modify only the production files listed by .github/.baseline-state.json after EstablishBrokenBaseline.ps1 runs. The gate confirms the expected revertable allow-list is exactly the two production files above and that neither is newly added.

Candidate approaches must differ at the root-cause/mechanism level from the PR's overflow-repeater container tracking and SelectsOnInvoked mutation. Candidate 2 must also differ from candidate 1 and use candidate 1's recorded result without reopening or rerunning it.

Validation

The gate has already proven FAIL without the PR fix and PASS with it; it must not be rerun. Each candidate gets one implementation/test pass and at most one focused correction/retest using only:

pwsh .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Controls -Platform windows -TestFilter "Category=Shell" -IncludeClasses "Microsoft.Maui.DeviceTests.ShellTests" -IncludeMethods "TabWithSeveralShellContentsOpensFromTabBarOverflowMenu;TabBarOverflowMenuTabFollowsItsShellContentChanges;TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu"

These are the detected primary/regression tests:

  1. TabWithSeveralShellContentsOpensFromTabBarOverflowMenu
  2. TabBarOverflowMenuTabFollowsItsShellContentChanges
  3. TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu

Gate evidence: without fix, 0 passed / 3 failed; with fix, 3 passed / 0 failed.

Execution Constraints

  • Platform: Windows.
  • One approach per candidate.
  • One initial test and at most one focused correction/retest.
  • No gate verification and no broader/full suite.
  • Inline expert self-review only; no separate reviewer agent.
  • Every attempt must capture required try-fix artifacts and restore solely with pwsh .github/scripts/EstablishBrokenBaseline.ps1 -Restore.

🔬 Code Review — Deep Analysis

Code Review — PR #39412

Independent Assessment

What this changes: The Windows Shell item handler tracks the NavigationView top-overflow ItemsRepeater across template application and handler lifecycle. For realized overflow containers whose view model has child menu items, it sets NavigationViewItem.SelectsOnInvoked to false; it clears the local value when containers leave the overflow host or no longer have child items. Three Windows device tests cover overflow expansion/navigation, dynamic child changes while open, and container behavior after returning to the top bar.

Inferred motivation: WinUI cannot safely select a data-bound parent item with children while that item is in the top overflow menu. Preventing selection lets the parent expand and defers navigation until a child is chosen, avoiding the native E_BOUNDS crash while preserving normal top-bar selection.

Expert Evaluation

The single maui-expert-reviewer pass found no actionable issue on an added or modified line. Event subscription and unsubscription are symmetric, old overflow hosts are detached when the template replaces them, recycled containers have their local SelectsOnInvoked value cleared, and MapMenuItems refreshes already-realized overflow containers after child-list changes.

The reviewer returned NEEDS_DISCUSSION with low confidence solely because its GitHub client could not retrieve required-check and prior-review surfaces; it did not identify a code defect. The trusted pipeline Gate supplied to this phase passed: regression tests fail without the fix and pass with it.

Post-Evaluation Prior Review Reconciliation

Anonymous GitHub retrieval subsequently exposed one older error-level review and three associated coverage comments from earlier PR commit 7478a41324e60113dcbacd8e505582f866eef618. Every concrete concern is resolved in the submitted diff reviewed here:

Prior finding Current status Evidence in submitted HEAD
A prepared overflow item retained stale SelectsOnInvoked when its child source changed ✅ Fixed MapMenuItems now calls UpdateOverflowMenuItems, and TabBarOverflowMenuTabFollowsItsShellContentChanges covers both one-to-many and many-to-one transitions while the item remains in overflow.
No negative-case selection test for an ordinary single-content overflow tab ✅ Fixed TabWithSeveralShellContentsOpensFromTabBarOverflowMenu first invokes a single-content overflow tab and verifies navigation before exercising the nested tab.
No test that a nested parent becomes selectable after leaving overflow ✅ Fixed TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu widens the control, verifies SelectsOnInvoked, invokes the visible parent, and verifies navigation.

The current PR head reported six check runs through the anonymous API: four succeeded, one was skipped, and Build Insights was still in progress. Required-check classification remained unavailable, so confidence stays low; this does not revive any prior code finding.

Findings

No actionable inline findings. inline-findings.json contains [].

Blast Radius and Failure-Mode Probing

  • Runs for all instances: The new hooks attach to each Windows ShellItemHandler, but only realized top-overflow containers are modified, and only parent items with child menu items receive SelectsOnInvoked = false.
  • Startup impact: No new startup or static initialization path.
  • Static/shared state: None.
  • Disconnect/reconnect: DisconnectHandler removes both NavigationView and ItemsRepeater handlers and clears the tracked host; reconnect attaches to the current host.
  • Template reapplication: OnApplyTemplateFinished swaps tracking from the old overflow host to the newly materialized host. The reviewer suggested an additional focused template-reapply test as hardening, not as a demonstrated correctness defect.
  • Container reuse: ElementClearing clears the local dependency-property value before the container can be reused outside the overflow host.
  • Dynamic child changes: MapMenuItems updates realized overflow containers after adding or removing child items.

External Output Contract

Not applicable.

Verdict: NEEDS_DISCUSSION

Confidence: low

Summary: The expert review found the implementation coherent and no actionable line-level defect. Its non-LGTM verdict reflects unavailable GitHub CI/prior-review evidence rather than a flaw in the submitted fix; the trusted Gate evidence independently confirms the regression behavior.


🛠️ Try-Fix — Analysis & Comparison

Candidate 1 Try-Fix Report (PR #39412)

Approach: Overflow invocation veto in SelectionChanged: detect overflow popup item with children and restore prior selection

Prior approach avoided: Existing PR approach exposes TopNavMenuItemsOverflowHost, tracks overflow ItemsRepeater containers, and toggles NavigationViewItem.SelectsOnInvoked for overflow parent containers with cleanup on recycle/refresh.

Mechanism-level difference: This candidate avoids overflow container tracking and avoids SelectsOnInvoked mutation. It attempts to keep WinUI away from the failing selected-parent state by reverting selection to the previous stable item when an overflow parent-with-children is invoked, while still expanding that parent.

Files changed:

  • src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs

Result: Fail

Exact test command:
pwsh .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Controls -Platform windows -TestFilter "Category=Shell" -IncludeClasses "Microsoft.Maui.DeviceTests.ShellTests" -IncludeMethods "TabWithSeveralShellContentsOpensFromTabBarOverflowMenu;TabBarOverflowMenuTabFollowsItsShellContentChanges;TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu"

Exact test result(s):
test-execution-summaryTests run: 72 Passed: 69 Inconclusive: 0 Failed: 3 Ignored: 0
Tests run: 523 Passed: 69 Inconclusive: 0 Failed: 3 Ignored: 451 Skipped: 0
Passed: 0
Failed: 3
Scoped to method(s): TabWithSeveralShellContentsOpensFromTabBarOverflowMenu;TabBarOverflowMenuTabFollowsItsShellContentChanges;TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu
Failed test(s): Microsoft.Maui.DeviceTests.ShellTests.TabBarOverflowMenuTabFollowsItsShellContentChanges; Microsoft.Maui.DeviceTests.ShellTests.TabWithSeveralShellContentsOpensFromTabBarOverflowMenu; Microsoft.Maui.DeviceTests.ShellTests.TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu
Tests completed with exit code: 1

Failure analysis:
All three mandatory tests failed with assertions that overflow parent tabs were still selecting (Clicking "Nested"/"Changing" in the overflow menu selects it). This indicates the invocation-time selection-restoration mechanism did not change the pre-click selection semantics required by the tests.

Self-review count: 0 (inline self-review;
eviewer-findings.json)

Captured diff:
`diff
diff --git a/src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs b/src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs
index 9ff802f4cb..8265ca425f 100644
--- a/src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs
+++ b/src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs
@@ -6,6 +6,7 @@ using Microsoft.Maui.Controls.Platform;
using Microsoft.Maui.Platform;
using Microsoft.UI.Xaml;
using Microsoft.UI.Xaml.Controls;
+using WVisualTreeHelper = Microsoft.UI.Xaml.Media.VisualTreeHelper;
using WApp = Microsoft.UI.Xaml.Application;
using WBrush = Microsoft.UI.Xaml.Media.Brush;

@@ -30,7 +31,8 @@ namespace Microsoft.Maui.Controls.Handlers
SearchHandler? _currentSearchHandler;
IShellAppearanceElement? _shellAppearanceElement;
readonly HashSet _trackedShellSections = new();

  •   ItemsRepeater? _overflowMenuItemsHost;
    
  •   object? _lastSelectedNavigationItem;
    
  •   bool _restoringSelection;
    
      public ShellItemHandler() : base(Mapper, CommandMapper)
      {
    

@@ -78,11 +80,7 @@ namespace Microsoft.Maui.Controls.Handlers
base.ConnectHandler(platformView);

		if (mauiNavView is not null)
  •   	{
      		mauiNavView.SelectionChanged += OnNavigationTabChanged;
    
  •   		mauiNavView.OnApplyTemplateFinished += OnNavigationViewApplyTemplateFinished;
    
  •   		TrackOverflowMenuItems(mauiNavView.TopNavMenuItemsOverflowHost);
    
  •   	}
    
      	if (VirtualView.Parent is Shell shell)
      	{
    

@@ -97,7 +95,6 @@ namespace Microsoft.Maui.Controls.Handlers
if (platformView is MauiNavigationView mnv)
{
mnv.SelectionChanged -= OnNavigationTabChanged;

  •   		mnv.OnApplyTemplateFinished -= OnNavigationViewApplyTemplateFinished;
      		if (mnv.AutoSuggestBox is { } autoSuggestBox)
      		{
      			autoSuggestBox.TextChanged -= OnSearchBoxTextChanged;
    

@@ -110,7 +107,6 @@ namespace Microsoft.Maui.Controls.Handlers
}

		platformView.Loaded -= OnNavigationViewLoaded;
  •   	TrackOverflowMenuItems(null);
    
      	if (_currentShellSection != null)
      		_currentShellSection.PropertyChanged -= OnCurrentShellSectionPropertyChanged;
    

@@ -183,10 +179,29 @@ namespace Microsoft.Maui.Controls.Handlers

	private void OnNavigationTabChanged(NavigationView sender, NavigationViewSelectionChangedEventArgs args)
	{
  •   	if (_restoringSelection)
    
  •   	{
    
  •   		_restoringSelection = false;
    
  •   		return;
    
  •   	}
    
  •   	if (args.SelectedItem == null)
      		return;
    
      	var selectedItem = (NavigationViewItemViewModel)args.SelectedItem;
    
  •   	if (ShouldPreserveSelectionForOverflowParent(sender, args, selectedItem))
    
  •   	{
    
  •   		if (args.SelectedItemContainer is NavigationViewItem selectedContainer)
    
  •   			selectedContainer.IsExpanded = true;
    
  •   		if (_lastSelectedNavigationItem is not null && !ReferenceEquals(sender.SelectedItem, _lastSelectedNavigationItem))
    
  •   		{
    
  •   			_restoringSelection = true;
    
  •   			sender.SelectedItem = _lastSelectedNavigationItem;
    
  •   		}
    
  •   		return;
    
  •   	}
    
      	if (selectedItem.Data is ShellSection shellSection && VirtualView.Parent is Shell shell)
      	{
    

@@ -236,70 +251,45 @@ namespace Microsoft.Maui.Controls.Handlers
parentShell.CurrentItem = shellContent;
}
}

  •   }
    
  •   void OnNavigationViewApplyTemplateFinished(object? sender, EventArgs e)
    
  •   {
    
  •   	if (sender is MauiNavigationView mauiNavView)
    
  •   		TrackOverflowMenuItems(mauiNavView.TopNavMenuItemsOverflowHost);
    
  •   	_lastSelectedNavigationItem = sender.SelectedItem;
      }
    
  •   void TrackOverflowMenuItems(ItemsRepeater? overflowMenuItemsHost)
    
  •   static bool ShouldPreserveSelectionForOverflowParent(
    
  •   	NavigationView navView,
    
  •   	NavigationViewSelectionChangedEventArgs args,
    
  •   	NavigationViewItemViewModel selectedItem)
      {
    
  •   	if (_overflowMenuItemsHost == overflowMenuItemsHost)
    
  •   		return;
    
  •   	if (selectedItem.MenuItemsSource == null || selectedItem.MenuItemsSource.Count == 0)
    
  •   		return false;
    
  •   	if (_overflowMenuItemsHost is not null)
    
  •   	{
    
  •   		_overflowMenuItemsHost.ElementPrepared -= OnOverflowMenuItemPrepared;
    
  •   		_overflowMenuItemsHost.ElementClearing -= OnOverflowMenuItemClearing;
    
  •   	}
    
  •   	if (args.SelectedItemContainer is not NavigationViewItem selectedContainer)
    
  •   		return false;
    
  •   	_overflowMenuItemsHost = overflowMenuItemsHost;
    
  •   	if (navView.XamlRoot is null)
    
  •   		return false;
    
  •   	if (_overflowMenuItemsHost is not null)
    
  •   	foreach (var popup in WVisualTreeHelper.GetOpenPopupsForXamlRoot(navView.XamlRoot))
      	{
    
  •   		_overflowMenuItemsHost.ElementPrepared += OnOverflowMenuItemPrepared;
    
  •   		_overflowMenuItemsHost.ElementClearing += OnOverflowMenuItemClearing;
    
  •   		if (popup?.Child is DependencyObject popupChild && IsDescendantOf(selectedContainer, popupChild))
    
  •   			return true;
      	}
    
  •   }
    
  •   static void OnOverflowMenuItemPrepared(ItemsRepeater sender, ItemsRepeaterElementPreparedEventArgs args)
    
  •   {
    
  •   	if (args.Element is NavigationViewItem item)
    
  •   		UpdateOverflowMenuItemSelectsOnInvoked(item);
    
  •   }
    
  •   static void OnOverflowMenuItemClearing(ItemsRepeater sender, ItemsRepeaterElementClearingEventArgs args)
    
  •   {
    
  •   	// Containers are reused between the overflow menu and the tab bar
    
  •   	if (args.Element is NavigationViewItem item)
    
  •   		item.ClearValue(NavigationViewItem.SelectsOnInvokedProperty);
    
  •   	return false;
      }
    
  •   // MapMenuItems can add or remove the sub tabs of a tab that already sits in the overflow menu
    
  •   void UpdateOverflowMenuItems()
    
  •   static bool IsDescendantOf(DependencyObject child, DependencyObject ancestor)
      {
    
  •   	if (_overflowMenuItemsHost?.ItemsSourceView is not { } overflowMenuItems)
    
  •   		return;
    
  •   	for (int i = 0; i < overflowMenuItems.Count; i++)
    
  •   	DependencyObject? current = child;
    
  •   	while (current != null)
      	{
    
  •   		if (_overflowMenuItemsHost.TryGetElement(i) is NavigationViewItem item)
    
  •   			UpdateOverflowMenuItemSelectsOnInvoked(item);
    
  •   		if (ReferenceEquals(current, ancestor))
    
  •   			return true;
    
  •   		current = WVisualTreeHelper.GetParent(current);
      	}
    
  •   }
    
  •   // WinUI can't select a data bound item that has children while it sits in the overflow menu of the
    
  •   // top tab bar. It doesn't find the item's container there, and expanding the item right after the
    
  •   // click fails with E_BOUNDS (https://github.com/dotnet/maui/issues/37985). So in the overflow menu
    
  •   // a tab with sub tabs only expands, and the navigation happens when one of its sub tabs is picked.
    
  •   static void UpdateOverflowMenuItemSelectsOnInvoked(NavigationViewItem item)
    
  •   {
    
  •   	if (item.DataContext is NavigationViewItemViewModel { MenuItemsSource: not null })
    
  •   		item.SelectsOnInvoked = false;
    
  •   	else
    
  •   		item.ClearValue(NavigationViewItem.SelectsOnInvokedProperty);
    
  •   	return false;
      }
    
      internal void MapMenuItems()
    

@@ -391,11 +381,11 @@ namespace Microsoft.Maui.Controls.Handlers
}
});

  •   	UpdateOverflowMenuItems();
    
  •   	if (PlatformView is NavigationView navView && navView.SelectedItem != selectedItem)
      		navView.SelectedItem = selectedItem;
    
  •   	_lastSelectedNavigationItem = selectedItem;
    
  •   	UpdateValue(Shell.TabBarIsVisibleProperty.PropertyName);
      }
    

diff --git a/src/Core/src/Platform/Windows/MauiNavigationView.cs b/src/Core/src/Platform/Windows/MauiNavigationView.cs
index a665f0b064..a1a56fbe46 100644
--- a/src/Core/src/Platform/Windows/MauiNavigationView.cs
+++ b/src/Core/src/Platform/Windows/MauiNavigationView.cs
@@ -16,7 +16,6 @@ namespace Microsoft.Maui.Platform

	internal StackPanel? TopNavArea { get; private set; }
	internal ItemsRepeater? TopNavMenuItemsHost { get; private set; }
  •   internal ItemsRepeater? TopNavMenuItemsOverflowHost { get; private set; }
      internal Grid? PaneContentGrid { get; private set; }
      internal event EventHandler? OnApplyTemplateFinished;
      internal SplitView? RootSplitView { get; private set; }
    

@@ -84,7 +83,6 @@ namespace Microsoft.Maui.Platform
RootSplitView = (SplitView)GetTemplateChild("RootSplitView");
TopNavArea = (StackPanel)GetTemplateChild("TopNavArea");
TopNavMenuItemsHost = (ItemsRepeater)GetTemplateChild("TopNavMenuItemsHost");

  •   	TopNavMenuItemsOverflowHost = (ItemsRepeater)GetTemplateChild("TopNavMenuItemsOverflowHost");
      	ContentPaneTopPadding = (Grid)GetTemplateChild("ContentPaneTopPadding");
      	PaneToggleButtonGrid = (Grid)GetTemplateChild("PaneToggleButtonGrid");
      	ButtonHolderGrid = (Grid)GetTemplateChild("ButtonHolderGrid");
    

`

Restoration outcome:
` ext
No baseline state found. Nothing to restore.

�[32;1mName �[0m�[32;1m Value�[0m
�[32;1m---- �[0m �[32;1m-----�[0m
Restored False
Message No baseline state found

`

Analysis artifact:

Analysis

Result: Fail

What happened: The required Windows Shell device-test command executed and the three mandatory methods all failed.

Why it failed: The candidate relied on selection restoration when overflow parent tabs are invoked. The tests fail on assertions that overflow-parent tabs with child items must already behave as non-selecting entries; they still appeared selecting, so this mechanism did not satisfy the regression behavior contract.

Self-review: 0 findings (clean).

Insights: Invocation-time selection rollback is too late for this regression suite; behavior must be correct before click handling is validated.

Exact test result(s):
test-execution-summaryTests run: 72 Passed: 69 Inconclusive: 0 Failed: 3 Ignored: 0
Tests run: 523 Passed: 69 Inconclusive: 0 Failed: 3 Ignored: 451 Skipped: 0
Passed: 0
Failed: 3
Scoped to method(s): TabWithSeveralShellContentsOpensFromTabBarOverflowMenu;TabBarOverflowMenuTabFollowsItsShellContentChanges;TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu
Failed test(s): Microsoft.Maui.DeviceTests.ShellTests.TabBarOverflowMenuTabFollowsItsShellContentChanges; Microsoft.Maui.DeviceTests.ShellTests.TabWithSeveralShellContentsOpensFromTabBarOverflowMenu; Microsoft.Maui.DeviceTests.ShellTests.TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu
Tests completed with exit code: 1


Candidate 2 Try-Fix Report (PR #39412)

Approach: Concrete top-level NavigationViewItem identity. Shell's top-level view models were wrapped in concrete NavigationViewItem instances while child items remained data-bound. The concrete items retained the existing view models as DataContext, used bindings for the existing presentation properties, and were unwrapped by the handler for MAUI navigation.

Prior approaches avoided: The existing PR exposes and tracks TopNavMenuItemsOverflowHost, then mutates and clears NavigationViewItem.SelectsOnInvoked on recycled overflow-repeater containers. Candidate 1 used a SelectionChanged-time overflow-parent veto and restored the previous selection; its recorded result was 0/3 mandatory tests passed because WinUI had already entered the invalid selection/expansion transition. Candidate 2 did not expose or track the overflow repeater, did not mutate or clear SelectsOnInvoked, and did not restore selection in any callback.

Mechanism-level difference: Candidate 2 targeted the data-item/container identity mismatch that makes WinUI fail to resolve a generated overflow container. Supplying each top-level item as its own concrete container was intended to remove that lookup from WinUI's top-navigation selection path, allowing native overflow realization to classify a hierarchical item before invocation. This changes what WinUI selects, rather than repairing container state or rolling selection back after it occurs.

Files changed:

  • src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs

Result: Fail

Exact test command (both permitted runs):

pwsh .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Controls -Platform windows -TestFilter "Category=Shell" -IncludeClasses "Microsoft.Maui.DeviceTests.ShellTests" -IncludeMethods "TabWithSeveralShellContentsOpensFromTabBarOverflowMenu;TabBarOverflowMenuTabFollowsItsShellContentChanges;TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu"

Exact test result(s):

  1. Initial run: build failed with exit code 1 before tests executed. Candidate errors were CS1503 at ShellItemHandler.Windows.cs(352-357) (Microsoft.Maui.Controls.Binding could not be passed as Microsoft.UI.Xaml.Data.BindingBase) and CS0029 at line 362. No mandatory method executed.
  2. One focused correction aliased the WinUI binding types. The single permitted retest again failed before tests executed with exit code 1: ShellItemHandler.Windows.cs(363,19): error CS0029: Cannot implicitly convert type 'string' to 'Microsoft.UI.Xaml.PropertyPath'. The remaining XAML compiler errors were cascading from the Controls build failure. No third correction or test was run.

Mandatory regression scope outcome:

  • TabWithSeveralShellContentsOpensFromTabBarOverflowMenu: not executed (build failed)
  • TabBarOverflowMenuTabFollowsItsShellContentChanges: not executed (build failed)
  • TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu: not executed (build failed)

Failure analysis: The concrete-container mechanism was never reached because the candidate's programmatic WinUI binding construction did not compile after the one allowed correction. Consequently this attempt neither proves nor disproves the runtime identity hypothesis. Under the explicit one-correction/retest limit, the honest outcome is Fail rather than expanding scope or running a third test.

Self-review count: 1 major finding. The refreshed final-diff review records that WBinding.Path requires a PropertyPath; the string assignment causes CS0029 and blocks compilation.

Captured diff:

diff --git a/src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs b/src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs
index 05d6a0e61d..00dafcdddc 100644
--- a/src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs
+++ b/src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Windows.cs
@@ -7,6 +7,8 @@ using Microsoft.Maui.Platform;
 using Microsoft.UI.Xaml;
 using Microsoft.UI.Xaml.Controls;
 using WApp = Microsoft.UI.Xaml.Application;
+using WBinding = Microsoft.UI.Xaml.Data.Binding;
+using WBindingMode = Microsoft.UI.Xaml.Data.BindingMode;
 using WBrush = Microsoft.UI.Xaml.Media.Brush;
 
 namespace Microsoft.Maui.Controls.Handlers
@@ -26,6 +28,7 @@ namespace Microsoft.Maui.Controls.Handlers
 
 		ShellSectionHandler? _shellSectionHandler;
 		ObservableCollection<NavigationViewItemViewModel> _mainLevelTabs;
+		ObservableCollection<NavigationViewItem> _mainLevelTabItems;
 		ShellItem? _shellItem;
 		SearchHandler? _currentSearchHandler;
 		IShellAppearanceElement? _shellAppearanceElement;
@@ -34,6 +37,7 @@ namespace Microsoft.Maui.Controls.Handlers
 		public ShellItemHandler() : base(Mapper, CommandMapper)
 		{
 			_mainLevelTabs = new ObservableCollection<NavigationViewItemViewModel>();
+			_mainLevelTabItems = new ObservableCollection<NavigationViewItem>();
 		}
 
 		protected override FrameworkElement CreatePlatformElement()
@@ -61,7 +65,7 @@ namespace Microsoft.Maui.Controls.Handlers
 				mauiNavView.IsSettingsVisible = false;
 				mauiNavView.IsPaneToggleButtonVisible = false;
 				mauiNavView.MenuItemTemplate = (UI.Xaml.DataTemplate)WApp.Current.Resources["TabBarNavigationViewMenuItem"];
-				mauiNavView.MenuItemsSource = _mainLevelTabs;
+				mauiNavView.MenuItemsSource = _mainLevelTabItems;
 
 				platformView.SetApplicationResource("NavigationViewMinimalHeaderMargin", null);
 				platformView.SetApplicationResource("NavigationViewHeaderMargin", null);
@@ -179,16 +183,18 @@ namespace Microsoft.Maui.Controls.Handlers
 			if (args.SelectedItem == null)
 				return;
 
-			var selectedItem = (NavigationViewItemViewModel)args.SelectedItem;
+			var selectedItem = GetViewModel(args.SelectedItem);
+			if (selectedItem is null)
+				return;
 
 			if (selectedItem.Data is ShellSection shellSection && VirtualView.Parent is Shell shell)
 			{
-				NavigationViewItemViewModel? currentItem = null;
-				foreach (var item in _mainLevelTabs)
+				NavigationViewItem? currentItem = null;
+				for (int i = 0; i < _mainLevelTabs.Count; i++)
 				{
-					if (shell.CurrentItem?.CurrentItem is not null && item.Data == shell.CurrentItem.CurrentItem)
+					if (shell.CurrentItem?.CurrentItem is not null && _mainLevelTabs[i].Data == shell.CurrentItem.CurrentItem)
 					{
-						currentItem = item;
+						currentItem = _mainLevelTabItems[i];
 						break;
 					}
 				}
@@ -320,12 +326,56 @@ namespace Microsoft.Maui.Controls.Handlers
 				}
 			});
 
+			SyncMainLevelTabItems();
+			selectedItem = GetPlatformItem(selectedItem);
+
 			if (PlatformView is NavigationView navView && navView.SelectedItem != selectedItem)
 				navView.SelectedItem = selectedItem;
 
 			UpdateValue(Shell.TabBarIsVisibleProperty.PropertyName);
 		}
 
+		void SyncMainLevelTabItems()
+		{
+			while (_mainLevelTabItems.Count < _mainLevelTabs.Count)
+				_mainLevelTabItems.Add(CreateNavigationViewItem());
+
+			while (_mainLevelTabItems.Count > _mainLevelTabs.Count)
+				_mainLevelTabItems.RemoveAt(_mainLevelTabItems.Count - 1);
+
+			for (int i = 0; i < _mainLevelTabs.Count; i++)
+				_mainLevelTabItems[i].DataContext = _mainLevelTabs[i];
+		}
+
+		static NavigationViewItem CreateNavigationViewItem()
+		{
+			var item = new NavigationViewItem();
+			item.SetBinding(ContentControl.ContentProperty, CreateBinding(nameof(NavigationViewItemViewModel.Content)));
+			item.SetBinding(Control.BackgroundProperty, CreateBinding(nameof(NavigationViewItemViewModel.Background)));
+			item.SetBinding(Control.IsEnabledProperty, CreateBinding(nameof(NavigationViewItemViewModel.IsEnabled)));
+			item.SetBinding(NavigationViewItem.IsSelectedProperty, CreateBinding(nameof(NavigationViewItemViewModel.IsSelected), WBindingMode.TwoWay));
+			item.SetBinding(NavigationViewItem.MenuItemsSourceProperty, CreateBinding(nameof(NavigationViewItemViewModel.MenuItemsSource)));
+			item.SetBinding(NavigationViewItem.IconProperty, CreateBinding(nameof(NavigationViewItemViewModel.Icon)));
+			return item;
+		}
+
+		static WBinding CreateBinding(string propertyName, WBindingMode mode = WBindingMode.OneWay) =>
+			new() { Path = propertyName, Mode = mode };
+
+		static NavigationViewItemViewModel? GetViewModel(object? item) =>
+			item as NavigationViewItemViewModel ?? (item as NavigationViewItem)?.DataContext as NavigationViewItemViewModel;
+
+		object? GetPlatformItem(object? item)
+		{
+			for (int i = 0; i < _mainLevelTabs.Count; i++)
+			{
+				if (ReferenceEquals(_mainLevelTabs[i], item))
+					return _mainLevelTabItems[i];
+			}
+
+			return item;
+		}
+
 		void UpdateSearchHandler()
 		{
 			if (VirtualView.Parent is not Shell shell)
@@ -670,13 +720,14 @@ namespace Microsoft.Maui.Controls.Handlers
 				return;
 
 			var currentItem = VirtualView.CurrentItem.CurrentItem;
-			NavigationViewItemViewModel? navigationViewItemViewModel = null;
+			object? navigationViewItem = null;
 
-			foreach (var item in _mainLevelTabs)
+			for (int i = 0; i < _mainLevelTabs.Count; i++)
 			{
+				var item = _mainLevelTabs[i];
 				if (item.Data == currentItem)
 				{
-					navigationViewItemViewModel = item;
+					navigationViewItem = _mainLevelTabItems[i];
 					break;
 				}
 
@@ -686,22 +737,22 @@ namespace Microsoft.Maui.Controls.Handlers
 					{
 						if (subItem.Data == currentItem)
 						{
-							navigationViewItemViewModel = subItem;
+							navigationViewItem = subItem;
 							break;
 						}
 
 					}
 				}
 
-				if (navigationViewItemViewModel != null)
+				if (navigationViewItem != null)
 					break;
 			}
 
-			if (navigationViewItemViewModel != null &&
+			if (navigationViewItem != null &&
 				PlatformView is NavigationView navView &&
-				navView.SelectedItem != navigationViewItemViewModel)
+				navView.SelectedItem != navigationViewItem)
 			{
-				navView.SelectedItem = navigationViewItemViewModel;
+				navView.SelectedItem = navigationViewItem;
 			}
 		}

Artifacts: CustomAgentLogsTmp/PRState/39412/PRAgent/try-fix/attempt-2/ contains baseline.log, approach.md, both run logs (test-output-1.log and test-output.log), reviewer-findings.json, reviewer-findings.diff, result.txt, fix.diff, and analysis.md. The required artifact gate passed.

Restoration outcome: pwsh .github/scripts/EstablishBrokenBaseline.ps1 -Restore reported Restored True for both baseline target files. The candidate diff exists only in fix.diff; production files were restored before this report was written.


🏁 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Winning candidate: pr-plus-reviewer

The submitted production fix is sound and already passes the trusted Gate, but the reviewer-refined candidate is the strongest version because it preserves the PR implementation and extends an existing required device test to exercise the otherwise-uncovered template-reapplication path. That refined candidate passed all three targeted Windows Shell regression methods in the prescribed sandbox. Because pr-plus-reviewer wins rather than the raw pr, the submitted PR should take the focused test change before approval.

Candidate Comparison

Rank Candidate Approach Validation Expert assessment Result
1 pr-plus-reviewer Keep the submitted overflow-ItemsRepeater/SelectsOnInvoked production fix and force a connected NavigationView template reapply inside the existing container-reuse test PASS — 3/3 targeted methods, exit code 0 No production-code finding; closes the reviewer's only identified coverage gap Winner
2 pr Track the current top-overflow host, disable selection for overflow parents with children, clear recycled-container state, and refresh realized containers after menu changes Trusted Gate: PASS — 3/3; tests fail without fix and pass with fix No actionable inline findings ([]); lifecycle wiring and cleanup are coherent Correct, but has less lifecycle coverage than the winner
3 try-fix-1 Let invocation select the overflow parent, then detect it in SelectionChanged, expand it, and restore the previous selection FAIL — 0/3; all mandatory tests reported that the overflow parent was still selecting Selection rollback occurs after the pre-click state needed to prevent WinUI's invalid transition Rejected
4 try-fix-2 Replace data-bound top-level items with concrete NavigationViewItem containers to avoid WinUI's data-item/container lookup FAIL — build did not complete on either permitted run; final CS0029 assigning string to PropertyPath Runtime hypothesis remained untested and the candidate was not compilable Rejected

Expert Review of the Submitted Fix

The single maui-expert-reviewer pass found no actionable defect on an added or modified line. In particular:

  • ConnectHandler/DisconnectHandler subscribe and unsubscribe symmetrically.
  • Template reapplication swaps tracking from the old overflow ItemsRepeater to the new one.
  • ElementClearing removes the local SelectsOnInvoked value before container reuse.
  • MapMenuItems refreshes already-realized overflow containers when a tab gains or loses children.
  • The change is Windows-only, has no new static/shared state, and does not add a startup path.

The expert returned NEEDS_DISCUSSION with low confidence only because its unauthenticated GitHub client could not retrieve required-check and prior-review surfaces. It reported no code finding. Anonymous follow-up retrieval found six current check runs: four succeeded, one was skipped, and Build Insights remained in progress. Required-check classification therefore remains undetermined, while the trusted Gate supplied to this phase confirms that the three targeted tests fail without the fix and pass with the raw PR.

Prior Review Reconciliation

An older review against PR commit 7478a41324e60113dcbacd8e505582f866eef618 raised one error-level dynamic-state defect and three related test-coverage concerns. None remains unresolved in the submitted HEAD:

Prior concern Status Current evidence
SelectsOnInvoked stayed stale when a realized overflow tab gained or lost children ✅ Fixed UpdateOverflowMenuItems runs after MapMenuItems; TabBarOverflowMenuTabFollowsItsShellContentChanges verifies both transitions.
A single-content top-level overflow tab was not exercised ✅ Fixed The first new test invokes a single-content overflow tab and verifies direct navigation.
Normal parent selection after moving from overflow to the visible bar was not exercised ✅ Fixed The third new test widens the bar, checks the visible parent is selectable, invokes it, and verifies navigation.

The older review summary also described a failed refinement that did not include the current UpdateOverflowMenuItems implementation. It is obsolete relative to the raw diff evaluated in this run.

Why pr-plus-reviewer Wins

The raw PR and pr-plus-reviewer share the same production implementation, so both avoid the crash with the same minimal platform-specific mechanism. The differentiator is regression prevention: the handler explicitly listens for OnApplyTemplateFinished, but the raw tests did not force the overflow host to be replaced while the handler remained connected. The consolidated reviewer patch extends the existing “leaving overflow” test to:

  1. close the current overflow menu;
  2. clear and restore the NavigationView template while the handler stays connected;
  3. reacquire the replacement overflow button/container;
  4. verify the nested parent still has SelectsOnInvoked == false; and
  5. continue verifying selection after the item moves back to the visible top bar.

This directly validates the new host-rebinding logic without changing product behavior or adding a broader test run.

Validation Evidence

The candidate sandbox identity was verified as:

D:\a\_temp\pr-39412-pr-plus-reviewer

at immutable baseline:

cfc8d23be2181266d7f738167ccda62919cbd3fe

Exactly one targeted validation command was run from that root:

pwsh .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Controls -Platform windows -TestFilter "Category=Shell" -IncludeClasses "Microsoft.Maui.DeviceTests.ShellTests" -IncludeMethods "TabWithSeveralShellContentsOpensFromTabBarOverflowMenu;TabBarOverflowMenuTabFollowsItsShellContentChanges;TabWithSeveralShellContentsIsSelectedAgainAfterLeavingTabBarOverflowMenu"

Result:

Passed: 3
Failed: 0
Skipped: 0
ExitCode: 0
Result file: D:\a\_temp\pr-39412-pr-plus-reviewer\artifacts\log\TestResults-Microsoft_Maui_Controls_DeviceTests_Shell.xml

No correction/retest loop and no full suite were run.

Required Change

Apply pr-plus-reviewer/reviewer.patch, which contains only the focused test hardening in:

src/Controls/tests/DeviceTests/Elements/Shell/ShellTabBarTests.Windows.cs

The complete candidate diff against the PR base is preserved in pr-plus-reviewer/candidate.patch.


📱 UI Tests — Shell,ViewBaseTests

Detected UI test categories: Shell,ViewBaseTests

[!WARNING]
No UI test results were produced for the detected categories. The PR build itself was
fine — the deep UI stage was skipped or interrupted on infrastructure (the
merge-for-testing step, emulator/simulator boot, or an Appium hang), not by this PR's code.
This is usually transient; the review re-runs on new commits (a maintainer can also re-run it).


🧭 Next Steps — reviewer changes required

The reviewer-enhanced candidate identified changes that are not yet in the submitted PR.

Why: The submitted production fix is sound, but pr-plus-reviewer adds focused coverage for replacing the overflow host during a connected NavigationView template reapply. It preserved the production approach and passed all 3 targeted Windows Shell regression methods, while both independent try-fix candidates failed.

Address the actionable findings in this review before merging.

@MauiBot MauiBot added s/agent-gate-passed AI verified tests catch the bug (fail without fix, pass with fix) s/agent-fix-win AI found a better alternative fix than the PR and removed s/agent-review-in-progress AI review is currently running for this PR labels Oct 19, 2026
Applying the NavigationView template again creates a new overflow menu, the new test checks that the handler follows it.
@Empiree

Empiree commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@kubaflo thanks for running it again. This time the review had no findings on the code, only the reviewer candidate with an extra template check in the test.

I added that check, but as its own test TabWithSeveralShellContentsOnlyExpandsInNewTabBarOverflowMenuAfterTemplateIsAppliedAgain. It opens the overflow menu, applies the NavigationView template again and checks that Nested in the new overflow menu only expands. I didnt put it into the leaving overflow test like the candidate did. It looks like after the template is applied again WinUI stops reusing the overflow container for the bar, so that test passes even without the reset in ElementClearing, and the reset is what it was added for.

Checked each part on my side. Without the reset only the leaving overflow test fails, without following the new template only the new test fails, with the full fix all four pass.

@github-actions github-actions Bot added the s/agent-review-in-progress AI review is currently running for this PR label Oct 20, 2026
@kubaflo

This comment has been minimized.

@azure-pipelines

This comment has been minimized.

@kubaflo

This comment has been minimized.

@github-actions

This comment has been minimized.

@kubaflo

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Tests Failure Analysis

@Empiree — test-failure analysis for commit e930ff5.

Scope CI failures Commit e930ff5


🧪 CI Analysis — click to expand
📊 maui-pr

No failures found.


🧪 maui-pr-devicetests

No failures found.


🧪 maui-pr-uitests

All five main history samples are partial; unmatched failures remain unresolved.


🧭 Follow-up — actions and refresh

Next action: Inspect the MenuBar click target and unmatched screenshots; restore provisioning diagnostics and compare identical-setup outcomes.

Maintainers: comment /review tests to refresh this report.

@kubaflo
kubaflo changed the base branch from main to inflight/current October 21, 2026 11:23
Preserve the overflow-menu regression tests and helpers alongside the independent dynamically-added disabled-tab appearance test.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@kubaflo

kubaflo commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Resolved and pushed the conflict with inflight/current in merge commit 7219dc7744, on the original fix/windows-shell-overflow-subtabs branch.

The only conflict was overlapping additions in ShellTabBarTests.Windows.cs. The resolution preserves all four overflow-menu regression tests and their helpers, plus the target branch's independently added disabled-tab appearance test and helpers. ShellItemHandler.Windows.cs merged automatically, retaining both the overflow fix and upstream appearance changes; MauiNavigationView.cs is unchanged from the PR head.

Validation: scoped dotnet format whitespace verification and git diff --check passed for the resolved file. The managed Shell unit-test assembly compiled; the initial test run could not start because the repo-local SDK has only the .NET 11 runtime. Rerunning with explicit DOTNET_ROLL_FORWARD=Major passed all 93 selected Shell tests. These managed tests do not compile or execute the Windows-specific code. Native Windows builds and device tests were not run on this macOS host and remain for Windows CI. No UI automation or XCTest was launched.

@kubaflo
kubaflo merged commit c47eea0 into dotnet:inflight/current Oct 21, 2026
2 of 3 checks passed
kubaflo added a commit that referenced this pull request Oct 21, 2026
…bBar overflow menu (#39412)

<!-- 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!

### Description of Change

On Windows, clicking a Shell tab with more than one ShellContent in the
overflow menu of the top tab bar (the ... button) crashed the app with
E_BOUNDS (invalid vector subscript).

The crash is inside WinUI NavigationView. From what I see in the WinUI
code, in Top mode it cant find the container of a data bound item that
sits in the overflow menu, so the clicked item never gets IsSelected,
only IsChildSelected, and when it expands right after the click WinUI
reads past the end of the selected index path. MAUI always fills the tab
bar through MenuItemsSource, so every tab with sub tabs runs into it. A
plain WinUI NavigationView with MenuItemsSource and a similar item
template crashes the same way.

Fixing up the selection state after the click only helps for the first
click, other overflow bugs in WinUI show up after that (for example
picking a sub tab from the overflow menu and closing the menu later
crashes again). So now ShellItemHandler sets SelectsOnInvoked to false
on overflow menu containers that have sub tabs, and clears it when the
container leaves the overflow menu. MapMenuItems also updates the items
that are already in the overflow menu, because a tab can get or lose sub
tabs while it sits there. A click on such a tab just expands it, picking
one of its sub tabs navigates like before and WinUI moves the tab into
the bar. Tabs in the bar work like before.

There are four device tests. The first one opens a tab with one
ShellContent from the overflow menu, then clicks the tab with sub tabs,
picks Sub B and checks the navigation. The second one adds and removes a
sub tab of a tab that already sits in the open overflow menu and checks
the item follows. The third one moves a tab with sub tabs from the
overflow menu back into the bar and checks it gets selected there like
before. The fourth one applies the NavigationView template again and
checks the new overflow menu behaves the same. Without the fix the click
makes WinUI kill the whole test app, so the tests check SelectsOnInvoked
before clicking to fail in a normal way.

### Issues Fixed

Fixes #37985

---------

Co-authored-by: Empiree <Empiree@users.noreply.github.com>
Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
kubaflo added a commit that referenced this pull request Oct 25, 2026
…bBar overflow menu (#39412)

<!-- 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!

### Description of Change

On Windows, clicking a Shell tab with more than one ShellContent in the
overflow menu of the top tab bar (the ... button) crashed the app with
E_BOUNDS (invalid vector subscript).

The crash is inside WinUI NavigationView. From what I see in the WinUI
code, in Top mode it cant find the container of a data bound item that
sits in the overflow menu, so the clicked item never gets IsSelected,
only IsChildSelected, and when it expands right after the click WinUI
reads past the end of the selected index path. MAUI always fills the tab
bar through MenuItemsSource, so every tab with sub tabs runs into it. A
plain WinUI NavigationView with MenuItemsSource and a similar item
template crashes the same way.

Fixing up the selection state after the click only helps for the first
click, other overflow bugs in WinUI show up after that (for example
picking a sub tab from the overflow menu and closing the menu later
crashes again). So now ShellItemHandler sets SelectsOnInvoked to false
on overflow menu containers that have sub tabs, and clears it when the
container leaves the overflow menu. MapMenuItems also updates the items
that are already in the overflow menu, because a tab can get or lose sub
tabs while it sits there. A click on such a tab just expands it, picking
one of its sub tabs navigates like before and WinUI moves the tab into
the bar. Tabs in the bar work like before.

There are four device tests. The first one opens a tab with one
ShellContent from the overflow menu, then clicks the tab with sub tabs,
picks Sub B and checks the navigation. The second one adds and removes a
sub tab of a tab that already sits in the open overflow menu and checks
the item follows. The third one moves a tab with sub tabs from the
overflow menu back into the bar and checks it gets selected there like
before. The fourth one applies the NavigationView template again and
checks the new overflow menu behaves the same. Without the fix the click
makes WinUI kill the whole test app, so the tests check SelectsOnInvoked
before clicking to fail in a normal way.

### Issues Fixed

Fixes #37985

---------

Co-authored-by: Empiree <Empiree@users.noreply.github.com>
Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
kubaflo added a commit that referenced this pull request Oct 26, 2026
…bBar overflow menu (#39412)

<!-- 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!

### Description of Change

On Windows, clicking a Shell tab with more than one ShellContent in the
overflow menu of the top tab bar (the ... button) crashed the app with
E_BOUNDS (invalid vector subscript).

The crash is inside WinUI NavigationView. From what I see in the WinUI
code, in Top mode it cant find the container of a data bound item that
sits in the overflow menu, so the clicked item never gets IsSelected,
only IsChildSelected, and when it expands right after the click WinUI
reads past the end of the selected index path. MAUI always fills the tab
bar through MenuItemsSource, so every tab with sub tabs runs into it. A
plain WinUI NavigationView with MenuItemsSource and a similar item
template crashes the same way.

Fixing up the selection state after the click only helps for the first
click, other overflow bugs in WinUI show up after that (for example
picking a sub tab from the overflow menu and closing the menu later
crashes again). So now ShellItemHandler sets SelectsOnInvoked to false
on overflow menu containers that have sub tabs, and clears it when the
container leaves the overflow menu. MapMenuItems also updates the items
that are already in the overflow menu, because a tab can get or lose sub
tabs while it sits there. A click on such a tab just expands it, picking
one of its sub tabs navigates like before and WinUI moves the tab into
the bar. Tabs in the bar work like before.

There are four device tests. The first one opens a tab with one
ShellContent from the overflow menu, then clicks the tab with sub tabs,
picks Sub B and checks the navigation. The second one adds and removes a
sub tab of a tab that already sits in the open overflow menu and checks
the item follows. The third one moves a tab with sub tabs from the
overflow menu back into the bar and checks it gets selected there like
before. The fourth one applies the NavigationView template again and
checks the new overflow menu behaves the same. Without the fix the click
makes WinUI kill the whole test app, so the tests check SelectsOnInvoked
before clicking to fail in a normal way.

### Issues Fixed

Fixes #37985

---------

Co-authored-by: Empiree <Empiree@users.noreply.github.com>
Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
github-actions Bot pushed a commit that referenced this pull request Oct 28, 2026
…bBar overflow menu (#39412)

<!-- 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!

### Description of Change

On Windows, clicking a Shell tab with more than one ShellContent in the
overflow menu of the top tab bar (the ... button) crashed the app with
E_BOUNDS (invalid vector subscript).

The crash is inside WinUI NavigationView. From what I see in the WinUI
code, in Top mode it cant find the container of a data bound item that
sits in the overflow menu, so the clicked item never gets IsSelected,
only IsChildSelected, and when it expands right after the click WinUI
reads past the end of the selected index path. MAUI always fills the tab
bar through MenuItemsSource, so every tab with sub tabs runs into it. A
plain WinUI NavigationView with MenuItemsSource and a similar item
template crashes the same way.

Fixing up the selection state after the click only helps for the first
click, other overflow bugs in WinUI show up after that (for example
picking a sub tab from the overflow menu and closing the menu later
crashes again). So now ShellItemHandler sets SelectsOnInvoked to false
on overflow menu containers that have sub tabs, and clears it when the
container leaves the overflow menu. MapMenuItems also updates the items
that are already in the overflow menu, because a tab can get or lose sub
tabs while it sits there. A click on such a tab just expands it, picking
one of its sub tabs navigates like before and WinUI moves the tab into
the bar. Tabs in the bar work like before.

There are four device tests. The first one opens a tab with one
ShellContent from the overflow menu, then clicks the tab with sub tabs,
picks Sub B and checks the navigation. The second one adds and removes a
sub tab of a tab that already sits in the open overflow menu and checks
the item follows. The third one moves a tab with sub tabs from the
overflow menu back into the bar and checks it gets selected there like
before. The fourth one applies the NavigationView template again and
checks the new overflow menu behaves the same. Without the fix the click
makes WinUI kill the whole test app, so the tests check SelectsOnInvoked
before clicking to fail in a normal way.

### Issues Fixed

Fixes #37985

---------

Co-authored-by: Empiree <Empiree@users.noreply.github.com>
Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community ✨ Community Contribution s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-fix-win AI found a better alternative fix than the PR s/agent-gate-passed AI verified tests catch the bug (fail without fix, pass with fix) s/agent-review-in-progress AI review is currently running for this PR s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Windows] Crash (E_BOUNDS / invalid vector subscript) when selecting a Tab with nested ShellContent from the TabBar overflow menu

4 participants