dotnet / winforms

Windows Forms is a .NET UI framework for building Windows desktop applications.
MIT License
4.42k stars 984 forks source link

An exception popped up when click "Type Here" on ContextMenuStrip control in DemoConsole application #9413

Closed MelonWang1 closed 8 months ago

MelonWang1 commented 1 year ago

.NET version

.NET 8.0.100-preview.7.23330.16

Did it work in .NET Framework?

Yes

Did it work in any of the earlier releases of .NET Core or .NET 5+?

It also repro's in .NET 7.0.

Issue description

In Winforms repo, there has a DemoConsole test app, System.NullReferenceException: 'Object reference not set to an instance of an object.' occurs when click "Type Here" on ContextMenuStrip control in DemoConsole application.

https://github.com/dotnet/winforms/assets/94418985/4ae6cb77-9592-41ae-96df-65fe24d9241b

menu-1

Call Stack: System.NullReferenceException HResult=0x80004003 Message=Object reference not set to an instance of an object. Source=System.Windows.Forms.Design StackTrace: at System.Windows.Forms.Design.ToolStripMenuItemDesigner.InitializeDropDown() in C:\test\winforms\src\System.Windows.Forms.Design\src\System\Windows\Forms\Design\ToolStripMenuItemDesigner.cs:line 1618

Steps to reproduce

Test sample: DemoConsole in Winforms repo

elachlan commented 1 year ago

MenuItem.Owner is null for some reason.

elachlan commented 1 year ago

@MelonWang1 does it repro in .net6?

MelonWang1 commented 1 year ago

@elachlan NET 6.0 fails when I build it in Winforms repo.

elachlan commented 1 year ago

EditTemplateNode in ToolStripMenuItemDesigner calls InitializeDropDown for the designer for a "dummyItem" created via CreateDummyItem. For whatever reason the dummy item has no owner, hence the exception. The owner is set when the item is added to a ToolStripItemCollection. But I don't think this is ever done for the dummy item.

If we wrap the Invalidate call in a null check:

if (MenuItem.Owner is not null)
{
    GetService<BehaviorService>()?.Invalidate(MenuItem.Owner.Bounds);
}

This ends up triggering an assert at: https://github.com/dotnet/winforms/blob/386d51b6a85689a0964d0596e8e0d03a471fc225/src/System.Windows.Forms.Design/src/System/Windows/Forms/Design/ToolStripItemDesigner.cs#L1219

Hope that helps.

elachlan commented 1 year ago

@dmitrii-drobotov any suggestions here I am a bit stuck.

dmitrii-drobotov commented 1 year ago

It also reproduced in release/6.0 branch. I will take a look.

dkazennov commented 1 year ago

@elachlan I'll take a look a little bit closer. Need some more time to investigate. Probably will come with some solution next week.

dkazennov commented 1 year ago

So this Issue only reproduces in DemoConsole. It has not been reported in Windows Forms applications and it doesn't reproduce there. image We must take a cation: any change in ToolStripMenuItemDesigner may lead for unforeseen regression.

And we can't simply assign newItem.Owner in CreateDummyItem(). This leads to exceptions in ToolStripItemDesigner.CommitEdit()

dkazennov commented 1 year ago

@dmitrii-drobotov any suggestions here I am a bit stuck.

I suppose it should have low priority since there were no reports of the similar problem in production. We can remove ContextMenuStrip from the DemoConsole or refactor it's MainForm to see if it can be added some other way (without surface.CreateControl maybe?).

But this Issue can show some very rare problem.

Syareel-Sukeri commented 8 months ago

Verified this on latest winforms repo from main branch, this issue was fixed. No exception popped up when click "Type Here" on ContextMenuStrip control in DemoConsole application.

https://github.com/dotnet/winforms/assets/156630333/8768d138-8f79-42b8-a884-6a09a35e11d8

Syareel-Sukeri commented 7 months ago

Verified this on latest release/9.0-preview3 branch from winforms repo, this issue was fixed. Test result is the same as above.