Null reference in v2 in FindDeepestView

[tznind]

Describe the bug
FindDeepestView can crash when a view has null Margin e.g. MenuBar (in below code start is a MenuBar

System.NullReferenceException
  HResult=0x80004003
  Message=Object reference not set to an instance of an object.
  Source=Terminal.Gui
  StackTrace:
   at Terminal.Gui.View.FindDeepestView(View start, Point& location) in Terminal.Gui\View.cs:line 2160

[tig]

The only way this can happen is if BeginInit/EndInit was not called on start.

[tig]

The only way this can happen is if BeginInit/EndInit was not called on start.

I take that back. That's old/fake news.

It should not be possible for those to be null if the View constructor was called:

    public View ()
    {
        SetupAdornments ();
        SetupKeyboard ();

        //SetupMouse ();
        SetupText ();
    }

    private void SetupAdornments ()
    {
        //// TODO: Move this to Adornment as a static factory method
        if (this is not Adornment)
        {
            Margin = new (this);
            Border = new (this);
            Padding = new (this);
        }
    }

... Unless the View was disposed...

    private void DisposeAdornments ()
    {
        Margin?.Dispose ();
        Margin = null;
        Border?.Dispose ();
        Border = null;
        Padding?.Dispose ();
        Padding = null;
    }

[BDisp]

For sure is that the case. View was already disposed.

[tznind]

Indeed the view being disposed is the bug. Bug originates in TopLevel which seems to have code that dispose the view on Remove.

Remove is not the same as getting rid of permenantly. There are many cases where you might want to remove a view e.g. to add it to a different view. It should not dispose things.

    [Fact]
    public void no_dispose_menu ()
    {
        var mb = new MenuBar ();
        var tl = new Toplevel ();

        Assert.False (mb.WasDisposed);
        tl.Add (mb);
        Assert.False (mb.WasDisposed);
        tl.Remove (mb);
        
        // Currently fails here
        Assert.False (mb.WasDisposed);
    }