Browse Source

Position bookmark navigation from non-decompiler active content

Activating a bookmark only positioned the view when a decompiler tab was
already the active content: the pending bookmark was set on
DockWorkspace.ActiveDecompilerTab, which is null while a metadata table, the
Options page, or the About page is showing. Navigating from there selected and
decompiled the node but opened at the top with no line highlight.

The pending bookmark now travels to the decompiler model that ShowSelectedNode
routes the selection to, and is consumed in the document-apply step alongside
the view-state restore -- mirroring PendingViewState -- rather than reacting to
a property change. The earlier property-change path fired synchronously during
selection, scrolled, then a deferred document apply reset the caret to the top
with nothing left to re-apply. For a node that is already decompiled (re-shown
after an interlude, where no apply step runs) a ScrollToBookmark callback on the
model scrolls the live view directly, mirroring NavigateBookmarkInFile.

Assisted-by: Claude:claude-opus-4-8:Claude Code
pull/3839/head
Siegfried Pammer 1 week ago committed by Siegfried Pammer
parent
commit
cabe7bf458
  1. 188
      ILSpy.Tests/Bookmarks/BookmarkNavigationViewTests.cs
  2. 15
      ILSpy/Bookmarks/BookmarkNavigator.cs
  3. 47
      ILSpy/Docking/DockWorkspace.cs
  4. 19
      ILSpy/TextView/DecompilerTabPageModel.cs
  5. 37
      ILSpy/TextView/DecompilerTextView.axaml.cs

188
ILSpy.Tests/Bookmarks/BookmarkNavigationViewTests.cs

