Runtime: ColorConverter in System.ComponentModel.TypeConverters does not handle SystemColors

Created on 27 Dec 2018  路  14Comments  路  Source: dotnet/runtime

It is not possible to set the BackColor and ForeColor properties of a control to a named value from SystemColors using a PropertyGrid. This is due to the fact that the ColorConverter in .NET Core 3 does not support SystemColors.

There are two versions of ColorConverter in corefx:

The first version does not support SystemColors. Unfortunately, that is also the version that is used for properties of type System.Drawing.Color in property descriptors (link), which is why this causes the PropertyGrid to fail.

Update

Proposal:

Move SystemColors into System.Drawing.Primitives to be part of .NET Core 3.0 inbox and add these colors into ColorTable so that Color.FromName resolves them.

SystemColors lives in System.Drawing.Common and not in System.Drawing.Primitives -- in order to achieve this we need to move the SystemColors type down to System.Drawing.Primitives. The reason why we need to do that instead of just adding a reference to System.Drawing.Common, it is because System.Drawing.Common is an OOB package(it is not part of the shared framework) and System.Drawing.Primitives is part of the shared framework. So it is not allowed to have something that is part of the shared framework (INBOX) take a dependency on something that is not (OOB).

We need to discuss if we're OK with SystemColors being added into .NET Core 3.0 inbox and potentially into .NET Standard 2.1.

This is needed for Winforms. Some previous discussion when we brought these types and we intentionally kept them out from .NET Standard 2.0:
https://github.com/dotnet/standard/issues/240
https://github.com/dotnet/corefx/pull/33520

cc @Tanya-Solyanik

api-approved area-System.Drawing

Most helpful comment

Thanks for the quick work everyone!

All 14 comments

It should, yes.

Hopefully the contributor is able to complete that PR .

Yes, hopefully, since this blocks a key feature of the WinForms Designer working. :smile:

The PR is in Drawing, so aligning area - @safern do we want to use this issue to tracke the API review?

Using this issue sounds good me.

Besides the API, I'd love to know what the behavior for SystemColors (and by extension FromName) will be when run on non-Windows computers. Do the properties throw? Do they return a fixed color?

Besides the API, I'd love to know what the behavior for SystemColors (and by extension FromName) will be when run on non-Windows computers. Do the properties throw? Do they return a fixed color?

They will actually work the same as it works now using System.Drawing.Common package, which returns a Color object with the following properties:

A [byte]:255
B [byte]:180
G [byte]:180
IsEmpty [bool]:false
IsKnownColor [bool]:true
IsNamedColor [bool]:true
IsSystemColor [bool]:true
Name [string]:"ActiveBorder"
R [byte]:180

Right now FromName returns an empty color, because SystemColors are not part of the ColorTable which after adding them into, should return the same as if you do now SystemColors.ActiveBorder.

What we could do is, for non-Windows return false on IsSystemColor but that would be odd, to do SystemColors.ActiveBorder.IsSystemColor and get a false value, when you're accessing it through a SystemColors class.

Hey everyone. This is working fine when ColorConverter.ConvertTo(value, typeof(string)) is called. However, it appears that this is still a problem when callingColorConverter.ConvertTo(value, typeof(InstanceDescriptor))`, which is used by the CodeDom serialization. So, this is still blocking the WinForms Designer.

I'm testing with Microsoft.NETCore.App, 3.0.0-preview5-27626-15.

> converter.GetType()

{Name = "ColorConverter" FullName = "System.Drawing.ColorConverter"}
    Assembly: {System.ComponentModel.TypeConverter, Version=4.2.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a}

...

It converts color values fine:

> converter.ConvertTo(System.Drawing.Color.Blue, typeof(string))
"Blue"

> converter.ConvertTo(System.Drawing.Color.Blue, typeof(InstanceDescriptor))
{System.ComponentModel.Design.Serialization.InstanceDescriptor}
    Arguments: {object[0]}
    IsComplete: true
    MemberInfo: {System.Drawing.Color Blue}

But not system color values;

> converter.ConvertTo(System.Drawing.SystemColors.ActiveCaption, typeof(string))
"ActiveCaption"

> converter.ConvertTo(System.Drawing.SystemColors.ActiveCaption, typeof(InstanceDescriptor))
null

cc @danmosemsft

Maybe @anipik can help figure this out

i will take a look into it.

The problem is here https://source.dot.net/#System.ComponentModel.TypeConverter/System/Drawing/ColorConverter.cs,115

For system colors typeof(Color).GetProperty(c.Name); this value is null. Earlier we had two checks, i.e checking system.Colors first and then checking for normal colors. We combined them into one earlier.

What we can do here is just add a null check in the if block

```c#
else if (ColorTable.IsKnownNamedColor(c.Name))
{
member = typeof(SystemColors).GetProperty(c.Name) ?? typeof(Color).GetProperty(c.Name) ;
}

```

@Anipik comparing .NET Framework, it has this extra block above the existing block:
c# else if (c.IsSystemColor) { member = typeof(SystemColors).GetProperty(c.Name); }
are we just missing that block? (and test)

We combined that functionality into colorTable (Known Color and System Color)
https://source.dot.net/#System.ComponentModel.TypeConverter/System/Drawing/ColorTable.cs,14

We assumed that typeof(Color).GetProperty(c.Name) will work for systemColors as well which is not the case.

We can also revert back to just the .Net Framework code i.e having specific checks for system colors and known colors. But i am sure if the color table is checking something else as well.

Thanks for the quick work everyone!

Was this page helpful?
0 / 5 - 0 ratings