Fixes #3667. Null reference in v2 in FindDeepestView.

Terminal.Gui/Views/Toplevel.cs

@@ -69,75 +69,21 @@ public partial class Toplevel : View
     #region Subviews
 
     // TODO: Deprecate - Any view can host a menubar in v2
-    /// <summary>Gets or sets the menu for this Toplevel.</summary>
-    public MenuBar? MenuBar { get; set; }
+    /// <summary>Gets the latest <see cref="MenuBar"/> added into this Toplevel.</summary>
+    public MenuBar? MenuBar => (MenuBar?)Subviews?.LastOrDefault (s => s is MenuBar);
 
     // TODO: Deprecate - Any view can host a statusbar in v2
-    /// <summary>Gets or sets the status bar for this Toplevel.</summary>
-    public StatusBar? StatusBar { get; set; }
+    /// <summary>Gets the latest <see cref="StatusBar"/> added into this Toplevel.</summary>
+    public StatusBar? StatusBar => (StatusBar?)Subviews?.LastOrDefault (s => s is StatusBar);
 
     /// <inheritdoc/>
     public override View Add (View view)
     {
         CanFocus = true;
-        AddMenuStatusBar (view);
 
         return base.Add (view);
     }
 
-    /// <inheritdoc/>
-    public override View Remove (View view)
-    {
-        if (this is Toplevel { MenuBar: { } })
-        {
-            RemoveMenuStatusBar (view);
-        }
-
-        return base.Remove (view);
-    }
-
-    /// <inheritdoc/>
-    public override void RemoveAll ()
-    {
-        if (this == Application.Top)
-        {
-            MenuBar?.Dispose ();
-            MenuBar = null;
-            StatusBar?.Dispose ();
-            StatusBar = null;
-        }
-
-        base.RemoveAll ();
-    }
-
-    internal void AddMenuStatusBar (View view)
-    {
-        if (view is MenuBar)
-        {
-            MenuBar = view as MenuBar;
-        }
-
-        if (view is StatusBar)
-        {
-            StatusBar = view as StatusBar;
-        }
-    }
-
-    internal void RemoveMenuStatusBar (View view)
-    {
-        if (view is MenuBar)
-        {
-            MenuBar?.Dispose ();
-            MenuBar = null;
-        }
-
-        if (view is StatusBar)
-        {
-            StatusBar?.Dispose ();
-            StatusBar = null;
-        }
-    }
-
     // TODO: Overlapped - Rename to AllSubviewsClosed - Move to View?
     /// <summary>
     ///     Invoked when the last child of the Toplevel <see cref="RunState"/> is closed from by

UICatalog/UICatalog.cs

@@ -408,7 +408,7 @@ public class UICatalogApp
             _themeMenuItems = CreateThemeMenuItems ();
             _themeMenuBarItem = new ("_Themes", _themeMenuItems);
 
-            MenuBar = new ()
+            MenuBar menuBar = new ()
             {
                 Menus =
                 [
@@ -462,13 +462,15 @@ public class UICatalogApp
                         )
                 ]
             };
+            Add (menuBar);
 
-            StatusBar = new ()
+            StatusBar statusBar = new ()
             {
                 Visible = ShowStatusBar,
                 AlignmentModes = AlignmentModes.IgnoreFirstOrLast,
                 CanFocus = false
             };
+            Add (statusBar);
 
             if (StatusBar is { })
             {
@@ -484,7 +486,11 @@ public class UICatalogApp
                     Title = "Show/Hide Status Bar",
                     CanFocus = false,
                 };