@ -0,0 +1,188 @@
// 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 Avalonia.Threading;
using Avalonia.VisualTree;
using AvaloniaEdit.Rendering;
using AwesomeAssertions;
using ICSharpCode.ILSpy.AppEnv;
using ICSharpCode.ILSpy.Bookmarks;
using ICSharpCode.ILSpy.Metadata;
using ICSharpCode.ILSpy.Metadata.CorTables;
using ICSharpCode.ILSpy.Properties;
using ICSharpCode.ILSpy.TextView;
using ICSharpCode.ILSpy.TreeNodes;
using NUnit.Framework;
namespace ICSharpCode.ILSpy.Tests.Bookmarks;
[TestFixture]
public class BookmarkNavigationViewTests
{
// Regression for the bookmark hand-off when the active content is not a decompiler tab.
// Activating a bookmark from a metadata table (or Options / About) must still select the saved
// node, scroll its decompiled document to the bookmarked line, and play the one-shot highlight --
// the pending bookmark has to reach the decompiler model that ends up displaying the node, not
// the (absent) currently-active decompiler tab.
[AvaloniaTest]
public async Task Navigating_From_NonDecompiler_Content_Scrolls_To_And_Highlights_The_Bookmark()
{
var (window, vm) = await TestHarness.BootAsync(3);
var manager = AppComposition.Current.GetExport<BookmarkManager>();
manager.Clear();
// Decompile System.Object and bookmark a line below the top so a scroll is observable.
var coreLibName = typeof(object).Assembly.GetName().Name!;
var objectNode = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(coreLibName, "System", "System.Object");
vm.AssemblyTreeModel.SelectNode(objectNode);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
var view = await window.WaitForComponent<DecompilerTextView>();
for (int i = 0; i < 8; i++)
{
Dispatcher.UIThread.RunJobs();
await Task.Delay(25);
}
int bookmarkLine = Enumerable.Range(1, view.Editor.Document.LineCount)
.Where(view.CanToggleBookmarkAtLine)
.Skip(3)
.First();
bookmarkLine.Should().BeGreaterThan(1, "the bookmark must sit below the top so the scroll is observable");
int offset = view.Editor.Document.GetLineByNumber(bookmarkLine).Offset;
// Put the caret on the bookmarked line before toggling, as Ctrl+B / a gutter click would, so
// the bookmark's captured view state agrees with its anchor.
view.Editor.TextArea.Caret.Offset = offset;
AppComposition.Current.GetExport<ContextMenuEntryRegistry>()
.GetEntry(nameof(Resources.BookmarkToggle))
.Execute(new TextViewContext { TextView = view, TextLocation = offset });
Dispatcher.UIThread.RunJobs();
manager.Bookmarks.Should().ContainSingle();
var bookmark = manager.Bookmarks[0];
// Switch the active content to a metadata table: now there is no active decompiler tab.
var typeDefNode = vm.AssemblyTreeModel.FindCoreLib()
.GetChild<MetadataTreeNode>()
.GetChild<MetadataTablesTreeNode>()
.GetChild<TypeDefTableTreeNode>();
vm.AssemblyTreeModel.SelectNode(typeDefNode);
await vm.DockWorkspace.WaitForMetadataTabAsync();
vm.DockWorkspace.ActiveDecompilerTab.Should().BeNull("the metadata table must be the active content");
// Activate the bookmark from there.
await AppComposition.Current.GetExport<BookmarkNavigator>().NavigateToAsync(bookmark);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
view = await window.WaitForComponent<DecompilerTextView>();
// Wait until the one-shot highlight has registered, then assert without any further delay: the
// adorner self-dismisses after an ~800 ms lifetime, so a fixed-length pump on a loaded CI runner
// can outlast it and observe an empty collection. The same deferred apply also lands the caret.
await Waiters.WaitForAsync(() => view.Editor.TextArea.TextView.BackgroundRenderers
.OfType<LineHighlightAdorner>().Any());
int targetLine = view.GetLineForBookmark(bookmark) ?? -1;
targetLine.Should().BeGreaterThan(1, "the bookmark resolves to a line below the top in the freshly shown document");
// P1: the caret/scroll landed on the bookmark's line rather than the default top.
view.Editor.TextArea.Caret.Line.Should().Be(targetLine,
"bookmark navigation from non-decompiler content must scroll to the saved line");
// P2: the one-shot line highlight is playing on the freshly shown view.
view.Editor.TextArea.TextView.BackgroundRenderers.OfType<LineHighlightAdorner>()
.Should().ContainSingle("the destination line must be highlighted after the content switch");
}
// Regression: when the active tab is frozen, navigating to a bookmark in a different node must
// open a fresh preview tab, decompile the node, and still scroll to + highlight the bookmark.
// This exercises the fresh-decompile hand-off (PendingBookmark consumed in the document-apply
// step), distinct from re-showing an already-decompiled node.
[AvaloniaTest]
public async Task Navigating_To_Bookmark_With_A_Frozen_Active_Tab_Opens_And_Positions_A_Fresh_Preview()
{
var (window, vm) = await TestHarness.BootAsync(3);
var manager = AppComposition.Current.GetExport<BookmarkManager>();
manager.Clear();
var coreLibName = typeof(object).Assembly.GetName().Name!;
// Bookmark a line in System.String.
var stringNode = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(coreLibName, "System", "System.String");
vm.AssemblyTreeModel.SelectNode(stringNode);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
var view = await window.WaitForComponent<DecompilerTextView>();
await PumpLayoutAsync();
int bookmarkLine = Enumerable.Range(1, view.Editor.Document.LineCount)
.Where(view.CanToggleBookmarkAtLine)
.Skip(3)
.First();
bookmarkLine.Should().BeGreaterThan(1);
int offset = view.Editor.Document.GetLineByNumber(bookmarkLine).Offset;
view.Editor.TextArea.Caret.Offset = offset;
AppComposition.Current.GetExport<ContextMenuEntryRegistry>()
.GetEntry(nameof(Resources.BookmarkToggle))
.Execute(new TextViewContext { TextView = view, TextLocation = offset });
Dispatcher.UIThread.RunJobs();
var bookmark = manager.Bookmarks.Should().ContainSingle().Subject;
// Show a different node, then freeze that tab so the next navigation must spawn a fresh preview.
var objectNode = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(coreLibName, "System", "System.Object");
vm.AssemblyTreeModel.SelectNode(objectNode);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
vm.DockWorkspace.FreezeCurrentTab();
// Activate the bookmark: a fresh preview tab must decompile System.String and land on the line.
await AppComposition.Current.GetExport<BookmarkNavigator>().NavigateToAsync(bookmark);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
var activeModel = vm.DockWorkspace.ActiveDecompilerTab;
activeModel.Should().NotBeNull("navigation must surface a decompiler tab");
// Wait until the fresh preview's view exists and its one-shot highlight has registered, then
// assert without any further delay: the adorner self-dismisses after an ~800 ms lifetime, so a
// fixed-length pump on a loaded CI runner can outlast it and observe an empty collection.
DecompilerTextView? ActiveView() => window.GetVisualDescendants().OfType<DecompilerTextView>()
.FirstOrDefault(v => ReferenceEquals(v.DataContext, activeModel));
await Waiters.WaitForAsync(() => ActiveView()?.Editor.TextArea.TextView.BackgroundRenderers
.OfType<LineHighlightAdorner>().Any() == true);
var activeView = ActiveView()!;
int targetLine = activeView.GetLineForBookmark(bookmark) ?? -1;
targetLine.Should().BeGreaterThan(1, "the fresh preview shows System.String with the bookmarked line below the top");
activeView.Editor.TextArea.Caret.Line.Should().Be(targetLine,
"opening a fresh preview for a frozen-tab navigation must still scroll to the bookmark");
activeView.Editor.TextArea.TextView.BackgroundRenderers.OfType<LineHighlightAdorner>()
.Should().ContainSingle("the destination line must be highlighted in the fresh preview");
}
static async Task PumpLayoutAsync()
{
for (int i = 0; i < 8; i++)
{
Dispatcher.UIThread.RunJobs();
await Task.Delay(25);
}
}
}

