Terminal.Gui icon indicating copy to clipboard operation
Terminal.Gui copied to clipboard

v2 - Null reference in SelectedTab setter during `RemoveTab` call

Open tznind opened this issue 1 year ago • 8 comments

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).

null-ref-tab-select

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

tznind avatar Mar 31 '24 18:03 tznind

That isn't the current code.

https://github.com/gui-cs/Terminal.Gui/blob/43e4a836645036b6aa8acc148d9e714d1bf47e09/Terminal.Gui/Views/TabView.cs#L145-L186

BDisp avatar Mar 31 '24 19:03 BDisp

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

tznind avatar Mar 31 '24 19:03 tznind

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.

BDisp avatar Mar 31 '24 19:03 BDisp

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:

image

tznind avatar Mar 31 '24 19:03 tznind

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

BDisp avatar Mar 31 '24 19:03 BDisp

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)

tznind avatar Mar 31 '24 19:03 tznind

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

BDisp avatar Mar 31 '24 20:03 BDisp

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 https://github.com/gui-cs/Terminal.Gui/issues/3365#issuecomment-2028893844, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHO3C5GFDP4M5FI72Y6V7N3Y3BVANAVCNFSM6AAAAABFQUD75KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRYHA4TGOBUGQ . You are receiving this because you authored the thread.Message ID: @.***>

tznind avatar Mar 31 '24 21:03 tznind