Skip to content

Don't enter a KeyboardNavigationMode.None group via its active element #22154

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
ethanstoner wants to merge 3 commits into
base: main
Choose a base branch
from
+124 −6
Open

Don't enter a KeyboardNavigationMode.None group via its active element #22154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
48307da
Add failing tests for TabNavigation None with an active element inside
ethanstoner Sep 6, 2026
97fd0b2
Don't enter a None group via its active element
ethanstoner Sep 6, 2026
dc931ac
Add a ListBox-level test for the reported navigation path
ethanstoner Sep 6, 2026
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
21 changes: 15 additions & 6 deletions src/Avalonia.Base/Input/Navigation/TabNavigation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,16 @@ internal static class TabNavigation
if (IsTabStop(container))
return container;

// Using ActiveElement if set
var activeElement = GetActiveElement(container);

@grokys grokys Sep 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This code comes directly from WPF:

https://github.com/dotnet/wpf/blob/360950b3ceb503d68bbb2fbc2ab9271e4cda90a5/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Input/KeyboardNavigation.cs#L2090-L2093

Does this issue also reproduce on WPF?

@ethanstoner ethanstoner Sep 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No, it doesn't reproduce on WPF, and the reason is that WPF gates the write rather than the read.

KeyboardNavigation.UpdateActiveElement only records an active element for Once groups:

// Update ActiveElement only if container has TabNavigation = Once
if (GetKeyNavigationMode(container) == KeyboardNavigationMode.Once)
{
    SetActiveElement(container, activeElement);
}

https://github.com/dotnet/wpf/blob/360950b3ceb503d68bbb2fbc2ab9271e4cda90a5/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Input/KeyboardNavigation.cs#L948-L960

That's the only write path: SetActiveElement is private with a single caller, and SetTabOnceActiveElement is only called from it. So in WPF a None container never has an active element, and the code you linked never gets the chance to follow one.

Avalonia records it unconditionally, in three places: ItemsControl.OnGotFocus (ItemsControl.cs:543), and the selection anchor in SelectingItemsControl.cs:539 and :971.

The selection path is the one in the report. The ListBox has an active element as soon as an item is selected, without ever being focused. That is also why the reporter's TreeView looked correct: it had no selected item, so nothing recorded one.