15
ILSpy/Bookmarks/BookmarkNavigator.cs

@ -76,16 +76,21 @@ namespace ICSharpCode.ILSpy.Bookmarks
if (node == null) if (node == null)
return; return;
// Hand the target line to the text view: it positions the caret once this node's
// document (and its IL-offset map) has landed.
if (AppComposition.TryGetExport<DockWorkspace>()?.ActiveDecompilerTab is { } tab)
tab.PendingBookmark = bookmark;
if (bookmark.ViewState?.RenderedLayoutSettings is { } layoutSettings if (bookmark.ViewState?.RenderedLayoutSettings is { } layoutSettings
&& AppComposition.TryGetExport<SettingsService>() is { } settingsService) && AppComposition.TryGetExport<SettingsService>() is { } settingsService)
{ {
layoutSettings.ApplyTo(settingsService.DisplaySettings); layoutSettings.ApplyTo(settingsService.DisplaySettings);
} }
assemblyTree.SelectNode(node);
// Select the node and hand the target line to the decompiler view it lands in. Routing
// through DockWorkspace (rather than setting PendingBookmark on the active decompiler tab
// here) covers the case where the currently active content is not a decompiler tab -- a
// metadata table, the Options page, the About page -- so there is no active decompiler tab
// to position directly.
if (AppComposition.TryGetExport<DockWorkspace>() is { } dockWorkspace)
dockWorkspace.NavigateToBookmark(node, bookmark);
else
assemblyTree.SelectNode(node);
} }
async Task OfferRemoveAsync(Bookmark bookmark) async Task OfferRemoveAsync(Bookmark bookmark)

47
ILSpy/Docking/DockWorkspace.cs

