Don't recentre the tree when selecting an already-visible row

Center-on-select reveals a navigated-to node centred in the viewport, but it was also yanking rows the user can already see: opening a visible row in a new tab (or any model-driven selection) pulled it to the middle. The skip cannot be decided from post-selection geometry because AutoScrollToSelectedItem first drags an off-screen row to an edge, making it look visible. Snapshot visibility in TreeSelectionBinder.SyncModelToTree before the selection changes: an already-visible primary is focused without scrolling, while a genuinely off-screen one (Back navigation, go-to-definition) is still centred.

Assisted-by: Claude:claude-opus-4-8:Claude Code

ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs

@@ -684,6 +684,74 @@ public class AssemblyTreeTests
 			"clicking an already-visible row must not move the viewport");
 	}
 
+	[AvaloniaTest]
+	public async Task Selecting_A_Visible_Row_Via_The_Model_Does_Not_Recentre_Viewport()
+	{
+		// "Decompile to new tab" (and any model-driven navigation) selects the node in the tree, which
+		// syncs through TreeSelectionBinder -> ScrollIntoNodeView -> CenterNodeInView. Unlike a real
+		// mouse click, that path DOES run the centring code, so the in-viewport early-return must also
+		// cover it -- otherwise opening an already-visible item in a new tab yanks the tree to the centre.
+
+		var (window, vm) = await TestHarness.BootAsync(3);
+
+		var enumerable = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
+			"System.Linq", "System.Linq", "System.Linq.Enumerable");
+		enumerable.Expand();
+		var ns = (NamespaceTreeNode)enumerable.Parent!;
+		ns.Expand();
+		var asm = (AssemblyTreeNode)ns.Parent!;
+		asm.IsExpanded = true;
+		foreach (var child in ns.Children.OfType<TypeTreeNode>())
+		{
+			child.Expand();
+		}
+
+		var pane = await window.WaitForComponent<AssemblyListPane>();
+		var grid = await pane.WaitForComponent<global::ILSpy.Controls.TreeView.SharpTreeView>();
+
+		vm.AssemblyTreeModel.SelectNode(enumerable);
+		await Waiters.WaitForAsync(() => ReferenceEquals(vm.AssemblyTreeModel.SelectedItem, enumerable));
+		for (int i = 0; i < 8; i++)
+		{
+			Dispatcher.UIThread.RunJobs();
+			await Task.Delay(25);
+		}
+		grid.UpdateLayout();
+
+		var scrollViewer = await grid.WaitForComponent<ScrollViewer>();
+		(scrollViewer.Extent.Height - scrollViewer.Viewport.Height).Should().BeGreaterThan(50,
+			"the grid must have something to scroll for this test to be meaningful");
+		scrollViewer.Offset = new Vector(scrollViewer.Offset.X, 50);
+		grid.UpdateLayout();
+		Dispatcher.UIThread.RunJobs();
+		scrollViewer.Offset.Y.Should().BeGreaterThan(5,
+			"the test scenario requires the viewport be parked mid-list");
+
+		// Pick the bottom-most fully-visible non-selected row -- the strictest probe for an unwanted recentre.
+		var candidateRow = grid.GetVisualDescendants().OfType<global::ILSpy.Controls.TreeView.SharpTreeViewItem>()
+			.Where(r => !r.IsSelected
+				&& r.TranslatePoint(new Point(0, 0), scrollViewer) is { } p
+				&& p.Y >= 0 && p.Y + r.Bounds.Height <= scrollViewer.Viewport.Height)
+			.OrderByDescending(r => r.TranslatePoint(new Point(0, 0), scrollViewer)!.Value.Y)
+			.FirstOrDefault();
+		candidateRow.Should().NotBeNull("the test needs a visible non-selected row to select");
+		var candidateNode = candidateRow!.Node;
+		((object?)candidateNode).Should().NotBeNull();
+
+		var offsetBefore = scrollViewer.Offset.Y;
+
+		// Act — model-driven selection (the open-in-new-tab path), NOT a mouse click.
+		vm.AssemblyTreeModel.SelectNode(candidateNode);
+		for (int i = 0; i < 8; i++)
+		{
+			Dispatcher.UIThread.RunJobs();
+			await Task.Delay(25);
+		}
+
+		scrollViewer.Offset.Y.Should().BeApproximately(offsetBefore, 1.0,
+			"selecting an already-visible row via the model (e.g. Decompile to new tab) must not move the viewport");
+	}
+
 	[AvaloniaTest]
 	public async Task Save_Code_Command_Dispatches_Single_Selected_Node_Save_Override()
 	{

ILSpy/Controls/TreeView/SharpTreeView.cs

@@ -196,17 +196,35 @@ namespace ILSpy.Controls.TreeView
 			}
 		}
 
