diff --git a/ILSpy.Tests/Bookmarks/BookmarkNavigationViewTests.cs b/ILSpy.Tests/Bookmarks/BookmarkNavigationViewTests.cs new file mode 100644 index 000000000..83a6d6479 --- /dev/null +++ b/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(); + 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(coreLibName, "System", "System.Object"); + vm.AssemblyTreeModel.SelectNode(objectNode); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + + var view = await window.WaitForComponent(); + 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() + .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() + .GetChild() + .GetChild(); + 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().NavigateToAsync(bookmark); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + + view = await window.WaitForComponent(); + // 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().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() + .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(); + manager.Clear(); + var coreLibName = typeof(object).Assembly.GetName().Name!; + + // Bookmark a line in System.String. + var stringNode = vm.AssemblyTreeModel.FindNode(coreLibName, "System", "System.String"); + vm.AssemblyTreeModel.SelectNode(stringNode); + await vm.DockWorkspace.WaitForDecompiledTextAsync(); + var view = await window.WaitForComponent(); + 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() + .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(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().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() + .FirstOrDefault(v => ReferenceEquals(v.DataContext, activeModel)); + await Waiters.WaitForAsync(() => ActiveView()?.Editor.TextArea.TextView.BackgroundRenderers + .OfType().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() + .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); + } + } +} diff --git a/ILSpy/Bookmarks/BookmarkNavigator.cs b/ILSpy/Bookmarks/BookmarkNavigator.cs index d12892459..3889f11c4 100644 --- a/ILSpy/Bookmarks/BookmarkNavigator.cs +++ b/ILSpy/Bookmarks/BookmarkNavigator.cs @@ -76,16 +76,21 @@ namespace ICSharpCode.ILSpy.Bookmarks if (node == null) 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()?.ActiveDecompilerTab is { } tab) - tab.PendingBookmark = bookmark; if (bookmark.ViewState?.RenderedLayoutSettings is { } layoutSettings && AppComposition.TryGetExport() is { } settingsService) { 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() is { } dockWorkspace) + dockWorkspace.NavigateToBookmark(node, bookmark); + else + assemblyTree.SelectNode(node); } async Task OfferRemoveAsync(Bookmark bookmark) diff --git a/ILSpy/Docking/DockWorkspace.cs b/ILSpy/Docking/DockWorkspace.cs index fd75c10cd..07d0a3eb1 100644 --- a/ILSpy/Docking/DockWorkspace.cs +++ b/ILSpy/Docking/DockWorkspace.cs @@ -685,6 +685,13 @@ namespace ICSharpCode.ILSpy.Docking // Created lazily on first need. 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 // 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 @@ -736,6 +743,33 @@ namespace ICSharpCode.ILSpy.Docking } } + /// + /// Selects and scrolls the decompiler view to 's + /// line once that node's document and IL-offset map have landed. Unlike setting + /// on , + /// 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 + /// routes the selection to. + /// + 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() { 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)")) main.Content = decTab; 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)")) decTab.CurrentNodes = nodes; main.SourceNode = nodes.Length == 1 ? nodes[0] : null; diff --git a/ILSpy/TextView/DecompilerTabPageModel.cs b/ILSpy/TextView/DecompilerTabPageModel.cs index ecfdc4910..397dff4b4 100644 --- a/ILSpy/TextView/DecompilerTabPageModel.cs +++ b/ILSpy/TextView/DecompilerTabPageModel.cs @@ -238,12 +238,21 @@ namespace ICSharpCode.ILSpy.TextView public DecompilerTextViewState? PendingViewState { get; set; } /// - /// A bookmark to scroll to once this document is shown. Set by bookmark navigation before the - /// target node is decompiled; the text view computes the line and positions the caret after - /// the new document lands (and clears this), mirroring . + /// A bookmark to scroll to the next time this document is (re)applied. Set by bookmark + /// navigation before the target node is decompiled; the text view reads it in its + /// 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. /// - [ObservableProperty] - private Bookmarks.Bookmark? pendingBookmark; + public Bookmarks.Bookmark? PendingBookmark { get; set; } + + /// + /// 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 . + /// + public System.Action? ScrollToBookmark { get; set; } /// Moves to the next (true) / previous (false) bookmark within this document. Set by the text view. public System.Action? NavigateBookmarkInFile { get; set; } diff --git a/ILSpy/TextView/DecompilerTextView.axaml.cs b/ILSpy/TextView/DecompilerTextView.axaml.cs index 645f4d664..45d1195c8 100644 --- a/ILSpy/TextView/DecompilerTextView.axaml.cs +++ b/ILSpy/TextView/DecompilerTextView.axaml.cs @@ -748,13 +748,26 @@ namespace ICSharpCode.ILSpy.TextView void ApplyPendingBookmark(DecompilerTabPageModel model) { - if (model.PendingBookmark is not { } bookmark) - return; + // Clear only once the line actually resolves and we scroll. A document-apply can run + // 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) { ScrollToLine(line, bookmark.ViewState); - model.PendingBookmark = null; + return true; } + return false; } void ScrollToLine(int line, Bookmarks.BookmarkViewState? viewState = null) @@ -918,6 +931,7 @@ namespace ICSharpCode.ILSpy.TextView previous.PropertyChanged -= OnModelPropertyChanged; previous.CaptureViewState = null; previous.NavigateBookmarkInFile = null; + previous.ScrollToBookmark = null; } boundModel = DataContext as DecompilerTabPageModel; @@ -930,6 +944,9 @@ namespace ICSharpCode.ILSpy.TextView model.CaptureViewState = GetCurrentViewState; // Bookmarks-pane toolbar navigation actions that operate on the active document route through this. 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); // 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). @@ -985,12 +1002,6 @@ namespace ICSharpCode.ILSpy.TextView { 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) @@ -1042,8 +1053,12 @@ namespace ICSharpCode.ILSpy.TextView if (restoreViewState) RestoreOrResetViewState(pendingState); - // Position at a navigated-to bookmark once its document (and debug map) has landed. - ApplyPendingBookmark(model); + // Position at a navigated-to bookmark once its document (and debug map) has landed. Only on + // 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);