@ -685,6 +685,13 @@ namespace ICSharpCode.ILSpy.Docking
// Created lazily on first need. // Created lazily on first need.
DecompilerTabPageModel? decompilerContent; DecompilerTabPageModel? decompilerContent;
// A bookmark whose line the next tree-node decompile should scroll to and highlight. Set by
// NavigateToBookmark before the node is selected, and consumed in ShowSelectedNode when the
// selection routes to the decompiler content -- so a bookmark activated while a metadata
// table, the Options page, or the About page is the active content still positions correctly,
// even though there is no active decompiler tab to hand it to directly.
Bookmarks.Bookmark? pendingBookmark;
// The startup welcome page (About content in the reusable MainTab, non-static). Tracked so // The startup welcome page (About content in the reusable MainTab, non-static). Tracked so
// Help > About can activate it instead of spawning a duplicate static About tab while it is // Help > About can activate it instead of spawning a duplicate static About tab while it is
// still on screen. Self-correcting: once a tree-node selection swaps MainTab.Content to the // still on screen. Self-correcting: once a tree-node selection swaps MainTab.Content to the
@ -736,6 +743,33 @@ namespace ICSharpCode.ILSpy.Docking
} }
} }
/// <summary>
/// Selects <paramref name="node"/> and scrolls the decompiler view to <paramref name="bookmark"/>'s
/// line once that node's document and IL-offset map have landed. Unlike setting
/// <see cref="DecompilerTabPageModel.PendingBookmark"/> on <see cref="ActiveDecompilerTab"/>,
/// this works when the currently active content is not a decompiler tab (a metadata table, the
/// Options page, the About page): the bookmark is handed to the decompiler model that
/// <see cref="ShowSelectedNode"/> routes the selection to.
/// </summary>
public void NavigateToBookmark(ICSharpCode.ILSpyX.TreeView.SharpTreeNode node, Bookmarks.Bookmark bookmark)
{
pendingBookmark = bookmark;
var previousSelection = assemblyTreeModel.SelectedItem;
assemblyTreeModel.SelectNode(node);
// Selecting an already-selected node is a no-op, so ShowSelectedNode never runs to consume
// the pending bookmark. When the node is already the active decompiler content, position
// against the live tab directly; otherwise drop the pending bookmark so it cannot bleed
// into a later navigation.
if (pendingBookmark is { } pending && ReferenceEquals(previousSelection, node))
{
pendingBookmark = null;
// The document is already displayed, so no document-apply step will run to consume a
// PendingBookmark -- scroll the live view directly instead.
if (ActiveDecompilerTab is { } tab)
tab.ScrollToBookmark?.Invoke(pending);
}
}
void ShowSelectedNode() void ShowSelectedNode()
{ {
using var _ = ICSharpCode.ILSpy.AppEnv.AppLog.Phase("DockWorkspace.ShowSelectedNode"); using var _ = ICSharpCode.ILSpy.AppEnv.AppLog.Phase("DockWorkspace.ShowSelectedNode");
@ -795,6 +829,19 @@ namespace ICSharpCode.ILSpy.Docking
using (ICSharpCode.ILSpy.AppEnv.AppLog.Phase("ShowSelectedNode: main.Content = decTab (Dock view-recycling)")) using (ICSharpCode.ILSpy.AppEnv.AppLog.Phase("ShowSelectedNode: main.Content = decTab (Dock view-recycling)"))
main.Content = decTab; main.Content = decTab;
decTab.Language = languageService.CurrentLanguage; decTab.Language = languageService.CurrentLanguage;
// Carry a bookmark navigation onto the model that will display the node, before the
// decompile starts; the text view positions the caret + highlight once the document lands.
if (pendingBookmark is { } bookmark)
{
pendingBookmark = null;
// When the node is already decompiled in this tab (re-shown after a metadata/Options/
// About interlude), no document-apply step runs to consume a PendingBookmark, so scroll
// the live view directly. Otherwise the upcoming decompile's apply step consumes it.
if (decTab.CurrentNodes.SequenceEqual(nodes))
decTab.ScrollToBookmark?.Invoke(bookmark);
else
decTab.PendingBookmark = bookmark;
}
using (ICSharpCode.ILSpy.AppEnv.AppLog.Phase("ShowSelectedNode: decTab.CurrentNodes = nodes (kicks off DecompileAsync)")) using (ICSharpCode.ILSpy.AppEnv.AppLog.Phase("ShowSelectedNode: decTab.CurrentNodes = nodes (kicks off DecompileAsync)"))
decTab.CurrentNodes = nodes; decTab.CurrentNodes = nodes;
main.SourceNode = nodes.Length == 1 ? nodes[0] : null; main.SourceNode = nodes.Length == 1 ? nodes[0] : null;

19
ILSpy/TextView/DecompilerTabPageModel.cs

@ -238,12 +238,21 @@ namespace ICSharpCode.ILSpy.TextView
public DecompilerTextViewState? PendingViewState { get; set; } public DecompilerTextViewState? PendingViewState { get; set; }
/// <summary> /// <summary>
/// A bookmark to scroll to once this document is shown. Set by bookmark navigation before the /// A bookmark to scroll to the next time this document is (re)applied. Set by bookmark
/// target node is decompiled; the text view computes the line and positions the caret after /// navigation before the target node is decompiled; the text view reads it in its
/// the new document lands (and clears this), mirroring <see cref="HighlightedReference"/>. /// document-apply step, computes the line from the saved token, positions the caret and plays
/// the highlight, then clears it. Consumed there -- alongside the view-state restore -- rather
/// than reacting to a property change, so the reset-to-top of a fresh navigation cannot scroll
/// the bookmark position away in a later pass.
/// </summary> /// </summary>
[ObservableProperty] public Bookmarks.Bookmark? PendingBookmark { get; set; }
private Bookmarks.Bookmark? pendingBookmark;
/// <summary>
/// Scrolls the live document to a bookmark immediately, for navigation that lands on the
/// already-displayed node (no re-decompile, so no document-apply step runs). Set by the text
/// view; mirrors <see cref="NavigateBookmarkInFile"/>.
/// </summary>
public System.Action<Bookmarks.Bookmark>? ScrollToBookmark { get; set; }
/// <summary>Moves to the next (true) / previous (false) bookmark within this document. Set by the text view.</summary> /// <summary>Moves to the next (true) / previous (false) bookmark within this document. Set by the text view.</summary>
public System.Action<bool>? NavigateBookmarkInFile { get; set; } public System.Action<bool>? NavigateBookmarkInFile { get; set; }

37
ILSpy/TextView/DecompilerTextView.axaml.cs

@ -748,13 +748,26 @@ namespace ICSharpCode.ILSpy.TextView
void ApplyPendingBookmark(DecompilerTabPageModel model) void ApplyPendingBookmark(DecompilerTabPageModel model)
{ {
if (model.PendingBookmark is not { } bookmark) // Clear only once the line actually resolves and we scroll. A document-apply can run
return; // against the not-yet-decompiled document (a fresh preview tab binds with empty text
// before its decompile lands); leaving PendingBookmark set there lets the apply that
// follows the real content position the caret instead of discarding the navigation.
if (model.PendingBookmark is { } bookmark && ApplyBookmark(bookmark))
model.PendingBookmark = null;
}
// Computes the bookmark's current line from its saved token/anchor (resilient to reflow) and
// scrolls there with the one-shot highlight. Returns false when the line cannot be resolved
// yet (document not decompiled, or not C#). Shared by the document-apply consumption of
// PendingBookmark and the ScrollToBookmark callback for an already-displayed node.
bool ApplyBookmark(Bookmarks.Bookmark bookmark)
{
if (GetLineForBookmark(bookmark) is { } line) if (GetLineForBookmark(bookmark) is { } line)
{ {
ScrollToLine(line, bookmark.ViewState); ScrollToLine(line, bookmark.ViewState);
model.PendingBookmark = null; return true;
} }
return false;
} }
void ScrollToLine(int line, Bookmarks.BookmarkViewState? viewState = null) void ScrollToLine(int line, Bookmarks.BookmarkViewState? viewState = null)
@ -918,6 +931,7 @@ namespace ICSharpCode.ILSpy.TextView
previous.PropertyChanged -= OnModelPropertyChanged; previous.PropertyChanged -= OnModelPropertyChanged;
previous.CaptureViewState = null; previous.CaptureViewState = null;
previous.NavigateBookmarkInFile = null; previous.NavigateBookmarkInFile = null;
previous.ScrollToBookmark = null;
} }
boundModel = DataContext as DecompilerTabPageModel; boundModel = DataContext as DecompilerTabPageModel;
@ -930,6 +944,9 @@ namespace ICSharpCode.ILSpy.TextView
model.CaptureViewState = GetCurrentViewState; model.CaptureViewState = GetCurrentViewState;
// Bookmarks-pane toolbar navigation actions that operate on the active document route through this. // Bookmarks-pane toolbar navigation actions that operate on the active document route through this.
model.NavigateBookmarkInFile = NavigateBookmarkInFile; model.NavigateBookmarkInFile = NavigateBookmarkInFile;
// Direct scroll for a bookmark activated on the already-displayed node, where no
// document-apply step runs to consume PendingBookmark.
model.ScrollToBookmark = bookmark => ApplyBookmark(bookmark);
ApplyDocument(model); ApplyDocument(model);
// Point the breadcrumb at this tab's node (the bar owns its own VM, so feed it the // Point the breadcrumb at this tab's node (the bar owns its own VM, so feed it the
// node rather than letting it inherit the document DataContext). // node rather than letting it inherit the document DataContext).
@ -985,12 +1002,6 @@ namespace ICSharpCode.ILSpy.TextView
{ {
ApplyHighlightedReference(m); ApplyHighlightedReference(m);
} }
// A bookmark navigation that lands on the already-open document (no re-decompile) scrolls here.
else if (sender is DecompilerTabPageModel bm
&& e.PropertyName == nameof(DecompilerTabPageModel.PendingBookmark))
{
ApplyPendingBookmark(bm);
}
} }
void ApplyHighlightedReference(DecompilerTabPageModel model) void ApplyHighlightedReference(DecompilerTabPageModel model)
@ -1042,8 +1053,12 @@ namespace ICSharpCode.ILSpy.TextView
if (restoreViewState) if (restoreViewState)
RestoreOrResetViewState(pendingState); RestoreOrResetViewState(pendingState);
// Position at a navigated-to bookmark once its document (and debug map) has landed. // Position at a navigated-to bookmark once its document (and debug map) has landed. Only on
ApplyPendingBookmark(model); // the final content (the Text change, where restoreViewState is set), AFTER the view-state
// reset above, so the bookmark scroll is the last word on position and the highlight plays
// on the settled layout instead of an intermediate render a later rebuild would scroll away.
if (restoreViewState)
ApplyPendingBookmark(model);
SwapCustomElementGenerators(model.CustomElementGenerators); SwapCustomElementGenerators(model.CustomElementGenerators);

Loading…
Cancel
Save