-                statusBarShortcut.Accept += (sender, args) => { StatusBar.Visible = !StatusBar.Visible; };
+                statusBarShortcut.Accept += (sender, args) =>
+                                            {
+                                                StatusBar.Visible = !StatusBar.Visible;
+                                                args.Handled = true;
+                                            };
 
                 ShForce16Colors = new ()
                 {

UnitTests/Views/ToplevelTests.cs

@@ -246,14 +246,26 @@ public partial class ToplevelTests (ITestOutputHelper output)
         top.OnUnloaded ();
         Assert.Equal ("Unloaded", eventInvoked);
 
-        top.AddMenuStatusBar (new MenuBar ());
+        top.Add (new MenuBar ());
         Assert.NotNull (top.MenuBar);
-        top.AddMenuStatusBar (new StatusBar ());
+        top.Add (new StatusBar ());
         Assert.NotNull (top.StatusBar);
-        top.RemoveMenuStatusBar (top.MenuBar);
+        var menuBar = top.MenuBar;
+        top.Remove (top.MenuBar);
         Assert.Null (top.MenuBar);
-        top.RemoveMenuStatusBar (top.StatusBar);
+        Assert.NotNull (menuBar);
+        var statusBar = top.StatusBar;
+        top.Remove (top.StatusBar);
         Assert.Null (top.StatusBar);
+        Assert.NotNull (statusBar);
+#if true
+        Assert.False (menuBar.WasDisposed);
+        Assert.False (statusBar.WasDisposed);
+        menuBar.Dispose ();
+        statusBar.Dispose ();
+        Assert.True (menuBar.WasDisposed);
+        Assert.True (statusBar.WasDisposed);
+#endif
 
         Application.Begin (top);
         Assert.Equal (top, Application.Top);
@@ -265,7 +277,7 @@ public partial class ToplevelTests (ITestOutputHelper output)
         Assert.Equal (0, ny);
         Assert.Null (sb);
 
-        top.AddMenuStatusBar (new MenuBar ());
+        top.Add (new MenuBar ());
         Assert.NotNull (top.MenuBar);
 
         // Application.Top with a menu and without status bar.
@@ -274,7 +286,7 @@ public partial class ToplevelTests (ITestOutputHelper output)
         Assert.Equal (1, ny);
         Assert.Null (sb);
 
-        top.AddMenuStatusBar (new StatusBar ());
+        top.Add (new StatusBar ());
         Assert.NotNull (top.StatusBar);
 
         // Application.Top with a menu and status bar.
@@ -286,8 +298,10 @@ public partial class ToplevelTests (ITestOutputHelper output)
         Assert.Equal (2, ny);
         Assert.NotNull (sb);
 
-        top.RemoveMenuStatusBar (top.MenuBar);
+        menuBar = top.MenuBar;
+        top.Remove (top.MenuBar);
         Assert.Null (top.MenuBar);
+        Assert.NotNull (menuBar);
 
         // Application.Top without a menu and with a status bar.
         View.GetLocationEnsuringFullVisibility (top, 2, 2, out nx, out ny, out sb);
@@ -298,8 +312,10 @@ public partial class ToplevelTests (ITestOutputHelper output)
         Assert.Equal (2, ny);
         Assert.NotNull (sb);
 
-        top.RemoveMenuStatusBar (top.StatusBar);
+        statusBar = top.StatusBar;
+        top.Remove (top.StatusBar);
         Assert.Null (top.StatusBar);
+        Assert.NotNull (statusBar);
         Assert.Null (top.MenuBar);
 
         var win = new Window { Width = Dim.Fill (), Height = Dim.Fill () };
@@ -318,7 +334,7 @@ public partial class ToplevelTests (ITestOutputHelper output)
         Assert.Equal (0, ny);
         Assert.Null (sb);
 
-        top.AddMenuStatusBar (new MenuBar ());
+        top.Add (new MenuBar ());
         Assert.NotNull (top.MenuBar);
 
         // Application.Top with a menu and without status bar.
@@ -327,7 +343,7 @@ public partial class ToplevelTests (ITestOutputHelper output)
         Assert.Equal (1, ny);
         Assert.Null (sb);
 
-        top.AddMenuStatusBar (new StatusBar ());
+        top.Add (new StatusBar ());
         Assert.NotNull (top.StatusBar);
 
         // Application.Top with a menu and status bar.
@@ -339,10 +355,14 @@ public partial class ToplevelTests (ITestOutputHelper output)
         Assert.Equal (20, ny);
         Assert.NotNull (sb);
 
-        top.RemoveMenuStatusBar (top.MenuBar);
-        top.RemoveMenuStatusBar (top.StatusBar);
-        Assert.Null (top.StatusBar);
+        menuBar = top.MenuBar;
+        statusBar = top.StatusBar;
+        top.Remove (top.MenuBar);
         Assert.Null (top.MenuBar);
+        Assert.NotNull (menuBar);
+        top.Remove (top.StatusBar);
+        Assert.Null (top.StatusBar);
+        Assert.NotNull (statusBar);
 
         top.Remove (win);
 
@@ -355,7 +375,7 @@ public partial class ToplevelTests (ITestOutputHelper output)
         Assert.Equal (0, ny);
         Assert.Null (sb);
 
-        top.AddMenuStatusBar (new MenuBar ());
+        top.Add (new MenuBar ());
         Assert.NotNull (top.MenuBar);
 
         // Application.Top with a menu and without status bar.
@@ -364,7 +384,7 @@ public partial class ToplevelTests (ITestOutputHelper output)
         Assert.Equal (2, ny);
         Assert.Null (sb);
 
-        top.AddMenuStatusBar (new StatusBar ());
+        top.Add (new StatusBar ());
         Assert.NotNull (top.StatusBar);
 
         // Application.Top with a menu and status bar.
@@ -387,7 +407,21 @@ public partial class ToplevelTests (ITestOutputHelper output)
         win.NewMouseEvent (new () { Position = new (6, 0), Flags = MouseFlags.Button1Pressed });
 
         //Assert.Null (Toplevel._dragPosition);
+#if true
+        Assert.False (top.MenuBar.WasDisposed);
+        Assert.False (top.StatusBar.WasDisposed);
+#endif
+        menuBar = top.MenuBar;
+        statusBar = top.StatusBar;
         top.Dispose ();
+        Assert.Null (top.MenuBar);
+        Assert.Null (top.StatusBar);
+        Assert.NotNull (menuBar);
+        Assert.NotNull (statusBar);
+#if true
+        Assert.True (menuBar.WasDisposed);
+        Assert.True (statusBar.WasDisposed);
+#endif
     }
 
     [Fact]
@@ -1568,4 +1602,31 @@ public partial class ToplevelTests (ITestOutputHelper output)
         t.Dispose ();
         Application.Shutdown ();
     }
+
+    [Fact]
+    public void Remove_Do_Not_Dispose_MenuBar_Or_StatusBar ()
+    {
+        var mb = new MenuBar ();
+        var sb = new StatusBar ();
+        var tl = new Toplevel ();
+
+#if DEBUG
+        Assert.False (mb.WasDisposed);
+        Assert.False (sb.WasDisposed);
+#endif
+        tl.Add (mb, sb);
+        Assert.NotNull (tl.MenuBar);
+        Assert.NotNull (tl.StatusBar);
+#if DEBUG
+        Assert.False (mb.WasDisposed);
+        Assert.False (sb.WasDisposed);
+#endif
+        tl.RemoveAll ();
+        Assert.Null (tl.MenuBar);
+        Assert.Null (tl.StatusBar);
+#if DEBUG
+        Assert.False (mb.WasDisposed);
+        Assert.False (sb.WasDisposed);
+#endif
+    }
 }