if (activeElement != null)
return GetNextTab(null, activeElement, true);
// Using ActiveElement if set. A None group never hands out an element
// inside itself, not even a remembered one: ItemsControl records the
// last focused child as the active element, so without this an
// ItemsControl with TabNavigation="None" is still entered.
if (tabbingType != KeyboardNavigationMode.None)
{
var activeElement = GetActiveElement(container);
if (activeElement != null)
return GetNextTab(null, activeElement, true);
}
}
else
{
Expand Down Expand Up @@ -95,8 +101,11 @@ internal static class TabNavigation

if (e == null)
{
// Using ActiveElement if set
var activeElement = GetActiveElement(container);

@grokys grokys Sep 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Again, this code is directly from WPF:

https://github.com/dotnet/wpf/blob/360950b3ceb503d68bbb2fbc2ab9271e4cda90a5/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Input/KeyboardNavigation.cs#L2266

I'd like to understand why we'd want to diverge from WPF.

@ethanstoner ethanstoner Sep 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed that matching WPF should be the default, so I tried WPF's approach first: gate the write the way UpdateActiveElement does and leave both of these hunks untouched.

It doesn't survive here. Avalonia uses TabOnceActiveElement for a second job that WPF doesn't give it, as the VirtualizingStackPanel focus anchor (VirtualizingStackPanel.cs:1140 and :1386).

Gating the write on Once broke that. Avalonia.Controls.UnitTests went from 3915 passed / 0 failed to 11 failures, all VirtualizingStackPanel focus tests (Focusing_Another_Element_Recycles_Original_Focus_Element, Focused_Element_Outside_Realized_Range_Is_Not_Arranged_In_Viewport and similar).

So the divergence is forced by that property carrying two responsibilities in Avalonia. The read side was where I could fix it without disturbing virtualization.

The cleaner long term option is to split the two concerns, a navigation active element and a separate virtualization anchor. Then Avalonia could take WPF's write gate verbatim and both of these hunks would go back to matching WPF exactly. That's a bigger change and your call, and I'm happy to do it that way if you'd prefer.

// Using ActiveElement if set, except for None groups, which never hand
// out an element inside themselves. See the matching note in GetNextTab.
var activeElement = tabbingType == KeyboardNavigationMode.None
? null
: GetActiveElement(container);
if (activeElement != null)
return GetPrevTab(null, activeElement, true);
else
Expand Down
70 changes: 70 additions & 0 deletions tests/Avalonia.Base.UnitTests/Input/KeyboardNavigationTests_Tab.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,76 @@ public void Next_None_Skips_Container()
Assert.Equal(next, result);
}

[Fact]
public void Next_None_Skips_Container_With_Active_Element_Inside()
{
StackPanel container;
Button current;
Button inside;
Button next;

var top = new StackPanel
{
[KeyboardNavigation.TabNavigationProperty] = KeyboardNavigationMode.Cycle,
Children =
{
(current = new Button { Name = "Button1" }),
(container = new StackPanel
{
[KeyboardNavigation.TabNavigationProperty] = KeyboardNavigationMode.None,
Children =
{
(inside = new Button { Name = "Button2" }),
new Button { Name = "Button3" },
}
}),
(next = new Button { Name = "Button4" }),
}
};

// ItemsControl.OnGotFocus records the last focused child as the active
// element, so a ListBox whose item has been focused has one set.
KeyboardNavigation.SetTabOnceActiveElement(container, inside);

var result = KeyboardNavigationHandler.GetNext(current, NavigationDirection.Next);

Assert.Equal(next, result);
}

[Fact]
public void Previous_None_Skips_Container_With_Active_Element_Inside()
{
StackPanel container;
Button current;
Button inside;
Button previous;

var top = new StackPanel
{
[KeyboardNavigation.TabNavigationProperty] = KeyboardNavigationMode.Cycle,
Children =
{
(previous = new Button { Name = "Button1" }),
(container = new StackPanel
{
[KeyboardNavigation.TabNavigationProperty] = KeyboardNavigationMode.None,
Children =
{
new Button { Name = "Button2" },
(inside = new Button { Name = "Button3" }),
}
}),
(current = new Button { Name = "Button4" }),
}
};

KeyboardNavigation.SetTabOnceActiveElement(container, inside);

var result = KeyboardNavigationHandler.GetNext(current, NavigationDirection.Previous);

Assert.Equal(previous, result);
}

[Fact]
public void Previous_Continue_Returns_Previous_Control_In_Container()
{
Expand Down
39 changes: 39 additions & 0 deletions tests/Avalonia.Controls.UnitTests/ListBoxTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,45 @@ private static FuncControlTemplate ScrollViewerTemplate()
});
}

[Fact]
public void TabNavigation_None_Is_Skipped_When_An_Item_Is_Selected()
{
using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface))
{
Button before;
Button after;

// Selecting an item records it as the tab-once active element, and the
// navigation code follows that without consulting the navigation mode.
var target = new ListBox
{
Template = ListBoxTemplate(),
ItemsSource = new[] { "Foo", "Bar" },
SelectedIndex = 0,
Width = 100,
Height = 100,
[KeyboardNavigation.TabNavigationProperty] = KeyboardNavigationMode.None,
};

var root = new TestRoot(new StackPanel
{
[KeyboardNavigation.TabNavigationProperty] = KeyboardNavigationMode.Cycle,
Children =
{
(before = new Button()),
target,
(after = new Button()),
}
});

root.LayoutManager.ExecuteInitialLayoutPass();

var result = KeyboardNavigationHandler.GetNext(before, NavigationDirection.Next);

Assert.Same(after, result);
}
}

private static void Prepare(ListBox target)
{
target.Width = target.Height = 100;
Expand Down