Skip to content

TabItem: fix StackOverflowException in OnPreviewGotKeyboardFocus #11680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
etvorun wants to merge 1 commit into
base: main
Choose a base branch
from
+117 −14
Open

TabItem: fix StackOverflowException in OnPreviewGotKeyboardFocus #11680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 27 additions & 14 deletions src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/TabItem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,11 @@ protected override void OnPreviewGotKeyboardFocus(KeyboardFocusChangedEventArgs
}
}

if (!e.Handled && GetBoolField(BoolField.SetFocusOnContent))
// Guard against re-entrancy: MoveFocus below can trigger a focus-changed
// cascade (e.g. a GotKeyboardFocus handler redirecting focus back to this
// TabItem), which would re-enter OnPreviewGotKeyboardFocus and overflow the stack.
if (!e.Handled && GetBoolField(BoolField.SetFocusOnContent)
&& !GetBoolField(BoolField.MovingFocusToContent))
{
TabControl parentTabControl = TabControlParent;
if (parentTabControl != null)
Expand All @@ -341,26 +345,34 @@ protected override void OnPreviewGotKeyboardFocus(KeyboardFocusChangedEventArgs
if (selectedContentPresenter != null)
{
parentTabControl.UpdateLayout(); // Wait for layout
bool success = selectedContentPresenter.MoveFocus(new TraversalRequest(FocusNavigationDirection.First));

// If we successfully move focus inside the content then don't set focus to the header
if (success)
SetBoolField(BoolField.MovingFocusToContent, true);
try
{
e.Handled = true;
bool success = selectedContentPresenter.MoveFocus(new TraversalRequest(FocusNavigationDirection.First));

// However, if the focus got switched to a different focus scope,
// mark the header as the one last focused in its focus scope. #8293
if (Keyboard.FocusedElement is DependencyObject focusedElement)
// If we successfully move focus inside the content then don't set focus to the header
if (success)
{
DependencyObject thisFocusScope = FocusManager.GetFocusScope(this);
if (thisFocusScope != null && Keyboard.FocusedElement is DependencyObject currentFocus)
e.Handled = true;

// However, if the focus got switched to a different focus scope,
// mark the header as the one last focused in its focus scope. #8293
if (Keyboard.FocusedElement is DependencyObject focusedElement)
{
DependencyObject currentFocusScope = FocusManager.GetFocusScope(currentFocus);
if (currentFocusScope != thisFocusScope && thisFocusScope != null)
FocusManager.SetFocusedElement(thisFocusScope, this);
DependencyObject thisFocusScope = FocusManager.GetFocusScope(this);
if (thisFocusScope != null && Keyboard.FocusedElement is DependencyObject currentFocus)
{
DependencyObject currentFocusScope = FocusManager.GetFocusScope(currentFocus);
if (currentFocusScope != thisFocusScope && thisFocusScope != null)
FocusManager.SetFocusedElement(thisFocusScope, this);
}
}
}
}
finally
{
SetBoolField(BoolField.MovingFocusToContent, false);
}
}
}
}
Expand Down Expand Up @@ -531,6 +543,7 @@ private enum BoolField
{
SetFocusOnContent = 0x10, // This flag determine if we want to set focus on active TabItem content
SettingFocus = 0x20, // This flag indicates that the TabItem is in the process of setting focus
MovingFocusToContent = 0x40, // Re-entrancy guard for MoveFocus in OnPreviewGotKeyboardFocus

// By default ListBoxItem is selectable
DefaultValue = 0,
Expand Down
90 changes: 90 additions & 0 deletions ...Microsoft.DotNet.Wpf/tests/UnitTests/PresentationFramework.Tests/Controls/TabItemTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Windows;
using System.Windows.Controls;
using System.Windows.Input;
using System.Windows.Threading;

namespace Wpf.UnitTests.Controls;

/// <summary>
/// Unit tests for <see cref="TabItem"/> class.
/// </summary>
public sealed class TabItemTests
{
/// <summary>
/// Verifies that TabItem.OnPreviewGotKeyboardFocus does not cause a
/// StackOverflowException when a GotKeyboardFocus handler on an ancestor
/// redirects keyboard focus back to the TabItem after MoveFocus has moved
/// focus into the tab's content.
///
/// The cycle is:
/// 1. TabItem.OnPreviewGotKeyboardFocus → MoveFocus(content) → content gets focus
/// 2. GotKeyboardFocus on content bubbles → ancestor handler → Keyboard.Focus(tabItem)
/// 3. TryChangeFocus → PreviewGotKeyboardFocus → back to step 1
///
/// The MovingFocusToContent re-entrancy guard breaks this cycle by skipping
/// MoveFocus when OnPreviewGotKeyboardFocus is already in progress.
/// </summary>
[WpfFact]
public void OnPreviewGotKeyboardFocus_NoStackOverflow_WhenGotKeyboardFocusRedirectsFocusToTabItem()
{
// Arrange: Window > TabControl with 2 tabs, each containing a focusable TextBox
Window window = new Window { Width = 400, Height = 300 };
TabControl tabControl = new TabControl();

TextBox textBox1 = new TextBox { Text = "Content1" };
TabItem tabItem1 = new TabItem { Header = "Tab1", Content = textBox1 };

TextBox textBox2 = new TextBox { Text = "Content2" };
TabItem tabItem2 = new TabItem { Header = "Tab2", Content = textBox2 };

tabControl.Items.Add(tabItem1);
tabControl.Items.Add(tabItem2);
window.Content = tabControl;
window.Show();

// Select Tab1 and put keyboard focus inside the TabControl
// (IsKeyboardFocusWithin must be true for TabControl.OnSelectionChanged to call SetFocus)
tabItem1.IsSelected = true;
tabControl.UpdateLayout();
textBox1.Focus();
Dispatcher.CurrentDispatcher.Invoke(() => { }, DispatcherPriority.Background);

int focusRedirectCount = 0;
const int safetyLimit = 20;

// Simulate an ancestor GotKeyboardFocus handler that redirects focus back
// to the TabItem whenever content receives focus. This pattern occurs when
// a hosting container tries to keep focus on the tab header element.
// Without the re-entrancy guard, this causes infinite recursion.
tabControl.AddHandler(Keyboard.GotKeyboardFocusEvent, new KeyboardFocusChangedEventHandler((_, e) =>
{
if (e.NewFocus == textBox2 && focusRedirectCount < safetyLimit)
{
focusRedirectCount++;
Keyboard.Focus(tabItem2);
}
}));

// Act: Switch to Tab2. This triggers:
// TabControl.OnSelectionChanged → TabItem2.SetFocus() (SetFocusOnContent=true)
// → Focus() → TryChangeFocus → PreviewGotKeyboardFocus
// → TabItem.OnPreviewGotKeyboardFocus → MoveFocus(content) → textBox2 gets focus
// → GotKeyboardFocus(textBox2) bubbles → our handler → Keyboard.Focus(tabItem2)
// → TryChangeFocus → PreviewGotKeyboardFocus → TabItem.OnPreviewGotKeyboardFocus
//
// Without the re-entrancy guard, this recurses until stack overflow.
// With the guard, the second entry skips MoveFocus and the cycle stops.
tabItem2.IsSelected = true;
Dispatcher.CurrentDispatcher.Invoke(() => { }, DispatcherPriority.Background);

// Assert: focus redirect should fire only a small number of times (not hundreds)
Assert.True(focusRedirectCount <= 5,
$"Focus was redirected {focusRedirectCount} times, suggesting the re-entrancy guard is not working. " +
"Expected ≤ 5 redirects.");

window.Close();
}
}
Loading