Fix #3476: Scroll position not restored when navigating back

Author: siegfriedpammer

ILSpy/AssemblyTree/AssemblyTreeModel.cs

@@ -21,6 +21,7 @@
 using System.Collections.Specialized;
 using System.ComponentModel;
 using System.Composition;
+using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Linq;
@@ -65,7 +66,7 @@ public class AssemblyTreeModel : ToolPaneModel
 		private readonly DispatcherThrottle refreshThrottle;
 
 		private readonly NavigationHistory<NavigationState> history = new();
-		private bool isNavigatingHistory;
+		private NavigationState? navigatingToState;
 		private readonly SettingsService settingsService;
 		private readonly LanguageService languageService;
 		private readonly IExportProvider exportProvider;
@@ -85,6 +86,7 @@ public AssemblyTreeModel(SettingsService settingsService, LanguageService langua
 			MessageBus<SettingsChangedEventArgs>.Subscribers += (sender, e) => Settings_PropertyChanged(sender, e);
 			MessageBus<ApplySessionSettingsEventArgs>.Subscribers += ApplySessionSettings;
 			MessageBus<ActiveTabPageChangedEventArgs>.Subscribers += ActiveTabPageChanged;
+			MessageBus<TabPagesCollectionChangedEventArgs>.Subscribers += (_, e) => history.RemoveAll(s => !DockWorkspace.TabPages.Contains(s.TabPage));
 			MessageBus<ResetLayoutEventArgs>.Subscribers += ResetLayout;
 			MessageBus<NavigateToEventArgs>.Subscribers += (_, e) => NavigateTo(e.Request, e.InNewTabPage);
 			MessageBus<MainWindowLoadedEventArgs>.Subscribers += (_, _) => {
@@ -153,9 +155,10 @@ public SharpTreeNode[] SelectedItems {
 				if (selectedItems.SequenceEqual(value))
 					return;
 
+				var oldSelection = selectedItems;
 				selectedItems = value;
 				OnPropertyChanged();
-				TreeView_SelectionChanged();
+				TreeView_SelectionChanged(oldSelection, selectedItems);
 			}
 		}
 
@@ -733,36 +736,45 @@ private void LoadAssemblies(IEnumerable<string> fileNames, List<LoadedAssembly>?
 
 		#region Decompile (TreeView_SelectionChanged)
 
-		private void TreeView_SelectionChanged()
+		private void TreeView_SelectionChanged(SharpTreeNode[] oldSelection, SharpTreeNode[] newSelection)
 		{
-			if (SelectedItems.Length <= 0)
+			var activeTabPage = DockWorkspace.ActiveTabPage;
+			ViewState? oldState = activeTabPage.GetState();
+			ViewState? newState;
+
+			if (navigatingToState == null)
 			{
-				// To cancel any pending decompilation requests and show an empty tab
-				DecompileSelectedNodes();
+				if (oldState != null)
+				{
+					history.UpdateCurrent(new NavigationState(activeTabPage, oldState));
+				}
+
+				newState = new ViewState { DecompiledNodes = [.. newSelection.Cast<ILSpyTreeNode>()] };
 			}
 			else
 			{
-				var activeTabPage = DockWorkspace.ActiveTabPage;
-
-				if (!isNavigatingHistory)
-				{
-					history.Record(new NavigationState(activeTabPage, SelectedItems));
-				}
+				newState = navigatingToState.ViewState;
+			}
 
+			if (newSelection.Length == 0)
+			{
+				// To cancel any pending decompilation requests and show an empty tab
+				DecompileSelectedNodes(newState);
+			}
+			else
+			{
 				var delayDecompilationRequestDueToContextMenu = Mouse.RightButton == MouseButtonState.Pressed;
 
 				if (!delayDecompilationRequestDueToContextMenu)
 				{
-					var decompiledNodes = activeTabPage
-						.GetState()
-						?.DecompiledNodes
+					var previousNodes = oldState?.DecompiledNodes
 						?.Select(n => FindNodeByPath(GetPathForNode(n), true))
 						.ExceptNullItems()
 						.ToArray() ?? [];
 
-					if (!decompiledNodes.SequenceEqual(SelectedItems))
+					if (!previousNodes.SequenceEqual(SelectedItems))
 					{
-						DecompileSelectedNodes();
+						DecompileSelectedNodes(newState);
 					}
 				}
 				else
@@ -790,7 +802,7 @@ void ContextMenuClosed(object? sender, EventArgs e)
 			}
 		}
 
-		public void DecompileSelectedNodes(DecompilerTextViewState? newState = null)
+		public void DecompileSelectedNodes(ViewState? newState = null)
 		{
 			var activeTabPage = DockWorkspace.ActiveTabPage;
 
@@ -801,6 +813,11 @@ public void DecompileSelectedNodes(DecompilerTextViewState? newState = null)
 
 			activeTabPage.SupportsLanguageSwitching = true;
 
+			if (newState != null && navigatingToState == null)
+			{
+				history.Record(new NavigationState(activeTabPage, newState));
+			}
+
 			if (SelectedItems.Length == 1)
 			{
 				if (SelectedItem is ILSpyTreeNode node && node.View(activeTabPage))
@@ -813,7 +830,7 @@ public void DecompileSelectedNodes(DecompilerTextViewState? newState = null)
 			}
 
 			var options = activeTabPage.CreateDecompilationOptions();
-			options.TextViewState = newState;
+			options.TextViewState = newState as DecompilerTextViewState;
 			activeTabPage.ShowTextViewAsync(textView => textView.DecompileAsync(this.CurrentLanguage, this.SelectedNodes, options));
 		}
 
@@ -834,26 +851,30 @@ public void NavigateHistory(bool forward)
 		{
 			try
 			{
-				isNavigatingHistory = true;
-
 				TabPageModel tabPage = DockWorkspace.ActiveTabPage;
 				var state = tabPage.GetState();
 				if (state != null)
 					history.UpdateCurrent(new NavigationState(tabPage, state));
 				var newState = forward ? history.GoForward() : history.GoBack();
+				navigatingToState = newState;
 
 				TabPageModel activeTabPage = newState.TabPage;
 
-				if (!DockWorkspace.TabPages.Contains(activeTabPage))
-					DockWorkspace.AddTabPage(activeTabPage);
-				else
-					DockWorkspace.ActiveTabPage = activeTabPage;
+				Debug.Assert(DockWorkspace.TabPages.Contains(activeTabPage));
+				DockWorkspace.ActiveTabPage = activeTabPage;
 
-				SelectNodes(newState.TreeNodes);
+				if (newState.TreeNodes.Any())
+				{
+					SelectNodes(newState.TreeNodes);
+				}
+				else if (newState.ViewState.ViewedUri != null)
+				{
+					NavigateTo(new(newState.ViewState.ViewedUri, null));
+				}
 			}
 			finally
 			{
-				isNavigatingHistory = false;
+				navigatingToState = null;
 			}
 		}
 
@@ -863,21 +884,46 @@ public void NavigateHistory(bool forward)
 
 		private void NavigateTo(RequestNavigateEventArgs e, bool inNewTabPage = false)
 		{
-			if (e.Uri.Scheme == "resource")
+			if (e.Uri.Scheme != "resource")
 			{
-				if (inNewTabPage)
+				return;
+			}
+
+			TabPageModel tabPage = DockWorkspace.ActiveTabPage;
+			ViewState? oldState = tabPage.GetState();
+			ViewState? newState;
+
+			if (navigatingToState == null)
+			{
+				if (oldState != null)
 				{
-					DockWorkspace.AddTabPage();
+					history.UpdateCurrent(new NavigationState(tabPage, oldState));
 				}
 
-				if (e.Uri.Host == "aboutpage")
+				newState = new ViewState { ViewedUri = e.Uri };
+
+				if (inNewTabPage)
 				{
-					RecordHistory();
-					MessageBus.Send(this, new ShowAboutPageEventArgs(DockWorkspace.ActiveTabPage));
-					e.Handled = true;
-					return;
+					tabPage = DockWorkspace.AddTabPage();
 				}
+			}
+			else
+			{
+				newState = navigatingToState.ViewState;
+				tabPage = DockWorkspace.ActiveTabPage = navigatingToState.TabPage;
+			}
+
+			bool needsNewNavigationEntry = !inNewTabPage && selectedItems?.Length == 0;
+
+			UnselectAll();
 
+			if (e.Uri.Host == "aboutpage")
+			{
+				MessageBus.Send(this, new ShowAboutPageEventArgs(DockWorkspace.ActiveTabPage));
+				e.Handled = true;
+			}
+			else
+			{
 				AvalonEditTextOutput output = new AvalonEditTextOutput {
 					Address = e.Uri,
 					Title = e.Uri.AbsolutePath,
@@ -896,23 +942,22 @@ private void NavigateTo(RequestNavigateEventArgs e, bool inNewTabPage = false)
 						}
 					}
 				}
-				RecordHistory();
 				DockWorkspace.ShowText(output);
 				e.Handled = true;
 			}
 
-			void RecordHistory()
+			if (navigatingToState == null)
 			{
-				if (isNavigatingHistory)
-					return;
-				TabPageModel tabPage = DockWorkspace.ActiveTabPage;
-				var currentState = tabPage.GetState();
-				if (currentState != null)
-					history.UpdateCurrent(new NavigationState(tabPage, currentState));
-
-				UnselectAll();
-
-				history.Record(new NavigationState(tabPage, new ViewState { ViewedUri = e.Uri }));
+				// the call to UnselectAll() above already creates a new navigation entry,
+				// we just need to make sure it contains something useful.
+				if (!needsNewNavigationEntry)
+				{
+					history.UpdateCurrent(new NavigationState(tabPage, tabPage.GetState()));
+				}
+				else
+				{
+					history.Record(new NavigationState(tabPage, tabPage.GetState()));
+				}
 			}
 		}
 
