DYN-9903: Avoid using closed Window as EditWindow owner (#17014)

Author: zeusongit

src/DynamoCoreWpf/UI/Prompts/EditWindow.xaml.cs

@@ -33,11 +33,23 @@ public EditWindow(DynamoViewModel dynamoViewModel,
             InitializeComponent();
             this.dynamoViewModel = dynamoViewModel;
 
-            Owner = dynamoViewModel.Owner;
-            this.WindowStartupLocation = WindowStartupLocation.CenterOwner;
+            var ownerWindow = dynamoViewModel.Owner;
+            if (ownerWindow != null && ownerWindow.IsVisible)
+            {
+                Owner = ownerWindow;
+                this.WindowStartupLocation = WindowStartupLocation.CenterOwner;
+            }
+            else
+            {
+                var mainWindow = Application.Current?.MainWindow;
+                if (mainWindow != null && mainWindow.IsVisible)
+                {
+                    Owner = mainWindow;
+                    this.WindowStartupLocation = WindowStartupLocation.CenterOwner;
+                }
+            }
 
-            // do not accept value if user closes 
-            this.Closing += (sender, args) => this.DialogResult = false;
+            this.Closing += EditWindow_Closing;
             if (false != updateSourceOnTextChange)
             {
                 this.editText.TextChanged += delegate
@@ -200,9 +212,19 @@ private void UIElement_OnMouseDown(object sender, MouseButtonEventArgs e)
             DragMove();
         }
 
+        // Do not accept value if user closes without confirming.
+        // DialogResult is only settable when opened via ShowDialog(); guard with try/catch
+        // since WPF provides no public API to distinguish dialog vs non-dialog mode.
+        private void EditWindow_Closing(object sender, System.ComponentModel.CancelEventArgs e)
+        {
+            try { this.DialogResult = false; }
+            catch (InvalidOperationException) { }
+        }
+
         private void EditWindow_Closed(object sender, EventArgs e)
         {
             this.editText.PreviewKeyDown -= EditText_PreviewKeyDown;
+            this.Closing -= EditWindow_Closing;
             this.Closed -= EditWindow_Closed;
         }
 

src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs

@@ -2582,6 +2582,9 @@ private void HandlePackageManagerWindowClosed(object sender, EventArgs e)
         {
             packageManagerWindow.Closed -= HandlePackageManagerWindowClosed;
             packageManagerWindow = null;
+            // Reset owner back to the main Dynamo window so that dialogs opened after
+            // the Package Manager closes do not attempt to use the now-closed window as owner.
+            dynamoViewModel.Owner = this;
 
             var cmd = Analytics.TrackCommandEvent("PackageManager");
             cmd.Dispose();

src/DynamoCoreWpf/Views/Menu/PreferencesView.xaml.cs

@@ -76,6 +76,7 @@ public PreferencesView(DynamoView dynamoView)
 
             Owner = dynamoView;
             dynViewModel.Owner = this;
+            this.Closed += PreferencesView_Closed;
             if (DataContext is PreferencesViewModel viewModelTemp)
             {
                 this.viewModel = viewModelTemp;
@@ -175,6 +176,15 @@ private void CloseButton_Click(object sender, RoutedEventArgs e)
             Close();
         }
 
+        private void PreferencesView_Closed(object sender, EventArgs e)
+        {
+            this.Closed -= PreferencesView_Closed;
+            // Reset owner back to the main Dynamo window so that dialogs opened after
+            // Preferences closes do not attempt to use the now-closed window as owner.
+            // This runs regardless of how the window was closed (button, Alt+F4, programmatic).
+            dynViewModel.Owner = this.Owner;
+        }
+
         /// <summary>
         /// handler for preferences dialog dragging action. When the TitleBar is clicked this method will be executed.
         /// </summary>

test/DynamoCoreWpf3Tests/NodeViewTests.cs

@@ -4,13 +4,15 @@
 using System.IO;
 using System.Linq;
 using System.Threading;
+using System.Windows;
 using System.Windows.Input;
 using System.Windows.Media;
 using Dynamo.Controls;
 using Dynamo.Graph;
 using Dynamo.Graph.Workspaces;
 using Dynamo.Models;
 using Dynamo.Selection;
+using Dynamo.UI.Prompts;
 using Dynamo.ViewModels;
 using DynamoCoreWpfTests.Utility;
 using NUnit.Framework;
@@ -381,6 +383,95 @@ private void NodeNameTest_PropChangedHandler(object sender, System.ComponentMode
              var temp = (sender as NodeViewModel).Name;
         }
 
+        /// <summary>
+        /// Regression test: opening the Package Manager or Preferences window temporarily sets
+        /// dynamoViewModel.Owner to that child window. If the child window is closed without
+        /// restoring the owner, the EditWindow constructor throws InvalidOperationException
+        /// ("Cannot set owner to a Window that has been closed") when a node rename is triggered.
+        /// </summary>
+        [Test]
+        [Category("RegressionTests")]
+        public void WhenDynamoViewModelOwnerIsClosedWindowThenEditWindowDoesNotThrow()
+        {
+            // Arrange: open a graph with at least one node.
+            Open(@"UI\CoreUINodes.dyn");
+            DispatcherUtil.DoEvents();
+
+            Window originalOwner = null;
+            try
+            {
+                // Simulate the stale-owner state left by PackageManagerView / PreferencesView:
+                // a secondary window sets itself as dynamoViewModel.Owner, then closes
+                // without restoring the owner back to the main DynamoView.
+                View.Dispatcher.Invoke(() =>
+                {
+                    originalOwner = ViewModel.Owner;
+                    var childWindow = new Window { Owner = View };
+                    childWindow.Show();
+                    childWindow.Close();
+                    // Owner now points to a closed window — the bug condition.
+                    ViewModel.Owner = childWindow;
+                });
+
+                // Act & Assert: constructing EditWindow (what node rename does) must not throw.
+                // Before the fix, this raised:
+                //   System.InvalidOperationException: Cannot set owner to a Window that has been closed.
+                Assert.DoesNotThrow(() =>
+                {
+                    View.Dispatcher.Invoke(() =>
+                    {
+                        var editWindow = new EditWindow(ViewModel, false, true);
+                        editWindow.Close();
+                    });
+                });
+            }
+            finally
+            {
+                // Restore owner to avoid leaking stale state into subsequent tests.
+                View.Dispatcher.Invoke(() => ViewModel.Owner = originalOwner ?? View);
+            }
+        }
+
+        /// <summary>
+        /// Verifies that dynamoViewModel.Owner is restored to the main DynamoView after a
+        /// child window that took over ownership is closed via its Closed event — the same
+        /// pattern used by HandlePackageManagerWindowClosed and PreferencesView_Closed.
+        /// </summary>
+        [Test]
+        [Category("RegressionTests")]
+        public void WhenChildWindowClosesOwnerIsRestoredToDynamoView()
+        {
+            View.Dispatcher.Invoke(() =>
+            {
+                Assert.AreEqual(View, ViewModel.Owner,
+                    "Owner should be the main DynamoView after startup.");
+
+                // Simulate PackageManagerView / PreferencesView taking ownership.
+                var childWindow = new Window { Owner = View };
+                childWindow.Show();
+                ViewModel.Owner = childWindow;
+
+                Assert.AreEqual(childWindow, ViewModel.Owner,
+                    "Owner should be the child window while it is open.");
+
+                // Wire up the Closed handler the same way HandlePackageManagerWindowClosed
+                // and PreferencesView_Closed do — restore the owner to the parent window.
+                childWindow.Closed += (s, e) => ViewModel.Owner = ((Window)s).Owner ?? View;
+
+                // Act: close the child window, which fires the Closed handler.
+                childWindow.Close();
+            });
+
+            DispatcherUtil.DoEvents();
+
+            // Assert: the Closed handler restored the owner — not manual assignment.
+            View.Dispatcher.Invoke(() =>
+            {
+                Assert.AreEqual(View, ViewModel.Owner,
+                    "Owner must be restored to the main DynamoView after the child window closes.");
+            });
+        }
+
         /// <summary>
         /// Check if elements are correctly displayed/collapsed on zoom level change 
         /// Current zoom level is 0.4 (hard-coded in multiple Converters