diff --git a/ILSpy.Tests/AssemblyList/ReloadAssemblyContextMenuTests.cs b/ILSpy.Tests/AssemblyList/ReloadAssemblyContextMenuTests.cs index 35b57774c..f3b50c9ca 100644 --- a/ILSpy.Tests/AssemblyList/ReloadAssemblyContextMenuTests.cs +++ b/ILSpy.Tests/AssemblyList/ReloadAssemblyContextMenuTests.cs @@ -116,9 +116,7 @@ public class ReloadAssemblyContextMenuTests var menu = pane.BuildContextMenuForCurrentState(registry.Entries); menu.Should().NotBeNull(); - var reloadItem = menu!.Items.OfType() - .Single(i => (string?)i.Header == Resources._Reload); - reloadItem.RaiseEvent(new RoutedEventArgs(MenuItem.ClickEvent)); + menu!.ClickItem(Resources._Reload); window.Capture("reload-clicked"); // Assert โ€” the LoadedAssembly for that file name is a different instance now. diff --git a/ILSpy.Tests/AssemblyList/RemoveAssemblyContextMenuTests.cs b/ILSpy.Tests/AssemblyList/RemoveAssemblyContextMenuTests.cs index f9193e90e..e6b6e3045 100644 --- a/ILSpy.Tests/AssemblyList/RemoveAssemblyContextMenuTests.cs +++ b/ILSpy.Tests/AssemblyList/RemoveAssemblyContextMenuTests.cs @@ -121,9 +121,7 @@ public class RemoveAssemblyContextMenuTests var menu = pane.BuildContextMenuForCurrentState(registry.Entries); menu.Should().NotBeNull(); - var removeItem = menu!.Items.OfType() - .Single(i => (string?)i.Header == Resources._Remove); - removeItem.RaiseEvent(new RoutedEventArgs(MenuItem.ClickEvent)); + menu!.ClickItem(Resources._Remove); window.Capture("remove-clicked"); // Assert โ€” the assembly is gone from the list; the survivor is still there. diff --git a/ILSpy.Tests/ContextMenus/DecompileInNewViewTests.cs b/ILSpy.Tests/ContextMenus/DecompileInNewViewTests.cs index 2eebe4374..e4de6a9c4 100644 --- a/ILSpy.Tests/ContextMenus/DecompileInNewViewTests.cs +++ b/ILSpy.Tests/ContextMenus/DecompileInNewViewTests.cs @@ -19,9 +19,15 @@ using System.Linq; using System.Threading.Tasks; +using Avalonia; using Avalonia.Controls; +using Avalonia.Controls.DataGridHierarchical; +using Avalonia.Headless; using Avalonia.Headless.NUnit; +using Avalonia.Input; using Avalonia.Interactivity; +using Avalonia.Threading; +using Avalonia.VisualTree; using AwesomeAssertions; @@ -99,13 +105,14 @@ public class DecompileInNewViewTests .Should().NotContain(header); } - // audit (2026-05-12): converted ๐Ÿ“ฆ โ†’ ๐Ÿงช. Was firing `entry.Execute(synthetic ctx)` - // directly; now selects the second method via the model and clicks the - // "Decompile in new tab" MenuItem produced by the live menu-build path. The Click - // routed event hits the same handler ContextMenuProvider attached during Build โ€” - // matching what the user's right-click โ†’ menu-item-click triggers. + // audit (2026-06-02): rewritten for the Thunderbird-style context target. Previously this + // test selected the second method via the model first โ€” encoding the OLD bug where + // right-clicking a row moved the selection there before the menu opened, so you ended up + // with the right-clicked node twice (the preview reused for it + the new tab) instead of + // "keep A, open B". Now A stays the active document and the menu targets B directly, so the + // result is A + B: the new tab shows B and A is untouched. [AvaloniaTest] - public async Task Clicking_DecompileInNewView_Spawns_A_New_Tab_For_The_Selected_Method() + public async Task Clicking_DecompileInNewView_Opens_The_Right_Clicked_Method_While_Keeping_The_Active_One() { var window = AppComposition.Current.GetExport(); window.Show(); @@ -119,41 +126,164 @@ public class DecompileInNewViewTests var secondMethod = typeNode.Children.OfType() .First(m => m.MethodDefinition.Name == "Empty"); - // Seed an existing decompile in the active tab so we have a "current" tab on screen - // before the gesture; that tab will be reused by the right-click's selection move - // (production behaviour โ€” right-clicking a row in the DataGrid moves selection there - // before the menu opens), so the assertion we care about is "a *new* tab spawned in - // addition to the existing one and shows the dispatched method". + // A is the active document: select firstMethod and decompile it into the preview tab. vm.AssemblyTreeModel.SelectNode(firstMethod); var firstTab = await vm.DockWorkspace.WaitForDecompiledTextAsync(); - window.Capture("first-method-decompiled-into-active-tab"); + firstTab.Text.Should().Contain("AsEnumerable"); + window.Capture("first-method-active"); var pane = await window.WaitForComponent(); var registry = AppComposition.Current.GetExport(); var documents = ((ILSpyDockFactory)vm.DockWorkspace.Factory).Documents!; var initialCount = documents.VisibleDockables?.Count ?? 0; - // Select the second method and click the menu entry through the live context-menu path. - vm.AssemblyTreeModel.SelectNode(secondMethod); - await vm.DockWorkspace.WaitForDecompiledTextAsync(); - window.Capture("second-method-replaced-active-tab"); - - var menu = pane.BuildContextMenuForCurrentState(registry.Entries); + // Right-click B (secondMethod) WITHOUT moving the selection, and click "Decompile to + // new tab" from the menu the right-click would produce. + var menu = pane.BuildContextMenuForCurrentState(registry.Entries, rightClickedNode: secondMethod); menu.Should().NotBeNull(); - var item = menu!.Items.OfType() - .Single(i => (string?)i.Header == Resources.DecompileToNewPanel); - item.RaiseEvent(new RoutedEventArgs(MenuItem.ClickEvent)); + menu!.ClickItem(Resources.DecompileToNewPanel); await Waiters.WaitForAsync( () => (documents.VisibleDockables?.Count ?? 0) > initialCount); var newTab = await vm.DockWorkspace.WaitForDecompiledTextAsync(); + + // The new tab shows B... ReferenceEquals(newTab, firstTab).Should().BeFalse( "a fresh decompiler tab must be created instead of reusing the existing one"); newTab.Text.Should().Contain("Empty"); - // Count grew by one โ€” the click added a tab on top of the right-click selection's reuse - // of firstTab. (The right-click selection move into secondMethod re-uses firstTab; the - // menu-click then creates an additional tab. Net effect: +1 dockable.) + // ...the original tab still shows A (it was NOT yanked to B by the right-click โ€” this is + // the whole point: you keep A and gain B, instead of ending up with B twice)... + firstTab.Text.Should().Contain("AsEnumerable"); + // ...and exactly one tab was added (A + B), not two copies of B. documents.VisibleDockables!.Count.Should().Be(initialCount + 1); - window.Capture("after-click-new-tab-spawned"); + window.Capture("a-kept-b-opened"); + } + + [AvaloniaTest] + public async Task Right_Clicking_An_Unselected_Row_Does_Not_Move_The_Selection() + { + // Thunderbird-style context target: right-clicking a row the user has NOT selected must + // leave the real selection (and therefore the preview document) untouched. Today + // ProDataGrid's default selects the right-clicked row on press, which yanks the preview + // to B before the menu opens โ€” so "Decompile to new tab" ends up with B twice instead of + // the intended A + B. This probe asserts the selection stays put under a real right-click. + + var window = AppComposition.Current.GetExport(); + window.Show(); + var vm = (MainWindowViewModel)window.DataContext!; + await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3); + var pane = await window.WaitForComponent(); + var grid = await pane.WaitForComponent(); + + var nodeA = vm.AssemblyTreeModel.FindNode("System.Linq"); + var nodeB = vm.AssemblyTreeModel.FindNode(TreeNavigation.CoreLibName); + + // Make A the active document/selection. + vm.AssemblyTreeModel.SelectNode(nodeA); + + // Let the top-level rows realise and layout settle. + for (int i = 0; i < 8; i++) + { + Dispatcher.UIThread.RunJobs(); + grid.UpdateLayout(); + await Task.Delay(25); + } + + var rowB = grid.GetVisualDescendants().OfType() + .FirstOrDefault(r => RowNodeEquals(r, nodeB)); + rowB.Should().NotBeNull("the top-level CoreLib row must be realised"); + + // Suppressing the right-press must not also suppress the context menu โ€” it still opens + // from ContextRequested (raised on release). Watch for it. + var menuRequested = false; + grid.AddHandler(Control.ContextRequestedEvent, (_, _) => menuRequested = true, + handledEventsToo: true); + + // Right-click the centre of B's row (clear of the far-left expander glyph). + var point = rowB!.TranslatePoint(new Point(rowB.Bounds.Width / 2, rowB.Bounds.Height / 2), window); + point.Should().NotBeNull(); + HeadlessWindowExtensions.MouseDown(window, point!.Value, MouseButton.Right); + HeadlessWindowExtensions.MouseUp(window, point.Value, MouseButton.Right); + for (int i = 0; i < 4; i++) + { + Dispatcher.UIThread.RunJobs(); + await Task.Delay(20); + } + + ReferenceEquals(vm.AssemblyTreeModel.SelectedItem, nodeA).Should().BeTrue( + "right-clicking an unselected row must not change the selection (Thunderbird-style context target)"); + menuRequested.Should().BeTrue( + "the context menu must still be requested after the suppressed right-press"); + rowB.Classes.Should().Contain("contextTarget", + "the right-clicked row must carry the focus-box style class while it is the menu target"); } + + [AvaloniaTest] + public async Task Right_Clicking_A_Second_Row_Moves_The_Context_Highlight_To_It() + { + // Regression: the focus box must follow every right-click, not stick after the first one + // and then never appear again. Reproduces "right-click B (highlight), dismiss, right-click + // C (no highlight)". + var window = AppComposition.Current.GetExport(); + window.Show(); + var vm = (MainWindowViewModel)window.DataContext!; + await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3); + var pane = await window.WaitForComponent(); + var grid = await pane.WaitForComponent(); + + var assemblies = vm.AssemblyTreeModel.Root!.Children.OfType().Take(3).ToArray(); + assemblies.Length.Should().BeGreaterThanOrEqualTo(3, "need three top-level rows to click"); + var nodeA = assemblies[0]; + var nodeB = assemblies[1]; + var nodeC = assemblies[2]; + + vm.AssemblyTreeModel.SelectNode(nodeA); + for (int i = 0; i < 8; i++) + { + Dispatcher.UIThread.RunJobs(); + grid.UpdateLayout(); + await Task.Delay(25); + } + + DataGridRow Row(SharpTreeNode node) => grid.GetVisualDescendants().OfType() + .First(r => RowNodeEquals(r, node)); + + async Task RightClick(SharpTreeNode node) + { + var row = Row(node); + var pt = row.TranslatePoint(new Point(row.Bounds.Width / 2, row.Bounds.Height / 2), window); + HeadlessWindowExtensions.MouseDown(window, pt!.Value, MouseButton.Right); + HeadlessWindowExtensions.MouseUp(window, pt.Value, MouseButton.Right); + for (int i = 0; i < 4; i++) + { + Dispatcher.UIThread.RunJobs(); + await Task.Delay(20); + } + } + + async Task Dismiss() + { + window.KeyPress(Key.Escape, RawInputModifiers.None, PhysicalKey.Escape, keySymbol: null); + for (int i = 0; i < 4; i++) + { + Dispatcher.UIThread.RunJobs(); + await Task.Delay(20); + } + } + + await RightClick(nodeB); + Row(nodeB).Classes.Should().Contain("contextTarget", "the first right-click highlights B"); + + // Dismiss B's menu, then right-click C โ€” the user's exact sequence. + await Dismiss(); + await RightClick(nodeC); + Row(nodeC).Classes.Should().Contain("contextTarget", + "a second right-click (after dismissing the first menu) must highlight C"); + Row(nodeB).Classes.Should().NotContain("contextTarget", + "B's highlight must be gone"); + } + + static bool RowNodeEquals(DataGridRow row, SharpTreeNode node) + => row.DataContext is HierarchicalNode hn && ReferenceEquals(hn.Item, node); + } diff --git a/ILSpy.Tests/TestHarness.cs b/ILSpy.Tests/TestHarness.cs index f0728cf8e..39671d189 100644 --- a/ILSpy.Tests/TestHarness.cs +++ b/ILSpy.Tests/TestHarness.cs @@ -21,6 +21,9 @@ using System.Linq; using System.Threading.Tasks; using System.Windows.Input; +using Avalonia.Controls; +using Avalonia.Interactivity; + using ICSharpCode.ILSpy.Properties; using ICSharpCode.ILSpyX; @@ -83,6 +86,23 @@ public static class TestHarness .Value; } + /// + /// Finds the whose Header equals (pass + /// the resolved resource string, e.g. Resources._Reload) in a menu built by + /// AssemblyListPane.BuildContextMenuForCurrentState and raises its Click โ€” the + /// same routed event the production menu fires when the user picks the item. Collapses the + /// Items.OfType<MenuItem>().Single(...).RaiseEvent(...) dance every context-menu + /// test repeats. + /// + public static void ClickItem(this ContextMenu menu, string header) + { + ArgumentNullException.ThrowIfNull(menu); + ArgumentNullException.ThrowIfNull(header); + var item = menu.Items.OfType() + .Single(i => string.Equals((string?)i.Header, header, StringComparison.Ordinal)); + item.RaiseEvent(new RoutedEventArgs(MenuItem.ClickEvent)); + } + /// /// Opens through the production Open command, waits for it to appear /// in the assembly list, awaits its load result, and returns the resulting diff --git a/ILSpy/App.axaml b/ILSpy/App.axaml index 0241d76c4..1bef450a4 100644 --- a/ILSpy/App.axaml +++ b/ILSpy/App.axaml @@ -55,6 +55,7 @@ + @@ -92,6 +93,7 @@ + diff --git a/ILSpy/AssemblyTree/AssemblyListPane.axaml b/ILSpy/AssemblyTree/AssemblyListPane.axaml index 7a839a8d2..56aff2cea 100644 --- a/ILSpy/AssemblyTree/AssemblyListPane.axaml +++ b/ILSpy/AssemblyTree/AssemblyListPane.axaml @@ -16,6 +16,15 @@ + + { + if (!ReferenceEquals(contextTargetRow, contextMenuOpenRow)) + return; + contextMenuTargetNode = null; + SetContextTargetRow(null); + }; TreeGrid.ContextMenu = menu; } @@ -172,6 +222,9 @@ namespace ILSpy.AssemblyTree { if (sender is not ContextMenu menu) return; + // Remember which row this menu belongs to, so its later Closed only clears the + // highlight if no newer right-click has moved the target to a different row. + contextMenuOpenRow = contextTargetRow; var built = BuildContextMenuForCurrentState(contextMenuEntries); if (built == null) { @@ -196,10 +249,38 @@ namespace ILSpy.AssemblyTree internal ContextMenu? BuildContextMenuForCurrentState(IReadOnlyList entries) => ContextMenuProvider.Build(entries, CreateContextMenuContext()); + /// + /// Test seam: builds the menu as if had been + /// right-clicked (the Thunderbird-style context target), without simulating a pointer + /// gesture on a possibly-unrealised deep row. Production sets the same target from the + /// tunnel right-press handler. The built menu captures the resulting context, so the + /// target is cleared again before returning. + /// + internal ContextMenu? BuildContextMenuForCurrentState( + IReadOnlyList entries, SharpTreeNode? rightClickedNode) + { + contextMenuTargetNode = rightClickedNode; + try + { + return ContextMenuProvider.Build(entries, CreateContextMenuContext()); + } + finally + { + contextMenuTargetNode = null; + } + } + TextViewContext CreateContextMenuContext() { - var nodes = (DataContext as AssemblyTreeModel)?.SelectedItems.ToArray() + var selection = (DataContext as AssemblyTreeModel)?.SelectedItems.ToArray() ?? System.Array.Empty(); + // A right-click outside the current selection targets just the clicked row, leaving + // the real selection (and preview document) untouched. A right-click inside the + // selection, or a keyboard-invoked menu (no target), acts on the whole selection. + var target = contextMenuTargetNode; + var nodes = target != null && !System.Array.Exists(selection, n => ReferenceEquals(n, target)) + ? new SharpTreeNode[] { target } + : selection; return new TextViewContext { TreeGrid = TreeGrid, SelectedTreeNodes = nodes, @@ -403,6 +484,43 @@ namespace ILSpy.AssemblyTree } } + void OnTreeGridContextRequested(object? sender, ContextRequestedEventArgs e) + { + // Resolve the row under the pointer (mouse / touch context gesture). A keyboard-invoked + // menu (Shift+F10 / Menu key) carries no position -> no target -> the menu acts on the + // real selection. Each request is a new generation so a stale menu's Closed can't wipe + // this fresh highlight. + SharpTreeNode? node = null; + DataGridRow? row = null; + if (e.TryGetPosition(TreeGrid, out var pos) && TreeGrid.InputHitTest(pos) is Visual hit) + { + node = HitTestRowNode(hit); + row = hit.GetVisualAncestors().OfType().FirstOrDefault(); + } + contextMenuTargetNode = node; + SetContextTargetRow(node != null ? row : null); + } + + void OnTreeGridPointerPressedPreview(object? sender, PointerPressedEventArgs e) + { + if (e.Source is not Visual hit) + return; + if (!e.GetCurrentPoint(hit).Properties.IsRightButtonPressed) + { + // Any non-right press (left navigation, middle new-tab) starts a fresh gesture โ€” + // drop a stale right-click target and its focus box. + contextMenuTargetNode = null; + SetContextTargetRow(null); + return; + } + // Right press over a row: swallow it so ProDataGrid doesn't move the selection there. + // We do NOT capture the menu target here โ€” when a previous menu is open this press is + // eaten by that menu's light-dismiss popup and never reaches us, so capture happens in + // OnTreeGridContextRequested instead (it fires on every menu open, including reopens). + if (HitTestRowNode(hit) != null) + e.Handled = true; + } + void OnTreeGridPointerPressed(object? sender, PointerPressedEventArgs e) { // MMB single-click is the only "open the row's node in a new tab" pointer gesture โ€”