String Collection Editor dialog is incorrect for PropertiesToLoad property in DirectorySearcher control

[John-Qiao]

.NET Core Version:

5.0.100-preview.4.20202.8

Have you experienced this same bug with .NET Framework?:

No

Problem description:

When edit the PropertiesToLoad property for a DirectorySearcher control in PropertyGrid, the incorrect String Collection Editor dialog be opened.

More info:

This issue occurs in .NET Core 3.1 too.

Expected behavior:

The correct String Collection Editor dialog should be opened.

Minimal repro:

1. Launch the attached project in VS and run it.
2. Click the … edit button for PropertiesToLoad property in PropertyGrid.
   WindowsFormsApp4.zip

[RussKie]

This is a peculiar line in the project file:

<TargetFramework>net5.0</TargetFramework>

Where did it come from?

It feels related to the new TFM work (e.g. dotnet/designs#109) but I'm not aware of any changes to our repo or WindowsDesktop that relate to that...

The app is working correctly for me if I change it to

<TargetFramework>netcoreapp3.1;netcoreapp5.0</TargetFramework>

[John-Qiao]

@RussKie yes, you're right. The target framework info in project file should be:

<TargetFramework>netcoreapp3.1</TargetFramework>
or
<TargetFramework>netcoreapp5.0</TargetFramework>

But this issue still can reproduce in .NET Core 3.1 and .NET Core 5.0, I re-uploaded two test apps, please check below:
Core3.1-TestApp.zip
Core5.0-TestApp.zip

[RussKie]

This looks similar to #1210, which was fixed in #1552.

The issue is originated by .NET Runtime who own System.DirectoryServices.dll.
In .NET Framework this property was decorated with the correct UITypeEditor:
https://referencesource.microsoft.com/#System.DirectoryServices/System/DirectoryServices/DirectorySearcher.cs,71128014b756ae64
In .NET this decoration was removed:
https://github.com/dotnet/runtime/blob/caa2ed96988d89a080dc63edb7c7868daca1cabc/src/libraries/System.DirectoryServices/src/System/DirectoryServices/DirectorySearcher.cs#L230-L235

I can see two distinct parts to the problem:

1. a missing mapping for System.Collections.Specialized.StringCollection.
   We have mapped the following editors for string collection types:
   

   winforms/src/System.Windows.Forms/src/System/Drawing/Design/UITypeEditor.cs

   Lines 33 to 37 in b666dc7

   	// System.Windows.Forms type Editors
   	[typeof(string[])] = "System.Windows.Forms.Design.StringArrayEditor, " + AssemblyRef.SystemDesign,
   	// System.Windows.Forms type Editors
   	[typeof(Collection<string>)] = "System.Windows.Forms.Design.StringCollectionEditor, " + AssemblyRef.SystemDesign,

   
   System.Collections.Specialized.StringCollection inherits from IList, which is mapped to
   

   winforms/src/System.Windows.Forms/src/System/Drawing/Design/UITypeEditor.cs

   Line 28 in b666dc7

   [typeof(IList)] = "System.ComponentModel.Design.CollectionEditor, " + AssemblyRef.SystemDesign,

   
   So technically the code is working as expected. To address the error we need to add the missing mapping.

2. a recurring "Constructor on type 'System.String' not found" error thrown on line:124:
   

   winforms/src/System.Windows.Forms.Design/src/System/ComponentModel/Design/CollectionEditor.cs

   Lines 105 to 125 in 46cd3ca

   	protected virtual object CreateInstance(Type itemType)
   	{
   	IDesignerHost host = GetService(typeof(IDesignerHost)) as IDesignerHost;
   	if (typeof(IComponent).IsAssignableFrom(itemType) && host != null)
   	{
   	IComponent instance = host.CreateComponent(itemType, null);
   	// Set component defaults
   	if (host.GetDesigner(instance) is IComponentInitializer init)
   	{
   	init.InitializeNewComponent(null);
   	}
   	if (instance != null)
   	{
   	return instance;
   	}
   	}
   	return TypeDescriptor.CreateInstance(host, itemType, null, null);
   	}

   
   Technically this error doesn't belong to us, but we should be able to fix it by explicit checks for typeof(string).

@merriemcgaw what are our appetites for servicing this in 3.1? The second one is likely a blocker.

[merriemcgaw]

The second issue is the top priority to fix, but the first one is good to have as well.

[Tanya-Solyanik]

@merriemcgaw , the second part of this issue is not a regression from the .NET framework. Does it really meet the servicing bar?

1, create a .NET Framework app
2. add user control
3. add a string collection property to the user control
public StringCollection Strings { get; set; }
4. build
5. add the user control to the form
6. open property browser for this user control
7. browse to the Strings property and open the editor
Result: error box

Why is it important to enable collection editor for specialized collections in .NET Core?

[weltkante]

Why is it important to enable collection editor for specialized collections in .NET Core?

As I read the discussion, because it is a regression that the dotnet runtime removed their attributes which were present on Desktop. Also there may be more places where this was removed.

WinForms has to provide the attributes because it owns the editor, but it can only do that easily by adding it for the whole type. Its technically possible to add attributes on properties through TypeDescriptors but its a lot more work.

[Tanya-Solyanik]

Adding intrinsic attribute to StringCollection type(PR #3120 ) is the right fix.

Enabling scenario like this(PR #3111):

        [Editor(typeof(CollectionEditor), typeof(UITypeEditor))]
        public StringCollection Strings { get; set; }

Has the following problems:

1. It had not worked in .NET framework, we are not enabling porting apps by this fix
2. We are not encouraging the best practices. Collection Editor provides poor experience for editing strings. Add button adds an empty string which has to be edited in the property grid, this adds multiple steps compare to the StringCollectionEditor where the user can add new items by pressing enter in the end of the previous one.

[RussKie]

2. We are not encouraging the best practices. Collection Editor provides poor experience for editing strings. Add button adds an empty string which has to be edited in the property grid, this adds multiple steps compare to the StringCollectionEditor where the user can add new items by pressing enter in the end of the previous one.

Agree here, but this is about unblocking customers who otherwise willmay be blocked.

[Tanya-Solyanik]

Why are customers blocked if we added intrinsic editor on this type? This is not a port from the .NET framework scenario because Collection editor had never worked with Specialized string collections.

[merriemcgaw]

I think the awful generic collection editor experience is better than the current error message and no experience. Is there any chance that this would break existing apps that have been ported or impact our designer work in any way? If not, I don't have a problem taking a fix.

[Tanya-Solyanik]

I think the awful generic collection editor experience is better than the current error message and no experience.

There is no current experience because it had never worked in .NET framework. Our Core out of box experience will show the correct editor. The user has to write code in order to associate the wrong editor with the StringCollection. The error box experience is the correct experience because it indicates that the developer uses incorrect editor. The fix is hiding a useful error.

Also the "first" fix should enable all existing StringCollection properties to work with the collection editor. If this is not the case, than intrinsic editors don't work and this is a problem

[John-Qiao]

Verified the issue in .NET Core 5.0.100-preview.5.20269.19, this issue has been fixed that the String Collection Editor dialog be opened now.