From 15c8cea75d58174507266b39467c3cd905825e44 Mon Sep 17 00:00:00 2001 From: Christoph Wille Date: Tue, 6 Oct 2026 07:41:55 +0200 Subject: [PATCH 1/8] Let Tab move the focus out of the decompiled text view AvaloniaEdit marks Tab handled while AcceptsTab is on, so the keyboard focus could never leave the read-only editor. WPF ILSpy removed the TabForward/TabBackward editing commands for the same reason. #4042 Assisted-by: Claude:claude-fable-5-1:Claude Code --- ILSpy.Tests/Editor/TabFocusNavigationTests.cs | 83 +++++++++++++++++++ ILSpy/TextView/DecompilerTextView.axaml.cs | 4 + 2 files changed, 87 insertions(+) create mode 100644 ILSpy.Tests/Editor/TabFocusNavigationTests.cs diff --git a/ILSpy.Tests/Editor/TabFocusNavigationTests.cs b/ILSpy.Tests/Editor/TabFocusNavigationTests.cs new file mode 100644 index 0000000000..4e2b96d76b --- /dev/null +++ b/ILSpy.Tests/Editor/TabFocusNavigationTests.cs @@ -0,0 +1,83 @@ +// Copyright (c) 2026 Christoph Wille +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + + +using System.Linq; +using System.Threading.Tasks; + +using Avalonia.Controls; +using Avalonia.Headless; +using Avalonia.Headless.NUnit; +using Avalonia.Input; +using Avalonia.VisualTree; + +using AvaloniaEdit.Editing; + +using AwesomeAssertions; + +using ICSharpCode.ILSpy.TextView; +using ICSharpCode.ILSpy.TreeNodes; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.Editor; + +/// +/// The decompiled view is read-only, so Tab has nothing to indent; it must move the keyboard +/// focus out of the editor like it does for every other control (WPF removed the editor's +/// TabForward/TabBackward commands for the same reason). +/// +[TestFixture] +public class TabFocusNavigationTests +{ + static async Task<(ICSharpCode.ILSpy.Views.MainWindow Window, TextArea TextArea)> FocusEditorAsync() + { + var (window, vm) = await TestHarness.BootAsync(); + var coreLibName = typeof(object).Assembly.GetName().Name!; + vm.AssemblyTreeModel.SelectNode(vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.String")); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + var textArea = window.GetVisualDescendants().OfType().First().Editor.TextArea; + textArea.Focus(); + Avalonia.Threading.Dispatcher.UIThread.RunJobs(); + var focused = TopLevel.GetTopLevel(window)!.FocusManager!.GetFocusedElement(); + focused.Should().BeSameAs(textArea, "precondition: the text area must own the keyboard focus"); + return (window, textArea); + } + + [AvaloniaTest] + public Task Tab_Moves_The_Keyboard_Focus_Out_Of_The_Editor() + => AssertTabLeavesTheEditorAsync(RawInputModifiers.None); + + [AvaloniaTest] + public Task Shift_Tab_Moves_The_Keyboard_Focus_Out_Of_The_Editor() + => AssertTabLeavesTheEditorAsync(RawInputModifiers.Shift); + + static async Task AssertTabLeavesTheEditorAsync(RawInputModifiers modifiers) + { + var (window, textArea) = await FocusEditorAsync(); + + window.KeyPress(Key.Tab, modifiers, PhysicalKey.Tab, null); + Avalonia.Threading.Dispatcher.UIThread.RunJobs(); + + var focused = TopLevel.GetTopLevel(window)!.FocusManager!.GetFocusedElement(); + focused.Should().NotBeNull("Tab must hand the focus to another control, not drop it"); + focused.Should().NotBeSameAs(textArea, "Tab must leave the read-only editor"); + ((Avalonia.Visual)focused!).GetVisualAncestors().Should().NotContain(textArea, + "Tab must leave the editor entirely, not land on one of its parts"); + } +} diff --git a/ILSpy/TextView/DecompilerTextView.axaml.cs b/ILSpy/TextView/DecompilerTextView.axaml.cs index 3293f14dac..617b7aae33 100644 --- a/ILSpy/TextView/DecompilerTextView.axaml.cs +++ b/ILSpy/TextView/DecompilerTextView.axaml.cs @@ -135,6 +135,10 @@ public DecompilerTextView() // like the About-page resource generator). Decompiler output uses hyperlinks // extensively for in-app navigation — a plain click is the expected affordance. Editor.Options.RequireControlModifierForHyperlinkClick = false; + // The view is read-only, so Tab has nothing to indent; with AcceptsTab the text area marks + // the key handled and the focus could never leave the editor. Leaving it unhandled lets + // Tab / Shift+Tab move the keyboard focus like in every other control. + Editor.Options.AcceptsTab = false; SetupElementGenerators(); SetupBackgroundRenderers(); From 85b85e0ea2ff123dc59d053b2df31255849f2dbe Mon Sep 17 00:00:00 2001 From: Christoph Wille Date: Tue, 6 Oct 2026 07:43:19 +0200 Subject: [PATCH 2/8] Look up the Analyzer "Remove" header through the resource key Every other context-menu entry names its header by resource key so the menu builder can localise it; this one shipped the raw string, which WPF ILSpy did as well. The "Remove" string already exists in the resx. #4042 Assisted-by: Claude:claude-fable-5-1:Claude Code --- ILSpy.Tests/Analyzers/AnalyzerPaneRemoveTests.cs | 5 +++-- ILSpy/Analyzers/RemoveAnalyzeContextMenuEntry.cs | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/ILSpy.Tests/Analyzers/AnalyzerPaneRemoveTests.cs b/ILSpy.Tests/Analyzers/AnalyzerPaneRemoveTests.cs index 23d767d1e2..c76bf6d050 100644 --- a/ILSpy.Tests/Analyzers/AnalyzerPaneRemoveTests.cs +++ b/ILSpy.Tests/Analyzers/AnalyzerPaneRemoveTests.cs @@ -26,6 +26,7 @@ using ICSharpCode.ILSpyX.TreeView; using ICSharpCode.ILSpy; +using ICSharpCode.ILSpy.Properties; using ICSharpCode.ILSpy.Analyzers; using ICSharpCode.ILSpy.AppEnv; using ICSharpCode.ILSpy.TreeNodes; @@ -46,7 +47,7 @@ public async Task Remove_Entry_Is_Visible_Only_For_Top_Level_Analysed_Entities() var registry = AppComposition.Current.GetExport(); var entry = registry.Entries - .Single(e => e.Metadata.Header == "Remove" + .Single(e => e.Metadata.Header == nameof(Resources.Remove) && e.Value is RemoveAnalyzeContextMenuEntry) .Value; @@ -70,7 +71,7 @@ public async Task Remove_Execute_Drops_The_Selected_Root_Children() var registry = AppComposition.Current.GetExport(); var entry = registry.Entries - .Single(e => e.Metadata.Header == "Remove" + .Single(e => e.Metadata.Header == nameof(Resources.Remove) && e.Value is RemoveAnalyzeContextMenuEntry) .Value; diff --git a/ILSpy/Analyzers/RemoveAnalyzeContextMenuEntry.cs b/ILSpy/Analyzers/RemoveAnalyzeContextMenuEntry.cs index e4eea336f4..9fe51a5989 100644 --- a/ILSpy/Analyzers/RemoveAnalyzeContextMenuEntry.cs +++ b/ILSpy/Analyzers/RemoveAnalyzeContextMenuEntry.cs @@ -19,6 +19,7 @@ using System.Composition; using System.Linq; +using ICSharpCode.ILSpy.Properties; using ICSharpCode.ILSpyX.TreeView; namespace ICSharpCode.ILSpy.Analyzers @@ -28,7 +29,7 @@ namespace ICSharpCode.ILSpy.Analyzers /// pane root. Only visible on rows whose parent is the analyzer root (the per-entity /// rows); search-tree-node headers and result rows can't be removed individually. /// - [ExportContextMenuEntry(Header = "Remove", Icon = "Images/Delete", Order = 9200)] + [ExportContextMenuEntry(Header = nameof(Resources.Remove), Icon = "Images/Delete", Order = 9200)] [Shared] public sealed class RemoveAnalyzeContextMenuEntry : IContextMenuEntry { From 4dc9784c0da153f67e055650292b75ec2cfcee34 Mon Sep 17 00:00:00 2001 From: Christoph Wille Date: Tue, 6 Oct 2026 07:45:18 +0200 Subject: [PATCH 3/8] Judge Analyze's IsEnabled on the same node shape as IsVisible IsVisible required every selected node to be a member node while IsEnabled filtered to the member subset first, so a mixed selection reported itself enabled-but-hidden. Hidden won, so nothing was visible to the user, but the two predicates disagreed about the same selection. #4042 Assisted-by: Claude:claude-fable-5-1:Claude Code --- .../Analyzers/AnalyzeContextMenuTests.cs | 22 +++++++++++++++++++ ILSpy/Analyzers/AnalyzeContextMenuEntry.cs | 6 ++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/ILSpy.Tests/Analyzers/AnalyzeContextMenuTests.cs b/ILSpy.Tests/Analyzers/AnalyzeContextMenuTests.cs index 2ec2be441f..53c883c4b2 100644 --- a/ILSpy.Tests/Analyzers/AnalyzeContextMenuTests.cs +++ b/ILSpy.Tests/Analyzers/AnalyzeContextMenuTests.cs @@ -83,6 +83,28 @@ public async Task Analyze_Entry_Is_Visible_For_Member_Tree_Nodes_And_Hidden_For_ .Should().BeFalse("an empty selection must hide the entry"); } + [AvaloniaTest] + public async Task Analyze_Entry_Is_Disabled_For_A_Selection_That_Mixes_Members_And_Other_Nodes() + { + // IsEnabled must judge the same node shape as IsVisible: a selection that contains a + // non-member node is hidden, so it must not report itself as enabled either (a filter over + // the member subset would call a type-plus-assembly selection analysable). + var (_, vm) = await TestHarness.BootAsync(); + + var entry = AppComposition.Current.GetExport() + .GetEntry(nameof(Resources.Analyze)); + + var typeNode = vm.AssemblyTreeModel.FindNode( + "System.Linq", "System.Linq", "System.Linq.Enumerable"); + var assemblyNode = vm.AssemblyTreeModel.FindNode("System.Linq"); + var mixed = new TextViewContext { SelectedTreeNodes = new SharpTreeNode[] { typeNode, assemblyNode } }; + + entry.IsVisible(mixed).Should().BeFalse("precondition: a mixed selection hides the entry"); + entry.IsEnabled(mixed).Should().BeFalse("a mixed selection must be disabled, not enabled-but-hidden"); + entry.IsEnabled(new TextViewContext { SelectedTreeNodes = new SharpTreeNode[] { assemblyNode } }) + .Should().BeFalse("a selection without any member node has nothing to analyse"); + } + [AvaloniaTest] public async Task Analyze_Is_Visible_And_Works_For_A_Clicked_Code_Reference() { diff --git a/ILSpy/Analyzers/AnalyzeContextMenuEntry.cs b/ILSpy/Analyzers/AnalyzeContextMenuEntry.cs index 49861418d4..863a700ffe 100644 --- a/ILSpy/Analyzers/AnalyzeContextMenuEntry.cs +++ b/ILSpy/Analyzers/AnalyzeContextMenuEntry.cs @@ -80,7 +80,11 @@ public static bool IsVisibleForContext(TextViewContext context) public static bool IsEnabledForContext(TextViewContext context) { if (context.SelectedTreeNodes is { Length: > 0 } nodes) - return nodes.OfType().All(n => IsAnalysable(n.Member)); + { + // Same node shape as IsVisibleForContext: a selection with a non-member node is not + // analysable, rather than "analysable for the member subset" and hidden anyway. + return nodes.All(n => n is IMemberTreeNode member && IsAnalysable(member.Member)); + } return context.Reference?.Reference is IEntity entity && IsAnalysable(entity); } From bd5544bbbb21011d0f3f2c9f2f0c76d412fb6dec Mon Sep 17 00:00:00 2001 From: Christoph Wille Date: Tue, 6 Oct 2026 07:51:38 +0200 Subject: [PATCH 4/8] Share context-menu targeting between the assembly and Analyzer trees The assembly tree had Thunderbird-style context targeting (right-click a row outside the selection without moving the selection, highlight the target, re-focus the row after a keyboard-invoked menu); the Analyzer pane only rebuilt the menu from its selection, so a right-click on another row opened the menu for the previous selection. Hosting the behaviour in one controller keyed on SharpTreeView gives both panes the same gestures and keeps the assembly pane down to its middle-click open-in-new-tab handler. #4042 Assisted-by: Claude:claude-fable-5-1:Claude Code --- .../Analyzers/AnalyzerTreeContextMenuTests.cs | 171 +++++++++++++++ ILSpy/Analyzers/AnalyzerTreeView.axaml.cs | 45 +--- ILSpy/AssemblyTree/AssemblyListPane.axaml | 9 - ILSpy/AssemblyTree/AssemblyListPane.axaml.cs | 148 +------------ ILSpy/Controls/TreeView/SharpTreeView.axaml | 9 + .../TreeView/TreeContextMenuController.cs | 202 ++++++++++++++++++ 6 files changed, 400 insertions(+), 184 deletions(-) create mode 100644 ILSpy.Tests/Analyzers/AnalyzerTreeContextMenuTests.cs create mode 100644 ILSpy/Controls/TreeView/TreeContextMenuController.cs diff --git a/ILSpy.Tests/Analyzers/AnalyzerTreeContextMenuTests.cs b/ILSpy.Tests/Analyzers/AnalyzerTreeContextMenuTests.cs new file mode 100644 index 0000000000..de5a2bd2ae --- /dev/null +++ b/ILSpy.Tests/Analyzers/AnalyzerTreeContextMenuTests.cs @@ -0,0 +1,171 @@ +// Copyright (c) 2026 Christoph Wille +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + + +using System.Linq; +using System.Threading.Tasks; + +using Avalonia; +using Avalonia.Controls; +using Avalonia.Headless; +using Avalonia.Headless.NUnit; +using Avalonia.Input; +using Avalonia.Threading; +using Avalonia.VisualTree; + +using AwesomeAssertions; + +using ICSharpCode.Decompiler.TypeSystem; +using ICSharpCode.ILSpyX.TreeView; + +using ICSharpCode.ILSpy; +using ICSharpCode.ILSpy.Analyzers; +using ICSharpCode.ILSpy.AppEnv; +using ICSharpCode.ILSpy.Controls.TreeView; +using ICSharpCode.ILSpy.Docking; +using ICSharpCode.ILSpy.TreeNodes; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.Analyzers; + +/// +/// The Analyzer pane's context menu follows the same Thunderbird-style rules as the assembly +/// tree: a right-click on a row outside the selection targets that row alone without moving the +/// selection, the target carries the highlight class while the menu is open, and a keyboard-invoked +/// menu returns the focus to the row it was opened for. +/// +[TestFixture] +public class AnalyzerTreeContextMenuTests +{ + static async Task<(ICSharpCode.ILSpy.Views.MainWindow Window, AnalyzerTreeView View, SharpTreeView Tree, + AnalyzerEntityTreeNode First, AnalyzerEntityTreeNode Second)> SetupTwoAnalysedTypesAsync() + { + var (window, vm) = await TestHarness.BootAsync(3); + var dockWorkspace = AppComposition.Current.GetExport(); + var analyzerVm = AppComposition.Current.GetExport(); + + var enumerable = vm.AssemblyTreeModel.FindNode("System.Linq", "System.Linq", "System.Linq.Enumerable"); + var lookup = vm.AssemblyTreeModel.FindNode("System.Linq", "System.Linq", "System.Linq.Lookup`2"); + var first = analyzerVm.Analyze((ITypeDefinition)enumerable.Member!); + var second = analyzerVm.Analyze((ITypeDefinition)lookup.Member!); + first.IsExpanded = false; + second.IsExpanded = false; + + dockWorkspace.ShowToolPane(AnalyzerTreeViewModel.PaneContentId); + var view = await window.WaitForComponent(); + var tree = await view.WaitForComponent(); + // Analyze selects the row it adds; put the selection back on the first row so the second + // one is the unselected target of the probes below. + tree.SelectedItem = first; + await Waiters.WaitForIdleAsync(); + ReferenceEquals(analyzerVm.SelectedItems.SingleOrDefault(), first).Should().BeTrue( + "precondition: the pane selection must sit on the first analysed row"); + return (window, view, tree, first, second); + } + + static SharpTreeViewItem? RowFor(SharpTreeView tree, SharpTreeNode node) + => tree.GetVisualDescendants().OfType().FirstOrDefault(r => ReferenceEquals(r.Node, node)); + + [AvaloniaTest] + public async Task Right_Clicking_An_Unselected_Analyzer_Row_Targets_It_Without_Moving_The_Selection() + { + var (window, _, tree, first, second) = await SetupTwoAnalysedTypesAsync(); + var analyzerVm = AppComposition.Current.GetExport(); + var menu = tree.ContextMenu!; + + await window.ClickAsync(() => RowFor(tree, second), MouseButton.Right, + pointInTarget: r => new Point(System.Math.Min(r.Bounds.Width, tree.Bounds.Width) / 2, r.Bounds.Height / 2)); + await Waiters.WaitForAsync(() => menu.IsOpen, description: "the right-clicked analyzer row's context menu to open"); + + ReferenceEquals(analyzerVm.SelectedItems.SingleOrDefault(), first).Should().BeTrue( + "right-clicking an unselected analyzer row must not move the selection"); + RowFor(tree, second)!.Classes.Should().Contain("contextTarget", + "the right-clicked row must carry the context-target highlight while the menu is open"); + + window.KeyPress(Key.Escape, RawInputModifiers.None, PhysicalKey.Escape, keySymbol: null); + await Waiters.WaitForAsync(() => !menu.IsOpen, description: "the context menu to close"); + RowFor(tree, second)!.Classes.Should().NotContain("contextTarget", + "the transient highlight must clear once the menu closes"); + } + + [AvaloniaTest] + public async Task Menu_Built_For_A_Right_Clicked_Row_Outside_The_Selection_Targets_Only_That_Row() + { + var (_, view, tree, first, second) = await SetupTwoAnalysedTypesAsync(); + + TextViewContext? seen = null; + var export = new StubExport(new RecordingEntry(c => seen = c), new ContextMenuEntryMetadata { Header = "Probe", Order = 0 }); + + var built = view.BuildContextMenuForCurrentState(new IContextMenuEntryExport[] { export }, rightClickedNode: second); + built.Should().NotBeNull("the probe entry must produce a menu"); + var item = built!.Items.OfType().Single(); + item.RaiseEvent(new Avalonia.Interactivity.RoutedEventArgs(MenuItem.ClickEvent)); + + seen.Should().NotBeNull("the probe entry must have been executed"); + ReferenceEquals(seen!.TreeGrid, tree).Should().BeTrue("the context must name the analyzer tree"); + seen.SelectedTreeNodes.Should().BeEquivalentTo(new SharpTreeNode[] { second }, + "a right-click outside the selection acts on the clicked row alone, not on the selection"); + + seen = null; + built = view.BuildContextMenuForCurrentState(new IContextMenuEntryExport[] { export }, rightClickedNode: first); + built!.Items.OfType().Single().RaiseEvent(new Avalonia.Interactivity.RoutedEventArgs(MenuItem.ClickEvent)); + seen!.SelectedTreeNodes.Should().BeEquivalentTo(new SharpTreeNode[] { first }, + "a right-click inside the selection acts on the whole selection"); + } + + [AvaloniaTest] + public async Task Keyboard_Invoked_Analyzer_Menu_Returns_Focus_To_The_Row_On_Close() + { + var (window, _, tree, first, _) = await SetupTwoAnalysedTypesAsync(); + var row = RowFor(tree, first); + row.Should().NotBeNull("the selected analyzer row must be realised"); + row!.Focus(NavigationMethod.Tab); + Dispatcher.UIThread.RunJobs(); + + var focusManager = TopLevel.GetTopLevel(window)!.FocusManager!; + (focusManager.GetFocusedElement() == row).Should().BeTrue("the row must hold focus before invoking the menu"); + + // Keyboard invocation raises ContextRequested with no pointer position (the Shift+F10 / Apps path). + row.RaiseEvent(new ContextRequestedEventArgs()); + await Waiters.WaitForIdleAsync(); + tree.ContextMenu!.IsOpen.Should().BeTrue("the keyboard gesture must open the analyzer context menu"); + row.Classes.Should().Contain("contextTarget", + "a keyboard-invoked menu must show the target highlight on the focused row, like the mouse path"); + + window.KeyPress(Key.Escape, RawInputModifiers.None, PhysicalKey.Escape, keySymbol: null); + await Waiters.WaitForIdleAsync(); + + (focusManager.GetFocusedElement() == row).Should().BeTrue( + "closing a keyboard-invoked context menu must return focus to the row, not strand it"); + row.Classes.Should().NotContain("contextTarget", "the transient highlight must clear once the menu closes"); + } + + sealed class RecordingEntry(System.Action onExecute) : IContextMenuEntry + { + public bool IsVisible(TextViewContext context) => true; + public bool IsEnabled(TextViewContext context) => true; + public void Execute(TextViewContext context) => onExecute(context); + } + + sealed class StubExport(IContextMenuEntry entry, ContextMenuEntryMetadata metadata) : IContextMenuEntryExport + { + public IContextMenuEntry Value { get; } = entry; + public ContextMenuEntryMetadata Metadata { get; } = metadata; + } +} diff --git a/ILSpy/Analyzers/AnalyzerTreeView.axaml.cs b/ILSpy/Analyzers/AnalyzerTreeView.axaml.cs index 36c9c395b1..f9f9a5062c 100644 --- a/ILSpy/Analyzers/AnalyzerTreeView.axaml.cs +++ b/ILSpy/Analyzers/AnalyzerTreeView.axaml.cs @@ -18,7 +18,6 @@ using System; using System.Collections.Generic; -using System.Collections.Specialized; using System.Linq; using Avalonia.Controls; @@ -26,6 +25,7 @@ using ICSharpCode.ILSpyX.TreeView; using ICSharpCode.ILSpy.AppEnv; +using ICSharpCode.ILSpy.Controls.TreeView; namespace ICSharpCode.ILSpy.Analyzers { @@ -33,53 +33,26 @@ public partial class AnalyzerTreeView : UserControl { AnalyzerTreeViewModel? boundModel; ICSharpCode.ILSpy.Controls.TreeView.TreeSelectionBinder? selectionBinder; - IReadOnlyList contextMenuEntries = Array.Empty(); + readonly TreeContextMenuController contextMenu; public AnalyzerTreeView() { InitializeComponent(); + contextMenu = new TreeContextMenuController(Tree, + () => boundModel?.SelectedItems ?? (IReadOnlyList)Array.Empty()); var registry = AppComposition.TryGetExport(); AttachContextMenu(registry?.Entries ?? Array.Empty()); } - internal void AttachContextMenu(IReadOnlyList entries) - { - contextMenuEntries = entries; - var menu = new ContextMenu(); - menu.Opening += OnContextMenuOpening; - Tree.ContextMenu = menu; - } - - void OnContextMenuOpening(object? sender, System.ComponentModel.CancelEventArgs e) - { - if (sender is not ContextMenu menu) - return; - var built = BuildContextMenuForCurrentState(contextMenuEntries); - if (built == null) - { - e.Cancel = true; - return; - } - menu.Items.Clear(); - foreach (var item in built.Items.OfType().ToArray()) - { - built.Items.Remove(item); - menu.Items.Add(item); - } - } + => contextMenu.Attach(entries); internal ContextMenu? BuildContextMenuForCurrentState(IReadOnlyList entries) - => ContextMenuProvider.Build(entries, CreateContextMenuContext()); + => contextMenu.Build(entries); - TextViewContext CreateContextMenuContext() - { - var nodes = boundModel?.SelectedItems.ToArray() ?? Array.Empty(); - return new TextViewContext { - TreeGrid = Tree, - SelectedTreeNodes = nodes, - }; - } + internal ContextMenu? BuildContextMenuForCurrentState( + IReadOnlyList entries, SharpTreeNode? rightClickedNode) + => contextMenu.Build(entries, rightClickedNode); protected override void OnDataContextChanged(EventArgs e) { diff --git a/ILSpy/AssemblyTree/AssemblyListPane.axaml b/ILSpy/AssemblyTree/AssemblyListPane.axaml index 590bbf887b..cd0ca7b127 100644 --- a/ILSpy/AssemblyTree/AssemblyListPane.axaml +++ b/ILSpy/AssemblyTree/AssemblyListPane.axaml @@ -8,15 +8,6 @@ x:Class="ICSharpCode.ILSpy.AssemblyTree.AssemblyListPane" x:DataType="tree:AssemblyTreeModel"> - - diff --git a/ILSpy/Controls/TreeView/TreeContextMenuController.cs b/ILSpy/Controls/TreeView/TreeContextMenuController.cs new file mode 100644 index 0000000000..bfe76c55e3 --- /dev/null +++ b/ILSpy/Controls/TreeView/TreeContextMenuController.cs @@ -0,0 +1,202 @@ +// Copyright (c) 2026 Christoph Wille +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + + +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Linq; + +using Avalonia; +using Avalonia.Controls; +using Avalonia.Input; +using Avalonia.Interactivity; +using Avalonia.Threading; +using Avalonia.VisualTree; + +using ICSharpCode.ILSpyX.TreeView; + +namespace ICSharpCode.ILSpy.Controls.TreeView +{ + /// + /// Hosts the registry-built context menu of a with Thunderbird-style + /// targeting, shared by every tree pane: a right-click on a row outside the selection opens the + /// menu for that row alone without moving the selection (the right press is swallowed so the + /// ListBox does not select the row), the target row carries the contextTarget highlight + /// while the menu is open, and a keyboard-invoked menu (Shift+F10 / Apps) adopts the focused row + /// and hands the focus back to it when the popup closes. + /// + public sealed class TreeContextMenuController + { + readonly SharpTreeView tree; + readonly Func> modelSelection; + IReadOnlyList entries = Array.Empty(); + + // The row whose context menu is open, highlighted without moving the real selection. + SharpTreeViewItem? contextTargetItem; + SharpTreeViewItem? contextMenuOpenItem; + SharpTreeNode? contextMenuTargetNode; + // For a keyboard-invoked menu, the row to re-focus when the menu closes (closing the popup + // otherwise drops the keyboard focus and its focus adorner). Null for pointer-invoked menus. + SharpTreeViewItem? focusToRestoreAfterMenu; + // Whether the last ContextRequested came from the keyboard (no pointer position). The keyboard + // path carries no target row, so the menu adopts the focused row (see OnContextMenuOpening). + bool lastContextRequestWasKeyboard; + + /// The pane's model selection, read when a menu is built. + public TreeContextMenuController(SharpTreeView tree, Func> modelSelection) + { + this.tree = tree ?? throw new ArgumentNullException(nameof(tree)); + this.modelSelection = modelSelection ?? throw new ArgumentNullException(nameof(modelSelection)); + tree.AddHandler(InputElement.PointerPressedEvent, OnTreePointerPressed, RoutingStrategies.Tunnel); + tree.AddHandler(Control.ContextRequestedEvent, OnTreeContextRequested, RoutingStrategies.Bubble, handledEventsToo: true); + } + + /// Installs a context menu on the tree that is rebuilt from every time it opens. + public void Attach(IReadOnlyList entries) + { + this.entries = entries; + var menu = new ContextMenu(); + menu.Opening += OnContextMenuOpening; + menu.Closed += (_, _) => { + RestoreFocusAfterKeyboardMenu(); + if (!ReferenceEquals(contextTargetItem, contextMenuOpenItem)) + return; + contextMenuTargetNode = null; + SetContextTargetItem(null); + }; + tree.ContextMenu = menu; + } + + void OnContextMenuOpening(object? sender, CancelEventArgs e) + { + if (sender is not ContextMenu menu) + return; + // A keyboard-invoked menu carries no pointer position, so OnTreeContextRequested set no + // transient target. Adopt the keyboard-FOCUSED row (which may differ from the selection + // after Ctrl+Arrow) as the target: opening the popup steals focus and drops the row's focus + // adorner, so we mark that row with the same context-target highlight the mouse gives the + // right-clicked row, and restore its focus + adorner on close (Avalonia's ContextMenu does not). + // Captured here, before the popup opens and takes focus (Opening fires ahead of it), and before + // contextMenuOpenItem is latched so the Closed handler still clears the highlight. + var focusedRow = TopLevel.GetTopLevel(tree)?.FocusManager?.GetFocusedElement() as SharpTreeViewItem; + if (lastContextRequestWasKeyboard && focusedRow?.Node != null) + { + contextMenuTargetNode = focusedRow.Node; + SetContextTargetItem(focusedRow); + focusToRestoreAfterMenu = focusedRow; + } + contextMenuOpenItem = contextTargetItem; + var built = Build(entries); + if (built == null) + { + // Menu won't open (so Closed won't fire) -- undo the transient target + captured focus. + focusToRestoreAfterMenu = null; + contextMenuTargetNode = null; + SetContextTargetItem(null); + e.Cancel = true; + return; + } + menu.Items.Clear(); + foreach (var item in built.Items.OfType().ToArray()) + { + built.Items.Remove(item); + menu.Items.Add(item); + } + } + + void RestoreFocusAfterKeyboardMenu() + { + if (focusToRestoreAfterMenu is not { } toFocus) + return; + focusToRestoreAfterMenu = null; + // Re-focus with a keyboard navigation method so the focus visual (the adorner) comes back, + // not just the logical focus. Posted so it runs after the popup has fully torn down. + Dispatcher.UIThread.Post(() => toFocus.Focus(NavigationMethod.Tab)); + } + + /// Builds the menu for the current selection, as the live Opening event would. + public ContextMenu? Build(IReadOnlyList entries) + => ContextMenuProvider.Build(entries, CreateContext()); + + /// Builds the menu as a right-click on would. + public ContextMenu? Build(IReadOnlyList entries, SharpTreeNode? rightClickedNode) + { + contextMenuTargetNode = rightClickedNode; + try + { + return ContextMenuProvider.Build(entries, CreateContext()); + } + finally + { + contextMenuTargetNode = null; + } + } + + TextViewContext CreateContext() + { + var selection = modelSelection().ToArray(); + // A right-click outside the selection targets just the clicked row; inside the selection + // (or a keyboard-invoked menu with no target) acts on the whole selection. + var target = contextMenuTargetNode; + var nodes = target != null && !Array.Exists(selection, n => ReferenceEquals(n, target)) + ? new[] { target } + : selection; + return new TextViewContext { + TreeGrid = tree, + SelectedTreeNodes = nodes, + }; + } + + void SetContextTargetItem(SharpTreeViewItem? item) + { + if (ReferenceEquals(contextTargetItem, item)) + return; + contextTargetItem?.Classes.Remove("contextTarget"); + contextTargetItem = item; + contextTargetItem?.Classes.Add("contextTarget"); + } + + void OnTreeContextRequested(object? sender, ContextRequestedEventArgs e) + { + SharpTreeViewItem? item = null; + // Keyboard invocation (Shift+F10 / Apps) raises ContextRequested with no pointer position. + lastContextRequestWasKeyboard = !e.TryGetPosition(tree, out var pos); + if (!lastContextRequestWasKeyboard && tree.InputHitTest(pos) is Visual hit) + item = hit.FindAncestorOfType(includeSelf: true); + contextMenuTargetNode = item?.Node; + SetContextTargetItem(item?.Node != null ? item : null); + } + + void OnTreePointerPressed(object? sender, PointerPressedEventArgs e) + { + if (e.Source is not Visual hit) + return; + if (e.GetCurrentPoint(hit).Properties.IsRightButtonPressed) + { + // Swallow the right press so the ListBox doesn't move the selection to the row. + if (hit.FindAncestorOfType(includeSelf: true)?.Node != null) + e.Handled = true; + return; + } + // Any non-right press starts a fresh gesture -- drop a stale right-click target. + contextMenuTargetNode = null; + SetContextTargetItem(null); + } + } +} From 0231a96f128912150d54cfa20edfec9837ed29cf Mon Sep 17 00:00:00 2001 From: Christoph Wille Date: Tue, 6 Oct 2026 07:55:18 +0200 Subject: [PATCH 5/8] Let Ctrl+R in the Analyzer pane analyze the pane's own selection The only Ctrl+R binding sat on the window and always analyzed the assembly tree's selection, so pressing it on a result row in the Analyzer pane promoted the wrong entity, or nothing. WPF ILSpy bound the key on the pane as well. Avalonia walks KeyBindings from the focused element up to the window, so a pane-level binding takes precedence while the focus is in the pane and falls through when it has nothing to analyze. The command takes the selection as its parameter so both bindings share one implementation. #4042 Assisted-by: Claude:claude-fable-5-1:Claude Code --- .../Analyzers/AnalyzerTreeKeyboardTests.cs | 53 +++++++++++++++++++ ILSpy/Analyzers/AnalyzeContextMenuEntry.cs | 13 +++-- ILSpy/Analyzers/AnalyzerTreeView.axaml.cs | 16 ++++++ 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/ILSpy.Tests/Analyzers/AnalyzerTreeKeyboardTests.cs b/ILSpy.Tests/Analyzers/AnalyzerTreeKeyboardTests.cs index 7cc914db17..7a49e1d065 100644 --- a/ILSpy.Tests/Analyzers/AnalyzerTreeKeyboardTests.cs +++ b/ILSpy.Tests/Analyzers/AnalyzerTreeKeyboardTests.cs @@ -30,6 +30,7 @@ using ICSharpCode.Decompiler.TypeSystem; using ICSharpCode.ILSpy.Analyzers; +using ICSharpCode.ILSpy.Analyzers.TreeNodes; using ICSharpCode.ILSpy.AppEnv; using ICSharpCode.ILSpy.Docking; using ICSharpCode.ILSpy.TreeNodes; @@ -167,4 +168,56 @@ await Waiters.WaitForAsync(() => analyzerVm.Root.Children.Count > before, .Any(n => n.Member is { } m && m.MetadataToken == ((ITypeDefinition)typeNode.Member!).MetadataToken) .Should().BeTrue("the analyzer pane must hold a node for the type that Ctrl+R analyzed"); } + + [AvaloniaTest] + public async Task Ctrl_R_On_An_Analyzer_Result_Row_Promotes_It_Instead_Of_The_Assembly_Tree_Selection() + { + // Ctrl+R with the focus inside the Analyzer pane analyzes the pane's own selection (a result + // row becomes a top-level entry), like the pane-level binding did in 10.x. The window-level + // Ctrl+R, which analyzes the assembly tree's selection, must not fire for the pane. + var (window, vm) = await TestHarness.BootAsync(3); + var dockWorkspace = AppComposition.Current.GetExport(); + var analyzerVm = AppComposition.Current.GetExport(); + + var typeNode = vm.AssemblyTreeModel.FindNode( + "System.Linq", "System.Linq", "System.Linq.Enumerable"); + typeNode.IsExpanded = true; + var method = typeNode.Children.OfType() + .First(m => m.MethodDefinition.Name == "Empty").MethodDefinition; + var analyzed = analyzerVm.Analyze((ITypeDefinition)typeNode.Member!); + analyzed.EnsureLazyChildren(); + // A result row lives underneath an analyzer-search header, never directly under the root. + var searchRow = analyzed.Children.OfType().First(); + var resultRow = new AnalyzedMethodTreeNode(method, typeNode.Member); + searchRow.Children.Add(resultRow); + analyzed.IsExpanded = true; + searchRow.IsExpanded = true; + + // Park the assembly tree on another analysable type: if the window binding fired instead, + // this is what would land in the pane. + var decoy = vm.AssemblyTreeModel.FindNode("System.Linq", "System.Linq", "System.Linq.Lookup`2"); + vm.AssemblyTreeModel.SelectNode(decoy); + await Waiters.WaitForIdleAsync(); + + dockWorkspace.ShowToolPane(AnalyzerTreeViewModel.PaneContentId); + var view = await window.WaitForComponent(); + var tree = await view.WaitForComponent(); + tree.SelectedItem = resultRow; + Dispatcher.UIThread.RunJobs(); + tree.FocusNode(resultRow); + Dispatcher.UIThread.RunJobs(); + ((object?)analyzerVm.SelectedItems.SingleOrDefault()).Should().BeSameAs(resultRow, + "precondition: the pane selection must sit on the result row"); + + int before = analyzerVm.Root.Children.Count; + window.KeyPress(Key.R, RawInputModifiers.Control, PhysicalKey.R, null); + await Waiters.WaitForAsync(() => analyzerVm.Root.Children.Count > before, + description: "Ctrl+R in the Analyzer pane must promote the selected result row"); + + var promoted = analyzerVm.Root.Children.OfType().Last(); + promoted.Member.Should().BeSameAs(method, "the pane's own selection is what Ctrl+R analyzes"); + analyzerVm.Root.Children.OfType() + .Any(n => n.Member is { } m && m.MetadataToken == ((ITypeDefinition)decoy.Member!).MetadataToken) + .Should().BeFalse("the assembly tree's selection must not be analyzed when the key is pressed inside the pane"); + } } diff --git a/ILSpy/Analyzers/AnalyzeContextMenuEntry.cs b/ILSpy/Analyzers/AnalyzeContextMenuEntry.cs index 863a700ffe..b24efdd8dd 100644 --- a/ILSpy/Analyzers/AnalyzeContextMenuEntry.cs +++ b/ILSpy/Analyzers/AnalyzeContextMenuEntry.cs @@ -16,11 +16,13 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. +using System.Collections.Generic; using System.Composition; using System.Linq; using ICSharpCode.Decompiler.TypeSystem; using ICSharpCode.ILSpy.Properties; +using ICSharpCode.ILSpyX.TreeView; using ICSharpCode.ILSpy.AssemblyTree; using ICSharpCode.ILSpy.Commands; @@ -138,17 +140,20 @@ public sealed class AnalyzeCommand( { public override bool CanExecute(object? parameter) { - var context = CreateContext(); + var context = CreateContext(parameter); return AnalyzeContextMenuEntry.IsVisibleForContext(context) && AnalyzeContextMenuEntry.IsEnabledForContext(context); } public override void Execute(object? parameter) - => AnalyzeContextMenuEntry.Analyze(CreateContext(), analyzerTreeViewModel, dockWorkspace); + => AnalyzeContextMenuEntry.Analyze(CreateContext(parameter), analyzerTreeViewModel, dockWorkspace); - TextViewContext CreateContext() + // The parameter is the selection to analyze: a tree pane binding the shortcut passes its own + // (the Analyzer pane promotes its selected result rows); the window-level binding passes + // none and analyzes the assembly tree's selection. + TextViewContext CreateContext(object? parameter) => new() { - SelectedTreeNodes = assemblyTreeModel.SelectedItems.ToArray(), + SelectedTreeNodes = (parameter as IEnumerable ?? assemblyTreeModel.SelectedItems).ToArray(), }; } } diff --git a/ILSpy/Analyzers/AnalyzerTreeView.axaml.cs b/ILSpy/Analyzers/AnalyzerTreeView.axaml.cs index f9f9a5062c..2544219c4b 100644 --- a/ILSpy/Analyzers/AnalyzerTreeView.axaml.cs +++ b/ILSpy/Analyzers/AnalyzerTreeView.axaml.cs @@ -21,6 +21,7 @@ using System.Linq; using Avalonia.Controls; +using Avalonia.Input; using ICSharpCode.ILSpyX.TreeView; @@ -34,10 +35,23 @@ public partial class AnalyzerTreeView : UserControl AnalyzerTreeViewModel? boundModel; ICSharpCode.ILSpy.Controls.TreeView.TreeSelectionBinder? selectionBinder; readonly TreeContextMenuController contextMenu; + // Ctrl+R analyzes the pane's own selection. Avalonia walks KeyBindings from the focused element + // up to the window before raising KeyDown, so this binding runs ahead of the window-level one + // (which analyzes the assembly tree's selection) while the focus is in the pane, and falls + // through to it when nothing analyzable is selected here. + readonly KeyBinding analyzeBinding = new() { + Gesture = new KeyGesture(Key.R, KeyModifiers.Control), + CommandParameter = Array.Empty(), + }; public AnalyzerTreeView() { InitializeComponent(); + if (AppComposition.TryGetExport() is { } analyze) + { + analyzeBinding.Command = analyze; + KeyBindings.Add(analyzeBinding); + } contextMenu = new TreeContextMenuController(Tree, () => boundModel?.SelectedItems ?? (IReadOnlyList)Array.Empty()); var registry = AppComposition.TryGetExport(); @@ -67,6 +81,7 @@ void AttachToModel(AnalyzerTreeViewModel model) boundModel = model; Tree.Root = model.Root; selectionBinder = new ICSharpCode.ILSpy.Controls.TreeView.TreeSelectionBinder(Tree, model.SelectedItems); + analyzeBinding.CommandParameter = model.SelectedItems; } void DetachFromModel() @@ -74,6 +89,7 @@ void DetachFromModel() selectionBinder?.Dispose(); selectionBinder = null; boundModel = null; + analyzeBinding.CommandParameter = Array.Empty(); } } } From 95576cb9e1bdcf1ea5f881be165b0217350a8c46 Mon Sep 17 00:00:00 2001 From: Christoph Wille Date: Tue, 6 Oct 2026 08:04:29 +0200 Subject: [PATCH 6/8] Focus a programmatically selected tree row only if the tree owns focus The tree selection binder focused the selected row on every model-driven change, so Back/Forward, search-result jumps, go-to-definition, tab activation and omnibar picks all pulled the keyboard focus into the assembly tree. WPF's SelectNodes only scrolled the row into view; the row took the focus when the pane itself was activated. The binder now checks where the keyboard focus sits when the selection changes and leaves it alone unless the tree already holds it. The Analyzer pane keeps focusing its row, as its WPF counterpart did on every selection change, so an Analyze request still lands the user in the pane. #4042 Assisted-by: Claude:claude-fable-5-1:Claude Code --- .../BrowseBackForwardCommandTests.cs | 47 +++++++++++++++++++ ILSpy/Analyzers/AnalyzerTreeView.axaml.cs | 4 +- .../Controls/TreeView/TreeSelectionBinder.cs | 37 ++++++++++++--- 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/ILSpy.Tests/Navigation/BrowseBackForwardCommandTests.cs b/ILSpy.Tests/Navigation/BrowseBackForwardCommandTests.cs index ce2a1a6c78..497f40645a 100644 --- a/ILSpy.Tests/Navigation/BrowseBackForwardCommandTests.cs +++ b/ILSpy.Tests/Navigation/BrowseBackForwardCommandTests.cs @@ -218,6 +218,53 @@ await Waiters.WaitForAsync(() => ReferenceEquals(vm.AssemblyTreeModel.SelectedIt "navigating back must not move the active pane to the editor"); } + [AvaloniaTest] + public async Task Browse_Back_Keeps_The_Keyboard_Focus_In_The_Editor() + { + // Back/Forward select the tree node of the previous entry, which must only scroll it into + // view: WPF's SelectNodes never took the keyboard focus, so a user reading the code pane + // stays in the code pane after Alt+Left / Mouse4. + var (window, vm) = await TestHarness.BootAsync(3); + var (firstMethod, _) = await BuildTwoEntryHistoryAsync(vm); + var textArea = window.GetVisualDescendants().OfType().First().Editor.TextArea; + textArea.Focus(); + Avalonia.Threading.Dispatcher.UIThread.RunJobs(); + var focusManager = TopLevel.GetTopLevel(window)!.FocusManager!; + focusManager.GetFocusedElement().Should().BeSameAs(textArea, "precondition: the editor must own the focus"); + + vm.DockWorkspace.NavigateBackCommand.Execute(null); + + await Waiters.WaitForAsync(() => ReferenceEquals(vm.AssemblyTreeModel.SelectedItem, firstMethod), + description: "BrowseBack must navigate back one history entry"); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + await Waiters.WaitForIdleAsync(); + focusManager.GetFocusedElement().Should().BeSameAs(textArea, + "navigating back must not move the keyboard focus to the tree row"); + } + + [AvaloniaTest] + public async Task Programmatic_Selection_Moves_The_Focus_Along_When_The_Tree_Owns_It() + { + // The scroll-only rule applies when the focus is elsewhere; a tree that already owns the + // keyboard focus follows the selection to the new row, so keyboard navigation inside the + // tree keeps working after a programmatic jump. + var (window, vm) = await TestHarness.BootAsync(3); + var (firstMethod, secondMethod) = await BuildTwoEntryHistoryAsync(vm); + var pane = await window.WaitForComponent(); + var tree = await pane.WaitForComponent(); + tree.FocusNode(secondMethod); + Avalonia.Threading.Dispatcher.UIThread.RunJobs(); + var focusManager = TopLevel.GetTopLevel(window)!.FocusManager!; + ReferenceEquals((focusManager.GetFocusedElement() as ICSharpCode.ILSpy.Controls.TreeView.SharpTreeViewItem)?.Node, secondMethod) + .Should().BeTrue("precondition: the tree row must own the focus"); + + vm.AssemblyTreeModel.SelectNode(firstMethod); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + await Waiters.WaitForAsync( + () => (focusManager.GetFocusedElement() as ICSharpCode.ILSpy.Controls.TreeView.SharpTreeViewItem)?.Node == firstMethod, + description: "the focus must follow the selection to the newly selected row"); + } + // Selects two methods of System.Linq.Enumerable with a pause in between so the history records // them as two separate entries; returns them in selection order. static async Task<(MethodTreeNode First, MethodTreeNode Second)> BuildTwoEntryHistoryAsync(MainWindowViewModel vm) diff --git a/ILSpy/Analyzers/AnalyzerTreeView.axaml.cs b/ILSpy/Analyzers/AnalyzerTreeView.axaml.cs index 2544219c4b..7b8d3fa47a 100644 --- a/ILSpy/Analyzers/AnalyzerTreeView.axaml.cs +++ b/ILSpy/Analyzers/AnalyzerTreeView.axaml.cs @@ -80,7 +80,9 @@ void AttachToModel(AnalyzerTreeViewModel model) { boundModel = model; Tree.Root = model.Root; - selectionBinder = new ICSharpCode.ILSpy.Controls.TreeView.TreeSelectionBinder(Tree, model.SelectedItems); + // focusOnSelect: an Analyze request from anywhere lands the user in the pane, on the new row + // (WPF's AnalyzerTreeView focused its row on every selection change). + selectionBinder = new TreeSelectionBinder(Tree, model.SelectedItems, focusOnSelect: true); analyzeBinding.CommandParameter = model.SelectedItems; } diff --git a/ILSpy/Controls/TreeView/TreeSelectionBinder.cs b/ILSpy/Controls/TreeView/TreeSelectionBinder.cs index ba167261ff..5089e55275 100644 --- a/ILSpy/Controls/TreeView/TreeSelectionBinder.cs +++ b/ILSpy/Controls/TreeView/TreeSelectionBinder.cs @@ -23,8 +23,10 @@ using System.Collections.Specialized; using System.Linq; +using Avalonia; using Avalonia.Controls; using Avalonia.Threading; +using Avalonia.VisualTree; using ICSharpCode.ILSpyX.TreeView; @@ -33,14 +35,16 @@ namespace ICSharpCode.ILSpy.Controls.TreeView /// /// Two-way binds a 's selection to a view-model's /// : user selection flows into the model, and a - /// model-driven change (restore, navigate, freshly-opened nodes) reveals + focuses the primary in - /// the tree. One implementation shared by every tree pane, replacing the per-pane sync code. + /// model-driven change (restore, navigate, freshly-opened nodes) reveals the primary in the tree + /// and focuses it when the tree already owns the keyboard focus (see the focusOnSelect parameter). + /// One implementation shared by every tree pane, replacing the per-pane sync code. /// public sealed class TreeSelectionBinder : IDisposable { readonly SharpTreeView tree; readonly ObservableCollection modelSelection; readonly Func? batchSelectionChange; + readonly bool focusOnSelect; bool syncing; /// @@ -48,12 +52,19 @@ public sealed class TreeSelectionBinder : IDisposable /// sync that touches many rows costs one notification instead of one per row. Panes whose /// model has no such scope pass null and get the per-item behaviour. /// + /// + /// True: every model-driven selection moves the keyboard focus to the selected row (the + /// Analyzer pane, where an Analyze request lands the user in the pane). False: the row is + /// only scrolled into view unless the tree already owns the keyboard focus, so Back/Forward, + /// search-result jumps, go-to-definition and tab activation leave the focus where it is. + /// public TreeSelectionBinder(SharpTreeView tree, ObservableCollection modelSelection, - Func? batchSelectionChange = null) + Func? batchSelectionChange = null, bool focusOnSelect = false) { this.tree = tree ?? throw new ArgumentNullException(nameof(tree)); this.modelSelection = modelSelection ?? throw new ArgumentNullException(nameof(modelSelection)); this.batchSelectionChange = batchSelectionChange; + this.focusOnSelect = focusOnSelect; tree.SelectionChanged += OnTreeSelectionChanged; tree.Loaded += OnTreeLoaded; modelSelection.CollectionChanged += OnModelSelectionChanged; @@ -154,16 +165,19 @@ void SyncModelToTree() primary = node; } } - // Reveal + focus the primary AFTER layout settles -- a model change that also reshapes + // Reveal (+ focus) the primary AFTER layout settles -- a model change that also reshapes // the tree (a reorder rebuilds the flattener) leaves the panel mid-arrange, and a - // synchronous ScrollIntoView would throw "Invalid Arrange rectangle". + // synchronous ScrollIntoView would throw "Invalid Arrange rectangle". Whether to focus is + // decided now, from where the keyboard focus sits at the time of the selection change. if (primary is { } toReveal) { bool wasVisible = visibleBefore.Contains(toReveal); + bool focus = focusOnSelect || TreeOwnsKeyboardFocus(); Dispatcher.UIThread.Post(() => { if (!wasVisible) tree.ScrollIntoNodeView(toReveal); - tree.FocusNode(toReveal, scroll: !wasVisible); + if (focus) + tree.FocusNode(toReveal, scroll: !wasVisible); }); } } @@ -172,5 +186,16 @@ void SyncModelToTree() syncing = false; } } + + // WPF's SelectNodes scrolled the row into view without focusing it; the row took the focus + // only when the pane itself was activated. The equivalent here: a tree that already holds + // the keyboard focus (or a window where nothing holds it yet, e.g. the selection restored at + // startup) follows the selection, any other focused control keeps the focus. + bool TreeOwnsKeyboardFocus() + { + var focused = TopLevel.GetTopLevel(tree)?.FocusManager?.GetFocusedElement(); + return focused is null or TopLevel + || (focused is Visual visual && (ReferenceEquals(visual, tree) || tree.IsVisualAncestorOf(visual))); + } } } From 7efd079adc0ee351638077c6409142be44ee79e1 Mon Sep 17 00:00:00 2001 From: Christoph Wille Date: Tue, 6 Oct 2026 08:08:51 +0200 Subject: [PATCH 7/8] Guard same-value SetActiveDockable in the dock factory Dock's ActiveDockable setter re-runs InitActiveDockable, and with it SetFocusedDockable, even when the value does not change, so activating the already-active document moved the active-pane highlight to the documents dock. Two callers guarded against that individually; the factory override does it for every caller, and ActivateAndFocus keeps its explicit SetFocusedDockable for the callers that do want the focus. #4042 Assisted-by: Claude:claude-fable-5-1:Claude Code --- ILSpy.Tests/Docking/SetActiveDockableTests.cs | 66 +++++++++++++++++++ ILSpy/Docking/DockWorkspace.cs | 19 ++---- ILSpy/Docking/ILSpyDockFactory.cs | 15 +++++ 3 files changed, 88 insertions(+), 12 deletions(-) create mode 100644 ILSpy.Tests/Docking/SetActiveDockableTests.cs diff --git a/ILSpy.Tests/Docking/SetActiveDockableTests.cs b/ILSpy.Tests/Docking/SetActiveDockableTests.cs new file mode 100644 index 0000000000..ff3740567b --- /dev/null +++ b/ILSpy.Tests/Docking/SetActiveDockableTests.cs @@ -0,0 +1,66 @@ +// Copyright (c) 2026 Christoph Wille +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + + +using System.Threading.Tasks; + +using Avalonia.Headless.NUnit; + +using AwesomeAssertions; + +using ICSharpCode.ILSpy.AssemblyTree; +using ICSharpCode.ILSpy.TreeNodes; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.Docking; + +/// +/// Dock's ActiveDockable setter re-runs its activation (ending in SetFocusedDockable) even when +/// the value does not change, so a plain SetActiveDockable of the already-active document would +/// move the active-pane highlight to the documents dock. The factory guards that centrally, so +/// every caller may re-activate without checking first. +/// +[TestFixture] +public class SetActiveDockableTests +{ + [AvaloniaTest] + public async Task Reactivating_The_Active_Document_Keeps_The_Focused_Dockable() + { + var (_, vm) = await TestHarness.BootAsync(); + var typeNode = vm.AssemblyTreeModel.FindNode( + "System.Linq", "System.Linq", "System.Linq.Enumerable"); + vm.AssemblyTreeModel.SelectNode(typeNode); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + + var docs = vm.DockWorkspace.Documents!; + var activeDocument = docs.ActiveDockable; + activeDocument.Should().NotBeNull("decompiling a type must leave an active document tab"); + + vm.DockWorkspace.ShowToolPane(AssemblyTreeModel.PaneContentId); + var focusedPane = vm.DockWorkspace.Layout.FocusedDockable; + focusedPane.Should().NotBeNull("showing the assembly pane must make it the focused dockable"); + focusedPane.Should().NotBeSameAs(activeDocument, "precondition: the focus must sit outside the documents dock"); + + vm.DockWorkspace.Factory.SetActiveDockable(activeDocument!); + + docs.ActiveDockable.Should().BeSameAs(activeDocument, "the active document stays active"); + vm.DockWorkspace.Layout.FocusedDockable.Should().BeSameAs(focusedPane, + "re-activating the already-active document must not move the focused dockable to it"); + } +} diff --git a/ILSpy/Docking/DockWorkspace.cs b/ILSpy/Docking/DockWorkspace.cs index d61695b393..c54f3e1227 100644 --- a/ILSpy/Docking/DockWorkspace.cs +++ b/ILSpy/Docking/DockWorkspace.cs @@ -607,11 +607,9 @@ void ApplyNavigationTarget(NavigationEntry target) suppressHistoryRecording = true; try { - // Only activate a tab that is not already active: Dock's ActiveDockable setter re-runs - // InitActiveDockable -> SetFocusedDockable even for an unchanged value, which would - // move the active pane to the document on every navigation. - if (factory.Documents is { VisibleDockables: { } docs } documents - && docs.Contains(target.Tab) && !ReferenceEquals(documents.ActiveDockable, target.Tab)) + // Bring the entry's tab to the front (a no-op for the already-active tab, so the active + // pane does not move to the document on every navigation). + if (factory.Documents is { VisibleDockables: { } docs } && docs.Contains(target.Tab)) factory.SetActiveDockable(target.Tab); if (target is TreeNodeEntry treeNode) { @@ -928,17 +926,14 @@ static bool IsActiveAssemblyOrPackageEntry(ICSharpCode.ILSpyX.LoadedAssembly ass } // Brings MainTab to the front of the documents dock so the just-updated content is - // what the user sees. No-op when MainTab is already active, when there's no - // documents dock, or when a Back/Forward navigation is in flight (ApplyNavigationTarget - // has already chosen which tab to activate, including possibly a sibling tab — our - // activation would override that intent). + // what the user sees. No-op when there's no documents dock, or when a Back/Forward + // navigation is in flight (ApplyNavigationTarget has already chosen which tab to activate, + // including possibly a sibling tab -- our activation would override that intent). void ActivateMainTabIfNeeded(ContentTabPage main) { if (suppressHistoryRecording) return; - if (factory.Documents is not { } docs) - return; - if (ReferenceEquals(docs.ActiveDockable, main)) + if (factory.Documents is null) return; factory.SetActiveDockable(main); } diff --git a/ILSpy/Docking/ILSpyDockFactory.cs b/ILSpy/Docking/ILSpyDockFactory.cs index 86d650f4ae..73f83b0759 100644 --- a/ILSpy/Docking/ILSpyDockFactory.cs +++ b/ILSpy/Docking/ILSpyDockFactory.cs @@ -466,6 +466,21 @@ static double ComputeMiddleColumnProportion(ToolDock? left, ToolDock? right) _ => ("BottomTools", Alignment.Bottom, 0.2), }; + /// + /// Activates within its owner, unless it is already the owner's + /// active dockable. Dock's ActiveDockable setter re-runs InitActiveDockable, which + /// ends in , even for an unchanged value, so an unguarded + /// re-activation would move the active-pane highlight to the dockable's dock. Callers that + /// do want the focus to move pair this with (see + /// ). + /// + public override void SetActiveDockable(IDockable dockable) + { + if (dockable.Owner is IDock { ActiveDockable: var active } && ReferenceEquals(active, dockable)) + return; + base.SetActiveDockable(dockable); + } + /// /// Makes both the active and the focused dockable within /// . The two always go together -- surfacing a dockable without From fa6b5f3b2645db9512d7a7e3c331231a28ed800b Mon Sep 17 00:00:00 2001 From: Christoph Wille Date: Tue, 6 Oct 2026 08:16:27 +0200 Subject: [PATCH 8/8] Let the editor gutter margins react to the left button only AvaloniaEdit's line-number margin moves the caret and selects the line, and a fold marker toggles its fold, on any pointer button; the WPF margins only reacted to the left button. A middle click opens a reference in a new tab over the text, so one that lands on the gutter should do nothing instead of folding code or moving the caret. The text area itself still sees every button. #4042 Assisted-by: Claude:claude-fable-5-1:Claude Code --- ILSpy.Tests/Editor/MarginPointerTests.cs | 120 +++++++++++++++++++++ ILSpy/TextView/DecompilerTextView.axaml.cs | 18 ++++ 2 files changed, 138 insertions(+) create mode 100644 ILSpy.Tests/Editor/MarginPointerTests.cs diff --git a/ILSpy.Tests/Editor/MarginPointerTests.cs b/ILSpy.Tests/Editor/MarginPointerTests.cs new file mode 100644 index 0000000000..d19394476b --- /dev/null +++ b/ILSpy.Tests/Editor/MarginPointerTests.cs @@ -0,0 +1,120 @@ +// Copyright (c) 2026 Christoph Wille +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + + +using System.Linq; +using System.Threading.Tasks; + +using Avalonia; +using Avalonia.Controls; +using Avalonia.Headless.NUnit; +using Avalonia.Input; +using Avalonia.VisualTree; + +using AvaloniaEdit.Editing; +using AvaloniaEdit.Folding; + +using AwesomeAssertions; + +using ICSharpCode.ILSpy.AppEnv; +using ICSharpCode.ILSpy.Options; +using ICSharpCode.ILSpy.TextView; +using ICSharpCode.ILSpy.TreeNodes; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.Editor; + +/// +/// The gutter margins react to the left button only, as they did in WPF: AvaloniaEdit's line-number +/// margin otherwise selects the line and a fold marker toggles on any button, so a middle press +/// (which opens references in a new tab over the text) would also move the caret or fold code when +/// it lands on the gutter. +/// +[TestFixture] +public class MarginPointerTests +{ + static async Task<(ICSharpCode.ILSpy.Views.MainWindow Window, DecompilerTextView View)> SetupAsync() + { + var (window, vm) = await TestHarness.BootAsync(1); + AppComposition.Current.GetExport().DisplaySettings.ShowLineNumbers = true; + + // A type (not a single method) so the decompiled body carries foldings and the folding margin + // is installed. + var coreLibName = typeof(object).Assembly.GetName().Name!; + var objectNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.Object"); + vm.AssemblyTreeModel.SelectNode(objectNode); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + var view = await window.WaitForComponent(); + await Waiters.WaitForAsync(() => view.Editor.TextArea.LeftMargins.OfType().Any(), + description: "the folding margin to be installed for the decompiled type"); + await Waiters.WaitForIdleAsync(); + return (window, view); + } + + [AvaloniaTest] + public async Task Middle_Press_On_The_Line_Number_Margin_Does_Not_Select_The_Line() + { + // The line-number margin draws text only, which the headless hit test does not see, so the + // press is raised on the margin directly; it still tunnels down from the window through the + // text area, which is where the margins' button filter sits. + var (window, view) = await SetupAsync(); + var margin = view.Editor.TextArea.LeftMargins.OfType().Single(); + var caretBefore = view.Editor.CaretOffset; + + margin.RaiseEvent(MiddlePress(window, margin, new Point(margin.Bounds.Width / 2, margin.Bounds.Height / 4))); + await Waiters.WaitForIdleAsync(); + + view.Editor.TextArea.Selection.IsEmpty.Should().BeTrue("a middle press on the line numbers must not select the line"); + view.Editor.CaretOffset.Should().Be(caretBefore, "a middle press on the line numbers must not move the caret"); + + margin.RaiseEvent(LeftPress(window, margin, new Point(margin.Bounds.Width / 2, margin.Bounds.Height / 4))); + await Waiters.WaitForIdleAsync(); + view.Editor.CaretOffset.Should().NotBe(caretBefore, "the same press with the left button moves the caret, so the middle press really reached the margin"); + } + + static PointerPressedEventArgs MiddlePress(Window window, Control target, Avalonia.Point position) + => Press(window, target, position, RawInputModifiers.MiddleMouseButton, PointerUpdateKind.MiddleButtonPressed); + + static PointerPressedEventArgs LeftPress(Window window, Control target, Avalonia.Point position) + => Press(window, target, position, RawInputModifiers.LeftMouseButton, PointerUpdateKind.LeftButtonPressed); + + static PointerPressedEventArgs Press(Window window, Control target, Avalonia.Point position, + RawInputModifiers button, PointerUpdateKind kind) + => new(target, new Pointer(Pointer.GetNextFreeId(), PointerType.Mouse, isPrimary: true), window, + target.TranslatePoint(position, window)!.Value, 0, new PointerPointProperties(button, kind), KeyModifiers.None); + + [AvaloniaTest] + public async Task Middle_Press_On_A_Fold_Marker_Does_Not_Toggle_The_Fold() + { + var (window, view) = await SetupAsync(); + var foldingMargin = view.Editor.TextArea.LeftMargins.OfType().Single(); + Control? Marker() => foldingMargin.GetVisualChildren().OfType().FirstOrDefault(c => c.IsVisible); + Marker().Should().NotBeNull("the folding margin must show a marker for the decompiled type"); + var foldedBefore = view.FoldedFoldingCount; + + await window.ClickAsync(Marker, MouseButton.Middle); + await Waiters.WaitForIdleAsync(); + view.FoldedFoldingCount.Should().Be(foldedBefore, "a middle press on a fold marker must not toggle the fold"); + + // The same marker toggles on a left click, so the middle press above really reached a marker. + await window.ClickAsync(Marker, MouseButton.Left); + await Waiters.WaitForAsync(() => view.FoldedFoldingCount != foldedBefore, + description: "a left click on the fold marker to toggle the fold"); + } +} diff --git a/ILSpy/TextView/DecompilerTextView.axaml.cs b/ILSpy/TextView/DecompilerTextView.axaml.cs index 617b7aae33..72f96b480e 100644 --- a/ILSpy/TextView/DecompilerTextView.axaml.cs +++ b/ILSpy/TextView/DecompilerTextView.axaml.cs @@ -259,6 +259,24 @@ void SetupElementGenerators() OnTextAreaPointerReleasedForReferenceClick, RoutingStrategies.Bubble, handledEventsToo: true); + // The gutter margins react to the left button only, like their WPF counterparts: + // AvaloniaEdit's line-number margin moves the caret and selects the line, and a fold + // marker toggles, on any button. Marking the press handled before it reaches the margin + // skips the margin's own press handler. The text area keeps seeing middle presses (a + // middle click on a reference opens it in a new tab). + Editor.TextArea.AddHandler(InputElement.PointerPressedEvent, + OnTextAreaPointerPressedForMargins, + RoutingStrategies.Tunnel); + } + + void OnTextAreaPointerPressedForMargins(object? sender, PointerPressedEventArgs e) + { + if (e.GetCurrentPoint(Editor.TextArea).Properties.PointerUpdateKind == PointerUpdateKind.LeftButtonPressed) + return; + if (e.Source is Visual hit + && hit.FindAncestorOfType(includeSelf: true) is { } margin + && Editor.TextArea.LeftMargins.Contains(margin)) + e.Handled = true; } // Position of the last link-button press, in this control's coordinates; null while no