The code generated by the ListBox control is not consistent with Framework

[Tanya-Solyanik]

• .NET Core Version: 3.1+

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

Problem description:

The generated code for ListBox1 have extra code: this.listBox1.ItemHeight = 15;

Expected behavior: The generated code for ListBox1 should not have this code: this.listBox1.ItemHeight = 15;

Minimal repro:

1. Create a Core winforms application.
2. Add a ListBox control to form.
3. Double click the Form1.Designer.cs file in Solution Explorer.
4. Observe the generated code of the ListBox. Result: The generated code for ListBox1 should not have this code: this.listBox1.ItemHeight = 15;

Note from Dustin: ListBox.ItemHeight property. dotnet/winforms@4bca5b3/src/System.Windows.Forms/src/System/Windows/Forms/ListBox.cs#L596-L616 It has a [DefaultValue(DefaultItemHeight)] attribute and DefaultItemHeight is 13. Note from Tanya: Our default value is not accurate anymore due to the default font change. 13 is the default for Microsoft Sans Serif, 8.25pt, 15 is what comctl32 listbox calculates for Segoe UI, 9pt. To confirm - change the parent control's font.

[RussKie]

@DustinCampbell @Tanya-Solyanik I don't think there's a fix for this.

We can't change DefaultItemHeight because it is a public const, any library/app that has referenced it must be recompiled. https://github.com/dotnet/winforms/blob/4bca5b30aea871a2c1761b39f8ff8128cb155119/src/System.Windows.Forms/src/System/Windows/Forms/ListBox.cs#L52-L56

I don't think we can divorce ItemHeight from DefaultItemHeight either, as it would mess with the designer, and create a mental disconnect for a developer. https://github.com/dotnet/winforms/blob/4bca5b30aea871a2c1761b39f8ff8128cb155119/src/System.Windows.Forms/src/System/Windows/Forms/ListBox.cs#L601-L605

[weltkante]

The problem is that the current ItemHeight behavior is broken:

• if you manually assign the item height value 13 in the designer UI because you want a smaller height then it won't be serialized, instead you get the dynamically derived value of 15 at runtime or when you reopen the designer.
• if you want the default size of the font inherited at runtime you can't get it, every time you open the designer it will force 15 on you (instead of leaving it unassigned).

This will in particular disturb user controls which are developed to be flexible, expecting to inherit the font from whatever they are placed on. By letting the designer write fixed 15 you remove the capability of dynamic default values depending on the assigned font.

I would argue that it is not possible to be non-breaking here, by keeping the behavior as it currently is you are also breaking semantics of existing code whenever you open the designer.

If this were Desktop Framework the best solution would be to teach the designer to not write 15 when its meant to be the default value. This might involve changing the attribute on ItemHeight (without changing DefaultItemHeight if desired, which is still a breaking change, but the "most correct" breaking change I can imagine) and perhaps marking the DefaultItemHeight const obsolete. If you want to do it entirely without breaking changes you'd have to teach the designer that the DefaultValue attribute on ItemHeight is incorrect (possible through the TypeDescriptor infrastructure).

[Tanya-Solyanik]

@RussKie , could you please explain why it's a problem to require apps to recompile. The issue, as described applies, to apps in active development. Apps that misuse public constant and expect it to be a certain value, rather than to represent the default height, should be re-done. Apps that expect this constant to contain the default height, will not get the fix unless they recompile, but they have to re-write the code otherwise. This issue is blocking porting well-written apps from the desktop, as described in @weltkante's second point.

[RussKie]

Looks like I missed an important word in my previous response - "I don't think there's a good fix for this."

Since ItemHeight is dependent on the default font, and the default can change, I don't think hardcoding it in a public const is good idea (exposing a public const is never a good idea IMO). https://github.com/dotnet/winforms/blob/26ee7b5fea05516d1caa83b94995c54115c148a1/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs#L1785-L1801 https://github.com/dotnet/winforms/blob/26ee7b5fea05516d1caa83b94995c54115c148a1/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs#L11461-L11468

Also I am not sure how significant this is problem is though:

• in the default scenario, ItemHeight will always calculate and return the value of the first entry, and
• in owner-draw scenarios only use the itemHeight value (initialised from DefaultItemHeight, and set manually).

https://github.com/dotnet/winforms/blob/26ee7b5fea05516d1caa83b94995c54115c148a1/src/System.Windows.Forms/src/System/Windows/Forms/ListBox.cs#L605-L644

As a fix I propose the following:

1. Remove DefaultValue attribute here, and in other places, where a property value depends on the default font metrics.
2. Fix the serialization mechanism, and only serialize/reset in owner-drawn scenarios.
3. Mark DefaultItemHeight as obsolete, because it is not a constant.
4. Replace DefaultItemHeight at all call sites with a new private const set at 16, which is the default value in the native ListBox implementation (SYSFONT_CYCHAR 16)

[weltkante]

Fix the serialization mechanism, and only serialize/reset in owner-drawn scenarios.

Just to be clear, this is already supported and just requires removing the DefaultValue attribute (as stated) and instead implementing ShouldSerializeItemHeight and ResetItemHeight methods (which will be picked up by the TypeDescriptor/PropertyDescriptor infrastructure used by property grids).

[RussKie]

Just to be clear, this is already supported and just requires removing the DefaultValue attribute (as stated) and instead implementing ShouldSerializeItemHeight and ResetItemHeight methods (which will be picked up by the TypeDescriptor/PropertyDescriptor infrastructure used by property grids).

Correct.

[RussKie]

Closing due to age. We can reopen it if/when we receive customer feedback.

[Tanya-Solyanik]

Breaking change doc for the obsoletion of the DefaultItemHeight contant https://github.com/dotnet/docs/issues/33953

[MandiMan]

Verified on latest .NET 8.0 version 8.0.100-preview.3.23151.7 and the latest main build dll. Both results are the same and issue is still repro. Is it because of the 8640 PR influence? Please check below screenshot:

[Tanya-Solyanik]

@dkazennov - FYI - ShouldSerialize and Reset methods should take into account DrawMode.OwnerDraw*

[dkazennov]

@MandiMan hello. Could you please tell me how have you reproduced the issue on the main branch? I don't have extra code for some reason.

@Tanya-Solyanik ShouldSerializeItemHeight() uses ItemHeight property. This property checks for DrawMode. I've tried additional checks in ShouldSerializeItemHeight(), but it looks like it has the same logic as property itself.

[MandiMan]

@dkazennov Verified this on VS latest build:17.6.0 Preview 3.0 [33512.13.main]. Please see the following GIF for the reproduction steps.

[Eudora-Li01]

Verified on .NET 9.0.100-alpha.1.24068.28 + latest dlls from Winforms Repo of main branch, it was fixed. The generated code for ListBox1 doesn't have this code: this.listBox1.ItemHeight = 15;

[MandiMan]

Verified this issue with .NET 9 Preview 1 test pass build, it was fixed. Test result is same as above.