Bug in DateTimeConverter when time has been specified

[ulfemsoy]

When converting DateTime, with a specified time, to string the wrong format is used. Source: System.ComponentModel.TypeConverter/src/System/ComponentModel/DateTimeConverter.cs (line dotnet/corefx#120)

The following line (line number 120) return dt.ToString(format, CultureInfo.CurrentCulture);

should be replaced with return dt.ToString(format, culture);

[Gnbrkm41]

https://github.com/dotnet/corefx/blob/e99ec129cfd594d53f4390bf97d1d736cff6f860/src/System.ComponentModel.TypeConverter/src/System/ComponentModel/DateTimeConverter.cs#L120

(For easier reference)

[ericstj]

Seems like a reasonable request. Bug has been around since desktop though and will be a breaking change. https://referencesource.microsoft.com/#System/compmod/system/componentmodel/DateTimeConverter.cs,121

Too late in 5.0 to consider such breaks unless there is a compelling scenario that's blocked without workaround.

[ericstj]

Too late to take breaking changes in 6.0.0 at this point.

[Maximys]

@ericstj , it's really very funny, that line with problem was ported to other classes, linked with Date and Time. Simple search inside my favorite file manager (Far Manager), had display me next result:

1. DateOnlyConverter;
2. DateTimeConverter;
3. DateTimeOffsetConverter;
4. TimeOnlyConverter.

Can I try to fix it by single commit?

[ericstj]

@dotnet/area-system-componentmodel @dotnet/area-system-globalization @dotnet/dotnet-winforms can i get your take on the risk of the breaking change here vs value of fix?

[KlausLoeffelmann]

For the WinForms Designer and the stock components, I would say since InvariantCulture is handled individually (and we need to make sure that remains!), it shouldn't be an issue.

For folks who wrote their custom controls with their own custom type converters, and who did not state any culture, it's also not a problem, because they would also end up with Invariant Culture.

I'd have a little bit of stomachache for those scenarios where folks had specified a specific culture as an apps defined "interchange" format (like "en-EN" to make sure for example, apps between cultures that specific apps support like America, England, Ireland and South Africa would always be serialized in the American date format) without ever checking that the actual written format was each respective's current culture. Now, if we changed this, a serialization which would have been made with "en-ZA" (South Africa: yyyy/mm/dd) before the fix is now attempted to be deserialized in "en-EN" (mm/dd/yyyy). That might become a (niche) problem?

[tarekgh]

I see this is something that needs to be fixed even if it is a breaking change. Currently ConvertTo is not consistent with ConvertFrom. In addition, it is very weird even in ConvertTo to get the date format pattern from the input culture and format with CurrentCulture. I see it can cause some issues. I am expecting most scenarios will be just passing either null, Invriant, or CurrentCulture which I think will reduce the breaking change risk. So I am supportive to the change.