Browse Source

ViewState — Back/Forward restores caret + scroll position

Closes two tracker items in one commit:
- `BrowseForwardCommand` was already wired end-to-end (command, toolbar
  button, Alt+Right key binding). New test pins the wiring so a future
  regression to any of the three surfaces is caught.
- `ViewState (caret + scroll persistence across navigation)` was missing.
  Navigating back to a previously-visited node landed the caret at offset
  0 / scroll at top instead of where the user was looking before they
  moved away.
pull/3755/head
Siegfried Pammer 2 months ago
parent
commit
3c527fc3ad
  1. 140
      ILSpy.Tests/Navigation/ViewStateRoundTripTests.cs
  2. 37
      ILSpy/Docking/DockWorkspace.cs
  3. 18
      ILSpy/NavigationEntry.cs
  4. 8
      ILSpy/NavigationHistory.cs
  5. 31
      ILSpy/TextView/DecompilerTabPageModel.cs
  6. 45
      ILSpy/TextView/DecompilerTextView.axaml.cs

140
ILSpy.Tests/Navigation/ViewStateRoundTripTests.cs

@ -0,0 +1,140 @@ @@ -0,0 +1,140 @@
// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team
//
// 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.Headless.NUnit;
using AwesomeAssertions;
using ILSpy.AppEnv;
using ILSpy.Navigation;
using ILSpy.TextView;
using ILSpy.TreeNodes;
using ILSpy.ViewModels;
using ILSpy.Views;
using NUnit.Framework;
namespace ICSharpCode.ILSpy.Tests.Navigation;
/// <summary>
/// End-to-end: select node A, move the caret, select node B, navigate Back. The caret
/// must land where the user left it on A — that's the contract for Back/Forward to
/// feel like a real browser instead of a "reset to top" gesture.
/// </summary>
[TestFixture]
public class ViewStateRoundTripTests
{
[AvaloniaTest]
public async Task Back_Restores_Caret_Position_The_User_Left_On_The_Previous_Node()
{
var window = AppComposition.Current.GetExport<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 1);
var dockWorkspace = vm.DockWorkspace;
// Pick two distinct decompiler targets so the navigation actually moves between
// them. CoreLib's System.Object and System.String are both always present.
var coreLibName = typeof(object).Assembly.GetName().Name!;
var objectNode = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(coreLibName, "System", "System.Object");
var stringNode = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(coreLibName, "System", "System.String");
vm.AssemblyTreeModel.SelectNode(objectNode);
await dockWorkspace.WaitForDecompiledTextAsync();
var tab = dockWorkspace.ActiveDecompilerTab!;
// Simulate the user moving the caret on node A. The view's PositionChanged handler
// is wired to push LastKnownCaretOffset onto the model, but headless tests don't
// fire that event by simulating user clicks — write the LastKnown values directly
// to model the post-interaction state.
tab.LastKnownCaretOffset = 500;
tab.LastKnownVerticalOffset = 120.5;
tab.LastKnownHorizontalOffset = 7.25;
// Navigate to a different node — captures A's state into the history's current
// entry and records B as the new current.
vm.AssemblyTreeModel.SelectNode(stringNode);
await dockWorkspace.WaitForDecompiledTextAsync();
// Verify the capture: the back stack's most recent entry should be A's, with
// the caret + scroll values we set.
var backEntries = dockWorkspace.BackHistory.OfType<TreeNodeEntry>().ToList();
backEntries.Should().NotBeEmpty("Select(B) must push A onto the back stack");
var captured = backEntries.Last();
ReferenceEquals(captured.Node, objectNode).Should().BeTrue(
"captured back-stack entry must reference node A");
captured.CaretOffset.Should().Be(500);
captured.VerticalOffset.Should().Be(120.5);
captured.HorizontalOffset.Should().Be(7.25);
// Navigate Back. The handler should set Pending* on the tab; the editor view
// applies them once Text lands.
dockWorkspace.NavigateBackCommand.Execute(null);
await dockWorkspace.WaitForDecompiledTextAsync();
// After the decompile lands the view consumes Pending* and clears them. To
// verify the round-trip without a real editor in headless mode, assert directly
// on the tab's LastKnown* — the view's caret-change handler writes those when
// it programmatically moves the caret in ApplyDocument.
tab.LastKnownCaretOffset.Should().Be(500,
"Back must restore the caret to where the user left it before navigating to B");
tab.LastKnownVerticalOffset.Should().Be(120.5,
"Back must restore the vertical scroll offset");
}
[AvaloniaTest]
public async Task Forward_Is_Wired_Through_The_Toolbar_And_Key_Binding()
{
// Pins the existing Forward command wiring: the tracker had listed BrowseForward
// as missing, but it was already implemented. This test makes a regression in any
// of the three wirings (command, toolbar button, Alt+Right) detectable.
var window = AppComposition.Current.GetExport<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 1);
((object?)vm.DockWorkspace.NavigateForwardCommand).Should().NotBeNull();
((object?)vm.DockWorkspace.NavigateBackCommand).Should().NotBeNull();
// Build up history: A → B, then go Back so Forward is enabled.
var coreLibName = typeof(object).Assembly.GetName().Name!;
var objectNode = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(coreLibName, "System", "System.Object");
var stringNode = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(coreLibName, "System", "System.String");
vm.AssemblyTreeModel.SelectNode(objectNode);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
vm.AssemblyTreeModel.SelectNode(stringNode);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
vm.DockWorkspace.NavigateBackCommand.Execute(null);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
vm.DockWorkspace.NavigateForwardCommand.CanExecute(null).Should().BeTrue(
"Forward must be enabled after Back leaves an entry on the forward stack");
vm.DockWorkspace.NavigateForwardCommand.Execute(null);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
// Back through Forward should land on B again.
ReferenceEquals(vm.AssemblyTreeModel.SelectedItem, stringNode).Should().BeTrue(
"NavigateForward must restore the tree selection to the entry that was just popped from forward");
}
}