@@ -1039,8 +1084,8 @@ private void ActiveTabPageChanged(object? sender, ActiveTabPageChangedEventArgs
 			{
 				NavigateTo(new(state.ViewedUri, null));
 			}
-
 		}
+
 		private void ResetLayout(object? sender, ResetLayoutEventArgs e)
 		{
 			RefreshDecompiledView();

ILSpy/Commands/DecompileInNewViewCommand.cs

@@ -20,14 +20,12 @@
 using System.Collections.Generic;
 using System.Composition;
 using System.Linq;
-using System.Windows.Threading;
 
 using ICSharpCode.Decompiler.TypeSystem;
 using ICSharpCode.ILSpy.AssemblyTree;
 using ICSharpCode.ILSpy.Docking;
 using ICSharpCode.ILSpy.Properties;
 using ICSharpCode.ILSpy.TreeNodes;
-using ICSharpCode.ILSpy.ViewModels;
 
 using TomsToolbox.Essentials;
 
@@ -49,11 +47,7 @@ public bool IsEnabled(TextViewContext context)
 
 		public void Execute(TextViewContext context)
 		{
-			var activePane = dockWorkspace.ActivePane;
-
 			DecompileNodes(GetNodes(context).ToArray());
-
-			dockWorkspace.ActivePane = activePane;
 		}
 
 		IEnumerable<ILSpyTreeNode> GetNodes(TextViewContext context)
@@ -91,7 +85,7 @@ void DecompileNodes(ILSpyTreeNode[] nodes)
 			if (nodes.Length == 0)
 				return;
 
-			dockWorkspace.AddTabPage();
+			dockWorkspace.ActiveTabPage = dockWorkspace.AddTabPage();
 
 			if (assemblyTreeModel.SelectedItems.SequenceEqual(nodes))
 				assemblyTreeModel.DecompileSelectedNodes();

ILSpy/Docking/DockWorkspace.cs

@@ -116,6 +116,8 @@ private void TabPages_CollectionChanged(object sender, NotifyCollectionChangedEv
 			{
 				item.IsCloseable = canClose;
 			}
+
+			MessageBus.Send(this, new TabPagesCollectionChangedEventArgs(e));
 		}
 
 		public TabPageModel AddTabPage(TabPageModel tabPage = null)

ILSpy/TextView/DecompilerTextView.cs

@@ -1343,7 +1343,7 @@ public FoldingManager? FoldingManager {
 		#endregion
 	}
 
-	[DebuggerDisplay("Nodes = {DecompiledNodes}, ViewedUri = {ViewedUri}")]
+	[DebuggerDisplay($"{{{nameof(GetDebuggerDisplay)}(),nq}}")]
 	public class ViewState : IEquatable<ViewState>
 	{
 		public HashSet<ILSpyTreeNode>? DecompiledNodes;
@@ -1364,6 +1364,11 @@ static bool NullSafeSetEquals(HashSet<ILSpyTreeNode>? a, HashSet<ILSpyTreeNode>?
 				return a.SetEquals(b);
 			}
 		}
+
+		protected virtual string GetDebuggerDisplay()
+		{
+			return $"Nodes = {DecompiledNodes?.Count.ToString() ?? "<null>"}, ViewedUri = {ViewedUri?.ToString() ?? "<null>"}";
+		}
 	}
 
 	public class DecompilerTextViewState : ViewState
@@ -1432,6 +1437,11 @@ public override bool Equals(ViewState? other)
 			}
 			return false;
 		}
+
+		protected override string GetDebuggerDisplay()
+		{
+			return $"{base.GetDebuggerDisplay()}, ExpandMemberDefinitions = {ExpandMemberDefinitions}, VerticalOffset = {VerticalOffset}, HorizontalOffset = {HorizontalOffset}, FoldingsChecksum = {FoldingsChecksum}";
+		}
 	}
 
 	static class ExtensionMethods

ILSpy/Util/MessageBus.cs

@@ -75,6 +75,7 @@ public static implicit operator T(WrappedEventArgs<T> outer)
 	}
 
 	public class CurrentAssemblyListChangedEventArgs(NotifyCollectionChangedEventArgs e) : WrappedEventArgs<NotifyCollectionChangedEventArgs>(e);
+	public class TabPagesCollectionChangedEventArgs(NotifyCollectionChangedEventArgs e) : WrappedEventArgs<NotifyCollectionChangedEventArgs>(e);
 
 	public class SettingsChangedEventArgs(PropertyChangedEventArgs e) : WrappedEventArgs<PropertyChangedEventArgs>(e);