dotnet / winforms

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

Missing instances of designers in the DemoConsoleApp #11669

Closed ricardobossan closed 2 weeks ago

ricardobossan commented 1 month ago

.NET version

9.0.100-preview.5.24307.3

Did it work in .NET Framework?

Not tested/verified

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

No response

Issue description

We have referenced the items in issue #4908 to verify their presence within the DemoConsole app. The results are detailed in this report, which mirrors the items listed in that issue.

This issue is created to track the progress of adding the missing instances to the DemoConsole app.

Missing Instances to Add from the System.Windows.Forms.Design namespace:

Steps to reproduce

Run DemoConsole app and look for missing instances of items mentioned in issue #4908.

ricardobossan commented 1 month ago

@Tanya-Solyanik @LeafShi1 @SimonZhao888 As suggested in #11707, I added breakpoints to all designers and debugged DemoConsole. All breakpoints were hit, except for BindingNavigatorDesigner, indicating that they were already instantiated.

I then instantiated BindingNavigatorDesigner, and the breakpoint was hit. However, this caused the whole tab to go blank, as mentioned here.

While debugging BindingNavigatorDesigner, I found an error that seems to be causing the tab to become blank. However, I am unsure how to resolve it. Could you help me with this?

System.InvalidOperationException
HResult=0x80131509
Message=DesignSurfaceExt::CreateComponent - Exception: (see Inner Exception)
Source=DesignSurfaceExt
StackTrace:
at DesignSurfaceExt.DesignSurfaceExt.CreateComponent[TComponent](IDesignerHost& host) in C:\Users\v-rbossan\source\repos\winforms\11669\src\System.Windows.Forms\tests\IntegrationTests\DesignSurface\DesignSurfaceExt\DesignSurfaceExt.cs:line 225
at DesignSurfaceExt.DesignSurfaceExt.CreateControl[TControl](Size controlSize, Point controlLocation) in C:\Users\v-rbossan\source\repos\winforms\11669\src\System.Windows.Forms\tests\IntegrationTests\DesignSurface\DesignSurfaceExt\DesignSurfaceExt.cs:line 164

This exception was originally thrown at this call stack:
System.ComponentModel.Container.ValidateName(System.ComponentModel.IComponent, string) in Container.cs
System.ComponentModel.Container.Add(System.ComponentModel.IComponent, string) in Container.cs
System.ComponentModel.Design.DesignerHost.PerformAdd(System.ComponentModel.IComponent, string) in DesignerHost.cs
System.ComponentModel.Design.DesignerHost.Add(System.ComponentModel.IComponent, string) in DesignerHost.cs
System.Windows.Forms.Design.BindingNavigatorDesigner.SiteItem(System.ComponentModel.Design.IDesignerHost, System.Windows.Forms.ToolStripItem) in BindingNavigatorDesigner.cs
System.Windows.Forms.Design.BindingNavigatorDesigner.SiteItems(System.ComponentModel.Design.IDesignerHost, System.Windows.Forms.ToolStripItemCollection) in BindingNavigatorDesigner.cs
System.Windows.Forms.Design.BindingNavigatorDesigner.InitializeNewComponent(System.Collections.IDictionary) in BindingNavigatorDesigner.cs
DesignSurfaceExt.DesignSurfaceExt.CreateComponent<TComponent>(out System.ComponentModel.Design.IDesignerHost) in DesignSurfaceExt.cs

Inner Exception 1:
ArgumentException: Duplicate component name 'BindingNavigatorSeparator'. Component names must be unique and case-insensitive.
SimonZhao888 commented 1 month ago

Hi @Tanya-Solyanik,

BindingNavigator is not displayed in the designer interface at design time, however, it can be displayed in the runtime interface, can we add the BindingNavigator control to the WinFormsControlsTest project. Is this a better way to test the BindingNavigatorDesigner?

SimonZhao888 commented 1 month ago