37
ILSpy/Docking/DockWorkspace.cs

@ -225,11 +225,37 @@ namespace ILSpy.Docking @@ -225,11 +225,37 @@ namespace ILSpy.Docking
void RecordHistoryEntry(NavigationEntry entry)
{
// Snapshot the editor's caret + scroll state into the OUTGOING entry — at this
// point the editor still shows the previous selection's content, so the values
// describe where the user was looking before this new entry takes over. The
// captured state is then restored if/when the user navigates Back/Forward to it.
CaptureCurrentViewState();
history.Record(entry);
NavigateBackCommand.NotifyCanExecuteChanged();
NavigateForwardCommand.NotifyCanExecuteChanged();
}
void CaptureCurrentViewState()
{
if (history.Current is not TreeNodeEntry current)
return;
var decompTab = UnwrapDecompilerTab(current.Tab);
if (decompTab == null)
return;
current.CaretOffset = decompTab.LastKnownCaretOffset;
current.VerticalOffset = decompTab.LastKnownVerticalOffset;
current.HorizontalOffset = decompTab.LastKnownHorizontalOffset;
}
// The recorded TabPageModel in TreeNodeEntry can be either a DecompilerTabPageModel
// directly OR a ContentTabPage whose Content is one — `factory.Documents.ActiveDockable`
// returns the latter in the normal layout. Try both shapes.
static TextView.DecompilerTabPageModel? UnwrapDecompilerTab(ViewModels.TabPageModel? tab) => tab switch {
TextView.DecompilerTabPageModel d => d,
ViewModels.ContentTabPage c => c.Content as TextView.DecompilerTabPageModel,
_ => null,
};
void NavigateBack() => NavigateHistory(forward: false);
void NavigateForward() => NavigateHistory(forward: true);
@ -261,7 +287,18 @@ namespace ILSpy.Docking @@ -261,7 +287,18 @@ namespace ILSpy.Docking
if (factory.Documents?.VisibleDockables is { } docs && docs.Contains(target.Tab))
factory.SetActiveDockable(target.Tab);
if (target is TreeNodeEntry treeNode)
{
// Stash the captured view state on the tab BEFORE setting SelectedItem.
// SelectedItem triggers the decompile; the editor's ApplyDocument reads
// the Pending* fields right after Text lands and restores caret + scroll.
if (UnwrapDecompilerTab(treeNode.Tab) is { } decompTab)
{
decompTab.PendingCaretOffset = treeNode.CaretOffset;
decompTab.PendingVerticalOffset = treeNode.VerticalOffset;
decompTab.PendingHorizontalOffset = treeNode.HorizontalOffset;
}
assemblyTreeModel.SelectedItem = treeNode.Node;
}
// StaticPageEntry: just reactivating the tab is enough — its content was
// preserved because IsStaticContent kept tree-node selections from targeting it.
}

