From b59d13a0598ca5f3967d0d7bb839a5e6eec6af0a Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Mon, 20 Jul 2026 16:02:44 -0400 Subject: [PATCH] Fix Copy as SysML copying stale clipboard text for reused diagram views The clipboard service on DiagramDocumentView was bound once per DiagramDocumentViewModel via a null-coalescing assignment (??=). When Dock recreated/reattached a tab's DiagramDocumentView control while reusing the same view model (for example creating two custom-view tabs in quick succession via the View Builder dialog), the clipboard service stayed anchored to the first, now-detached view instance. TopLevel.GetTopLevel then resolved null for that anchor, so SetTextAsync silently no-op'd instead of throwing - the correct snippet text reached the clipboard call, but never actually reached the OS clipboard, leaving whatever the last-successfully-bound tab copied. Fix: always rebind ClipboardService to the currently-attached view instance in OnDataContextChanged instead of only the first ever attached one. Also hardens AvaloniaClipboardService.SetTextAsync to marshal its work through Dispatcher.UIThread, since native OS clipboard access must happen on the UI thread and a caller further up an awaited chain could otherwise resume on a thread-pool thread. Adds a regression test that reproduces the original bug end-to-end through the real Dock-composed MainWindowView (confirmed to fail before the fix, returning null instead of the first tab's snippet). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../AvaloniaClipboardService.cs | 26 +++- .../DiagramDocumentView.axaml.cs | 8 +- test/OtsSoftwareTests/AvaloniaTests.cs | 128 ++++++++++++++++++ 3 files changed, 155 insertions(+), 7 deletions(-) diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/AvaloniaClipboardService.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/AvaloniaClipboardService.cs index fa4a788..d3024ec 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/AvaloniaClipboardService.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/AvaloniaClipboardService.cs @@ -1,6 +1,7 @@ using Avalonia; using Avalonia.Controls; using Avalonia.Input.Platform; +using Avalonia.Threading; using DemaConsulting.SysML2Workbench.LoggingSubsystem; namespace DemaConsulting.SysML2Workbench.AppShellSubsystem; @@ -37,17 +38,30 @@ public AvaloniaClipboardService(Visual anchor, RollingFileLogger? logger = null) } /// + /// + /// The actual clipboard write is always marshaled onto via + /// , even when this method + /// is itself already called from the UI thread: the native OS clipboard (Win32 OpenClipboard in + /// particular) can only be accessed from the single UI/STA thread, and a caller further up the awaited + /// chain may have resumed on a thread-pool thread (for example after a ConfigureAwait(false) + /// elsewhere in the call chain) without this method's own caller being aware of it. Marshaling + /// unconditionally here is the only way this unit can guarantee correctness regardless of the calling + /// thread. + /// public async Task SetTextAsync(string text) { ArgumentNullException.ThrowIfNull(text); - var clipboard = TopLevel.GetTopLevel(_anchor)?.Clipboard; - if (clipboard is null) + await Dispatcher.UIThread.InvokeAsync(async () => { - _logger?.Log(LogLevel.Error, "Unable to copy to the clipboard: no Avalonia TopLevel/clipboard is available."); - return; - } + var clipboard = TopLevel.GetTopLevel(_anchor)?.Clipboard; + if (clipboard is null) + { + _logger?.Log(LogLevel.Error, "Unable to copy to the clipboard: no Avalonia TopLevel/clipboard is available."); + return; + } - await clipboard.SetTextAsync(text); + await clipboard.SetTextAsync(text); + }); } } diff --git a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DiagramDocumentView.axaml.cs b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DiagramDocumentView.axaml.cs index 8246e47..8b7b690 100644 --- a/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DiagramDocumentView.axaml.cs +++ b/src/DemaConsulting.SysML2Workbench/AppShellSubsystem/DiagramDocumentView.axaml.cs @@ -55,7 +55,13 @@ private void OnDataContextChanged(object? sender, EventArgs e) if (_viewModel is not null) { _viewModel.DiagramChanged += OnDiagramChanged; - _viewModel.ClipboardService ??= new AvaloniaClipboardService(this); + + // Always rebind to this view instance rather than only the first ever attached (`??=`): Dock can + // recreate/reattach a tab's DiagramDocumentView while its DiagramDocumentViewModel persists (for + // example when two custom-preview tabs are created in quick succession), which would otherwise leave + // the clipboard service anchored to a now-detached, stale view - causing TopLevel.GetTopLevel to + // resolve null and the clipboard write to silently no-op. + _viewModel.ClipboardService = new AvaloniaClipboardService(this); LoadCurrentDiagram(); } } diff --git a/test/OtsSoftwareTests/AvaloniaTests.cs b/test/OtsSoftwareTests/AvaloniaTests.cs index 6c87b1e..35e7fd8 100644 --- a/test/OtsSoftwareTests/AvaloniaTests.cs +++ b/test/OtsSoftwareTests/AvaloniaTests.cs @@ -1,3 +1,4 @@ +using System.Reflection; using Avalonia; using Avalonia.Controls; using Avalonia.Headless.XUnit; @@ -181,6 +182,133 @@ await File.WriteAllTextAsync( window.Close(); } + /// + /// Regression test for the "Copy as SysML always returns the last-created custom view" bug: opening the + /// View Builder's real "OK" commit sequence twice ( + /// immediately followed by , exactly as + /// ViewBuilderDialogViewModel.TryCommit does) creates two distinct, real + /// tabs hosted by the real Dock-composed . + /// Selecting the first tab and copying must yield the first view's snippet, not the second (previously + /// failing because DiagramDocumentView.OnDataContextChanged bound the clipboard service only once + /// per view model via ??=, so re-selecting a tab whose view had been detached and later + /// re-attached left its clipboard write silently targeting a stale, detached anchor). + /// + [AvaloniaFact] + public async Task DiagramContextMenu_CopyAsSysml_TwoCustomViewTabs_EachCopiesItsOwnSnippet() + { + // Arrange + await File.WriteAllTextAsync( + Path.Combine(_tempRoot, "Sample.sysml"), + "package Sample {\n" + + " part def Engine;\n" + + " part def Gearbox;\n" + + "}\n"); + using var shell = CreateShell(); + var window = new MainWindowView(shell); + window.Show(); + Dispatcher.UIThread.RunJobs(); + + await shell.AddFolderSourceAsync(_tempRoot); + Dispatcher.UIThread.RunJobs(); + + // Act: commit the first custom view (General diagram over Engine), mirroring ViewBuilderDialog's real + // OK-button sequence. + var firstDefinition = new ViewDefinitionModel(); + firstDefinition.SetViewKind(ViewKind.General); + firstDefinition.AddExposeTarget("Sample::Engine"); + shell.OpenNewCustomPreviewTab(); + shell.PreviewCustomView(firstDefinition); + Dispatcher.UIThread.RunJobs(); + var firstTabId = shell.ActiveTabId!; + var expectedFirstSnippet = shell.ExportTabAsSysmlSnippet(firstTabId); + Assert.NotNull(expectedFirstSnippet); + + // Act: commit a second, distinct custom view (Interconnection diagram over Gearbox), which opens a + // second real tab/DiagramDocumentView rather than re-rendering the first in place. + var secondDefinition = new ViewDefinitionModel(); + secondDefinition.SetViewKind(ViewKind.Interconnection); + secondDefinition.AddExposeTarget("Sample::Gearbox"); + shell.OpenNewCustomPreviewTab(); + shell.PreviewCustomView(secondDefinition); + Dispatcher.UIThread.RunJobs(); + var secondTabId = shell.ActiveTabId!; + var expectedSecondSnippet = shell.ExportTabAsSysmlSnippet(secondTabId); + Assert.NotNull(expectedSecondSnippet); + Assert.NotEqual(firstTabId, secondTabId); + Assert.NotEqual(expectedFirstSnippet, expectedSecondSnippet); + + // Act: switch back to the first tab and copy - this is the step that previously returned the second + // (last-created) tab's snippet regardless of which tab was actually active. Uses the real Dock factory's + // SetActiveDockable/SetFocusedDockable, exactly as Dock's own tab-header click handler does, so this + // faithfully reproduces a user clicking back to the first tab rather than merely poking shell state. + SelectDiagramTab(window, firstTabId); + Dispatcher.UIThread.RunJobs(); + var firstCopiedSnippet = await CopyActiveTabAsSysmlAsync(window); + + // Assert: the first tab's own snippet was copied, not the second tab's. + Assert.Equal(expectedFirstSnippet, firstCopiedSnippet); + + // Act: switch to the second tab and copy too, confirming it still copies its own snippet correctly. + SelectDiagramTab(window, secondTabId); + Dispatcher.UIThread.RunJobs(); + var secondCopiedSnippet = await CopyActiveTabAsSysmlAsync(window); + + // Assert + Assert.Equal(expectedSecondSnippet, secondCopiedSnippet); + + window.Close(); + } + + /// + /// Selects the diagram document tab with the given tab id via the real Dock factory's + /// SetActiveDockable/SetFocusedDockable - exactly what Dock's own tab-header click handler + /// invokes - so the resulting notification (raised + /// through 's FocusedDockableChanged forwarding) faithfully reflects a + /// real user tab click rather than only updating shell-side bookkeeping. Reached via reflection since + /// 's Dock factory and per-tab view-model tracking are private implementation + /// details not otherwise exposed to tests. + /// + private static void SelectDiagramTab(MainWindowView window, string tabId) + { + var windowType = typeof(MainWindowView); + var dockFactoryField = windowType.GetField("_dockFactory", BindingFlags.NonPublic | BindingFlags.Instance); + var tabViewModelsField = windowType.GetField("_tabViewModelsByTabId", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(dockFactoryField); + Assert.NotNull(tabViewModelsField); + + var dockFactory = (WorkbenchDockFactory)dockFactoryField!.GetValue(window)!; + var tabViewModels = (Dictionary)tabViewModelsField!.GetValue(window)!; + var tabViewModel = tabViewModels[tabId]; + + dockFactory.SetActiveDockable(tabViewModel); + dockFactory.SetFocusedDockable(dockFactory.DiagramDock, tabViewModel); + } + + /// + /// Drives the real "Copy as SysML" context-menu MenuItem for whichever + /// is currently hosted by , and returns whatever text ends up on the headless + /// platform's real clipboard afterward. + /// + private static async Task CopyActiveTabAsSysmlAsync(MainWindowView window) + { + var diagramBorder = FindByName(window, "DiagramBorder"); + Assert.NotNull(diagramBorder); + var contextMenu = diagramBorder!.ContextMenu; + Assert.NotNull(contextMenu); + contextMenu!.Open(diagramBorder); + Dispatcher.UIThread.RunJobs(); + + var copyMenuItem = FindByName(window, "CopyAsSysmlMenuItem"); + Assert.NotNull(copyMenuItem); + Assert.True(copyMenuItem!.IsEnabled); + copyMenuItem.RaiseEvent(new RoutedEventArgs(MenuItem.ClickEvent)); + Dispatcher.UIThread.RunJobs(); + + var clipboard = TopLevel.GetTopLevel(window)?.Clipboard; + Assert.NotNull(clipboard); + return await clipboard!.TryGetTextAsync(); + } + /// /// Finds a named control anywhere in 's visual tree, regardless of which /// Dock-hosted UserControl's own compiled NameScope it belongs to (unlike a control's own