v2 - Null reference in SelectedTab setter during RemoveTab call

[tznind]

Describe the bug

I am updating designer from v2 pre.250 to v2 pre.710.

It seems there has been some changes to TabView, I am now getting a null reference in RemoveTab (within the setter on SelectedTab).

System.NullReferenceException
  HResult=0x80004003
  Message=Object reference not set to an instance of an object.
  Source=Terminal.Gui
  StackTrace:>	Terminal.Gui.dll!Terminal.Gui.TabView.SelectedTab.set(Terminal.Gui.Tab value) Line 501	C#
 	Terminal.Gui.dll!Terminal.Gui.TabView.RemoveTab(Terminal.Gui.Tab tab) Line 698	C#
 	TerminalGuiDesigner.dll!TerminalGuiDesigner.TabViewExtensions.ReOrderTabs(Terminal.Gui.TabView tabView, Terminal.Gui.Tab[] newOrder) Line 58	C#
 	TerminalGuiDesigner.dll!TerminalGuiDesigner.Operations.TabOperations.MoveTabOperation..ctor.AnonymousMethod__0_1(Terminal.Gui.TabView v, Terminal.Gui.Tab[] a) Line 23	C#
 	TerminalGuiDesigner.dll!TerminalGuiDesigner.Operations.Generics.MoveOperation<Terminal.Gui.TabView, Terminal.Gui.Tab>.DoImpl() Line 108	C#
 	TerminalGuiDesigner.dll!TerminalGuiDesigner.Operations.Operation.Do() Line 48	C#
 	UnitTests.dll!UnitTests.Operations.MoveTabOperationTests.TestMoveTab_DoUndo.AnonymousMethod__0(TerminalGuiDesigner.Design d, Terminal.Gui.TabView v) Line 57	C#
 	UnitTests.dll!UnitTests.Tests.RoundTrip<Terminal.Gui.Window, Terminal.Gui.TabView>(System.Action<TerminalGuiDesigner.Design, Terminal.Gui.TabView> adjust, out Terminal.Gui.TabView viewOut, string caller) Line 105	C#
 	UnitTests.dll!UnitTests.Operations.MoveTabOperationTests.TestMoveTab_DoUndo(int idxToMove, int adjustment, int expectedNewIndex, bool expectPossible) Line 41	C#

To Reproduce
Run TestMoveTab_DoUndo in TGD

Expected behavior
No crash

[BDisp]

That isn't the current code.

Terminal.Gui/Terminal.Gui/Views/TabView.cs

Lines 145 to 186 in 43e4a83

	public Tab SelectedTab
	{
	get => _selectedTab;
	set
	{
	UnSetCurrentTabs ();
	Tab old = _selectedTab;
	if (_selectedTab is { })
	{
	if (_selectedTab.View is { })
	{
	// remove old content
	_contentView.Remove (_selectedTab.View);
	}
	}
	_selectedTab = value;
	if (value is { })
	{
	// add new content
	if (_selectedTab.View is { })
	{
	_contentView.Add (_selectedTab.View);
	}
	}
	EnsureSelectedTabIsVisible ();
	if (old != value)
	{
	if (old?.HasFocus == true)
	{
	SelectedTab.SetFocus ();
	}
	OnSelectedTabChanged (old, value);
	}
	}
	}

[tznind]

Strange, I get this in 710 and 720, maybe 2.0.0-pre.720 was released from a different branch?

[BDisp]

Strange, I get this in 710 and 720, maybe 2.0.0-pre.720 was released from a different branch?

I don't know. You have to wait from the @tig response.

[tznind]

I think my source mapping is off. I have added Terminal.Gui as a submodule and checked out v2_develop. I still get the error (so nuget package probably is latest version) but I now see the correct code:

[BDisp]

Good catch. Maybe the value is null and setting the _selectedTab to null.

[tznind]

Here is a unit test

[Fact]
public void RemoveTab_ThatHasFocus ()
{
    TabView tv = GetTabView (out Tab tab1, out Tab tab2);

    tv.SelectedTab = tab2;
    tab2.HasFocus = true;

    Assert.Equal (2, tv.Tabs.Count);
    foreach(var t in tv.Tabs.ToArray())
    {
        tv.RemoveTab(t);
    }

    Assert.Equal (0, tv.Tabs.Count);

    // Shutdown must be called to safely clean up Application if Init has been called
    Application.Shutdown ();
}

 Terminal.Gui.ViewsTests.TabViewTests.RemoveTab_ThatHasFocus
   Source: TabViewTests.cs line 30
   Duration: 388 ms

  Message: 
System.NullReferenceException : Object reference not set to an instance of an object.

  Stack Trace: 
TabView.set_SelectedTab(Tab value) line 180
TabView.RemoveTab(Tab tab) line 360
TabViewTests.RemoveTab_ThatHasFocus() line 40
RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

[BDisp]

Even if old?.HasFocus == true that doesn't mean that there is currently any tab. Do you think SelectedTab?.SetFocus (); is enough?

[tznind]

Looks like it would work :)

…

On Sun, 31 Mar 2024, 21:13 BDisp, ***@***.***> wrote: Even if old?.HasFocus == true that doesn't main that there is currently any tab. Do you think SelectedTab?.SetFocus (); is enough? — Reply to this email directly, view it on GitHub <#3365 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHO3C5GFDP4M5FI72Y6V7N3Y3BVANAVCNFSM6AAAAABFQUD75KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRYHA4TGOBUGQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>