18
ILSpy/NavigationEntry.cs

@ -57,12 +57,28 @@ namespace ILSpy.Navigation @@ -57,12 +57,28 @@ namespace ILSpy.Navigation
/// <summary>
/// History entry for a tree-node selection. Replaying activates the recorded tab and sets
/// the assembly-tree selection to <see cref="Node"/>.
/// the assembly-tree selection to <see cref="Node"/>. The optional caret + scroll fields
/// capture the editor's view state when the user navigated AWAY from this entry, so
/// Back/Forward restores cursor position and scroll position alongside the selection.
/// </summary>
public sealed class TreeNodeEntry : NavigationEntry
{
public SharpTreeNode Node { get; }
/// <summary>
/// Caret offset (character index into the decompiled text) captured when the user
/// navigated away from this entry. <c>null</c> means "no capture yet" — entries
/// land with null and are filled in by <see cref="Docking.DockWorkspace"/>'s
/// record-history hook just before a new entry takes over.
/// </summary>
public int? CaretOffset { get; set; }
/// <summary>Vertical scroll offset captured at navigate-away time.</summary>
public double? VerticalOffset { get; set; }
/// <summary>Horizontal scroll offset captured at navigate-away time.</summary>
public double? HorizontalOffset { get; set; }
public TreeNodeEntry(TabPageModel tab, SharpTreeNode node)
: base(tab)
{

8
ILSpy/NavigationHistory.cs

@ -39,6 +39,14 @@ namespace ILSpy.Navigation @@ -39,6 +39,14 @@ namespace ILSpy.Navigation
public bool CanNavigateBack => back.Count > 0;
public bool CanNavigateForward => forward.Count > 0;
/// <summary>
/// The most recently navigated-to entry, or null if no navigation has happened
/// yet. Mutable callers (e.g. <c>DockWorkspace.CaptureCurrentViewState</c>) reach
/// in here to stamp the editor's caret + scroll state into the entry just before
/// it gets pushed onto the back stack — so a subsequent Back restores the position.
/// </summary>
public T? Current => current;
// Read-only views over the history stacks for the toolbar's split-button dropdowns.
// Both lists are oldest-first (matches push/append order); the UI reverses for "newest
// first" display.

31
ILSpy/TextView/DecompilerTabPageModel.cs

@ -135,6 +135,37 @@ namespace ILSpy.TextView @@ -135,6 +135,37 @@ namespace ILSpy.TextView
[ObservableProperty]
private object? highlightedReference;
/// <summary>
/// Last caret offset the editor view reported. The text view writes this on every
/// caret-position change so <see cref="Docking.DockWorkspace"/> can read the current
/// position when recording a navigation away from this tab. Zero until the view
/// reports its first caret position; that's a safe default because a freshly-shown
/// document also has caret at zero.
/// </summary>
public int LastKnownCaretOffset { get; set; }
/// <summary>Last vertical scroll offset the editor view reported. See <see cref="LastKnownCaretOffset"/>.</summary>
public double LastKnownVerticalOffset { get; set; }
/// <summary>Last horizontal scroll offset the editor view reported. See <see cref="LastKnownCaretOffset"/>.</summary>
public double LastKnownHorizontalOffset { get; set; }
/// <summary>
/// Caret offset the editor view should restore on the next document apply. Set when
/// Back/Forward navigation lands on an entry that carries a captured caret position
/// (see <see cref="Navigation.TreeNodeEntry.CaretOffset"/>); cleared by the view
/// after it reads the value. The pattern mirrors <see cref="HighlightedReference"/> —
/// the editor consumes this in its <c>ApplyDocument</c> path so the value survives
/// the async gap between Back firing and the decompile finishing.
/// </summary>
public int? PendingCaretOffset { get; set; }
/// <summary>Vertical scroll offset to restore alongside <see cref="PendingCaretOffset"/>.</summary>
public double? PendingVerticalOffset { get; set; }
/// <summary>Horizontal scroll offset to restore alongside <see cref="PendingCaretOffset"/>.</summary>
public double? PendingHorizontalOffset { get; set; }
/// <summary>
/// Fired when the user clicks a cross-document reference. The host (DockWorkspace)
/// resolves the target on the assembly tree side.

45
ILSpy/TextView/DecompilerTextView.axaml.cs

@ -166,6 +166,28 @@ namespace ILSpy.TextView @@ -166,6 +166,28 @@ namespace ILSpy.TextView
// need to be in any visual tree of ours.
WireUpDisplaySettings();
// Push caret + scroll positions onto the bound tab model on every change so
// DockWorkspace can read the live state when recording a navigation away. Cheap
// — both events fire only on actual user motion and the model write is two
// integer / double assignments.
Editor.TextArea.Caret.PositionChanged += OnCaretPositionChanged;
Editor.TextArea.TextView.ScrollOffsetChanged += OnScrollOffsetChanged;
}
void OnCaretPositionChanged(object? sender, EventArgs e)
{
if (DataContext is DecompilerTabPageModel m)
m.LastKnownCaretOffset = Editor.TextArea.Caret.Offset;
}
void OnScrollOffsetChanged(object? sender, EventArgs e)
{
if (DataContext is DecompilerTabPageModel m)
{
m.LastKnownVerticalOffset = Editor.VerticalOffset;
m.LastKnownHorizontalOffset = Editor.HorizontalOffset;
}
}
/// <summary>
@ -385,6 +407,29 @@ namespace ILSpy.TextView @@ -385,6 +407,29 @@ namespace ILSpy.TextView
if (model.HighlightedReference != null)
HighlightLocalReferences(model, model.HighlightedReference);
// Restore caret + scroll position captured when the user previously navigated
// away from this node. Back/Forward (and the dropdown-history "GoTo") set the
// Pending* fields just before triggering the decompile; here we read them once
// and clear them so a subsequent user-initiated decompile doesn't jump the
// cursor again. Clamp to the new document length — text-changed-since-capture
// is the common case for Refresh.
if (model.PendingCaretOffset is { } caret)
{
model.PendingCaretOffset = null;
Editor.TextArea.Caret.Offset = Math.Clamp(caret, 0, Editor.Document.TextLength);
Editor.TextArea.Caret.BringCaretToView();
}
if (model.PendingVerticalOffset is { } vy)
{
model.PendingVerticalOffset = null;
Editor.ScrollToVerticalOffset(vy);
}
if (model.PendingHorizontalOffset is { } hx)
{
model.PendingHorizontalOffset = null;
Editor.ScrollToHorizontalOffset(hx);
}
// Swap any tab-contributed visual-line element generators (e.g. About-page hyperlink
// generators). Tracked separately from the always-on referenceElementGenerator and
// uiElementGenerator so the two sets don't collide on tab change.

Loading…
Cancel
Save