-		/// <summary>Scrolls the node into view and gives it keyboard focus.</summary>
-		public void FocusNode(SharpTreeNode node)
+		/// <summary>Scrolls the node into view (unless <paramref name="scroll"/> is false) and gives it
+		/// keyboard focus. Pass <c>scroll: false</c> to focus a row that is already visible without
+		/// disturbing the scroll position.</summary>
+		public void FocusNode(SharpTreeNode node, bool scroll = true)
 		{
 			ArgumentNullException.ThrowIfNull(node);
-			ScrollIntoNodeView(node);
+			if (scroll)
+				ScrollIntoNodeView(node);
 			if (ContainerFromItem(node) is { } container)
 				container.Focus();
 			else
 				Dispatcher.UIThread.Post(() => (ContainerFromItem(node))?.Focus(), DispatcherPriority.Loaded);
 		}
 
+		/// <summary>True when the node's row is realised and lies fully within the scroll viewport.
+		/// Used to decide, before a selection change scrolls the list, whether a reveal is needed at
+		/// all -- an already-visible row should not be pulled to the centre.</summary>
+		public bool IsNodeFullyVisible(SharpTreeNode node)
+		{
+			ArgumentNullException.ThrowIfNull(node);
+			var scrollViewer = this.GetVisualDescendants().OfType<ScrollViewer>().FirstOrDefault();
+			if (scrollViewer is null)
+				return false;
+			if (ContainerFromItem(node) is Control row && row.IsVisible
+				&& row.TranslatePoint(new Point(0, 0), scrollViewer) is { } top)
+				return top.Y >= 0 && top.Y + row.Bounds.Height <= scrollViewer.Viewport.Height;
+			return false;
+		}
+
 		/// <summary>Moves the (single) selection to <paramref name="node"/> and focuses it.
 		/// Used by keyboard navigation where selection must follow the caret.</summary>
 		void SelectAndFocus(SharpTreeNode node)
@@ -245,7 +263,8 @@ namespace ILSpy.Controls.TreeView
 			// keeps a re-selection of an already-centred row from twitching. We deliberately do NOT
 			// skip a merely-visible row sitting at an edge: the ListBox's AutoScrollToSelectedItem
 			// drags the selected row to the nearest edge first, and a reveal should still pull it to
-			// the centre from there.
+			// the centre from there. (Skipping an already-visible row is decided one level up, before
+			// AutoScroll runs, in the model->tree sync -- see TreeSelectionBinder.SyncModelToTree.)
 			if (ContainerFromItem(node) is Control visible && visible.IsVisible
 				&& visible.TranslatePoint(new Point(0, 0), scrollViewer) is { } top)
 			{

ILSpy/Controls/TreeView/TreeSelectionBinder.cs

@@ -18,6 +18,7 @@
 
 using System;
 using System.Collections;
+using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.Collections.Specialized;
 using System.Linq;
@@ -111,6 +112,16 @@ namespace ILSpy.Controls.TreeView
 			syncing = true;
 			try
 			{
+				// Snapshot which selected rows are already fully visible BEFORE mutating the selection:
+				// the change triggers the ListBox's AutoScrollToSelectedItem, which drags an off-screen
+				// row to an edge and would make it look "visible" by reveal time. A row that was already
+				// on screen must not be revealed/recentred (e.g. Decompile to new tab on a visible row);
+				// a genuinely off-screen one still gets centred (Back navigation, go-to-definition).
+				var visibleBefore = new HashSet<SharpTreeNode>(ReferenceEqualityComparer.Instance);
+				foreach (var node in modelSelection)
+					if (tree.IsNodeFullyVisible(node))
+						visibleBefore.Add(node);
+
 				tree.SelectedItems!.Clear();
 				SharpTreeNode? primary = null;
 				var items = tree.ItemsSource as IList;
@@ -132,9 +143,11 @@ namespace ILSpy.Controls.TreeView
 				// synchronous ScrollIntoView would throw "Invalid Arrange rectangle".
 				if (primary is { } toReveal)
 				{
+					bool wasVisible = visibleBefore.Contains(toReveal);
 					Dispatcher.UIThread.Post(() => {
-						tree.ScrollIntoNodeView(toReveal);
-						tree.FocusNode(toReveal);
+						if (!wasVisible)
+							tree.ScrollIntoNodeView(toReveal);
+						tree.FocusNode(toReveal, scroll: !wasVisible);
 					});
 				}
 			}