System.InvalidOperationException
HResult=0x80131509
Message=DesignSurfaceExt::CreateComponent - Exception: (see Inner Exception)
Source=DesignSurfaceExt
StackTrace:
at DesignSurfaceExt.DesignSurfaceExt.CreateComponent[TComponent](IDesignerHost& host) in C:\Users\v-rbossan\source\repos\winforms\11669\src\System.Windows.Forms\tests\IntegrationTests\DesignSurface\DesignSurfaceExt\DesignSurfaceExt.cs:line 225

Hi @ricardobossan ,

This issue is caused by the fact that all 3 separator lines have the same name. https://github.com/dotnet/winforms/blob/00b8d566610cf2383763d576c1bdad51cae4d882/src/System.Windows.Forms/src/System/Windows/Forms/DataBinding/BindingNavigator.cs#L161-L163 That's why the program reports an error at https://github.com/dotnet/winforms/blob/00b8d566610cf2383763d576c1bdad51cae4d882/src/System.Windows.Forms.Design/src/System/Windows/Forms/Design/BindingNavigatorDesigner.cs#L104

If we update the name of the divider, this problem doesn't occur. image image

ricardobossan commented 1 month ago

@SimonZhao888 Thanks! I created PR #11780 to address this and to add the BindingNavigator to the DemoConsole.

Tanya-Solyanik commented 1 month ago

Hi @Tanya-Solyanik,

BindingNavigator is not displayed in the designer interface at design time, however, it can be displayed in the runtime interface, can we add the BindingNavigator control to the WinFormsControlsTest project. Is this a better way to test the BindingNavigatorDesigner?

If you add BindingNavigator control to the WinFormsControlsTest, its designer will not be created.

Tanya-Solyanik commented 1 month ago

@SimonZhao888 , @ricardobossan - This code had always generated the same names for separators - https://referencesource.microsoft.com/#System.Windows.Forms/winforms/Managed/System/WinForms/BindingNavigator.cs,171 Why didn't we see a bug in the past? How does this work in the InProc designer? I assume we never hit an unnamed control case in that designer... Instead, the nested designerhost generated control name (nested host handles controls nested inside the parent control, i.e. separator inside the navigator) This host is probably missing in the demo console.

Also try to call AddStandardItems from the code, in a runtime application and see what name is generated in .NET Framework and in NET10. I think you have a valid fix for a runtime bug, but want to confirm that we have a runtime bug.

But for the demo console bug, we probably need the nested host.

ricardobossan commented 1 month ago

@SimonZhao888, @ricardobossan - This code has always generated the same names for separators: Reference Source. Why didn't we see a bug in the past? How does this work in the InProc designer? I assume we never encountered an unnamed control case in that designer. Instead, the nested designerHost generated the control name (handling controls nested inside the parent control, like a separator inside the navigator). This host is probably missing in the demo console.

Also, try to call AddStandardItems from the code in a runtime application and see what name is generated in .NET Framework and in .NET 10. I think you have a valid fix for a runtime bug, but I want to confirm that we have a runtime bug.

But for the demo console bug, we probably need the nested host.

@Tanya-Solyanik

"How does this work in the InProc designer?"

In InProc designer, there is indeed logic responsible for naming items in the host, which is absent in DemoConsole.

"This host is probably missing in the demo console"

True. The logic from the InProc designer responsible for naming items in the host doesn't exist in DemoConsole, nor could I find any similar logic in the DemoConsole codebase.

"Call AddStandardItems from code in a runtime application"

I created WinForms apps with BindingNavigator.AddStandardItems() on both .NET Framework and .NET 8.0/9.0. In all cases, the apps work fine and the names of the separators are the same. So, the error complaining about separators having the same name happens only in the surface app (DemoConsole).

.NET Framework:

AddStandardItems_NET481

.NET 9.0:

AddStandardItems_NET8 0

"For the demo console bug, we probably need the nested host"

Should we create an issue to add this nested host to DemoConsole?

Tanya-Solyanik commented 1 month ago

Thank you for experiments!

So, the error complaining about separators having the same name happens only in the surface app (DemoConsole).

This is still a bug even if it's not reported. Let's fix it.

Tanya-Solyanik commented 1 month ago

Please create a workitem to add the missing logic to DemoConsole - DesignerUtils.GetUniqueSiteName(host, item.Name). But it will be a low priority one.