Runtime crash due to localization and the way designer serializes keyboard shortcuts

[filipnavara]

• .NET Core Version: 3.1.2

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

Problem description:

Steps to reproduce:

• Create a blank .NET Framework WinForms application with a Form.
• Set the Form to Localizable = true in the designer.
• Add context menu with single item.
• Set the ShortcutKeys on the item to Control+C.
• Observe that the application works just fine on .NET Framework 4.8.
• Switch locale at startup to German by System.Threading.Thread.CurrentThread.CurrentUICulture = new System.Globalization.CultureInfo("de-DE");
• Observe that the application still starts just fine on .NET Framework 4.8.
• Change the application to target netcoreapp31 and it immediately crashes on startup.

The reason for the crash is that the .resx file contains the shortcut key serialized as

  <data name="booToolStripMenuItem.ShortcutKeys" type="System.Windows.Forms.Keys, System.Windows.Forms">
    <value>Ctrl+C</value>
  </data>

Notice that the serialized value starts with Ctrl, not Control. It is produced and consumed by KeysConverter class that is locale dependent. While the value Control would work in any locale due to the fallback to names in the Keys enum the value Ctrl is taken from a dictionary that contains localized names. On .NET Core 3.1 in the German localization the name for Control key is Strg. Thus the value Ctrl+C in the resource file is invalid and causes conversion error at runtime.

Notably we migrated a large project from .NET Framework to .NET Core where we hit this issue. The .NET Framework also has similar issue in code but the localizations may differ which would explain why the problem is not hit.

Manually fixing the value in the .resx file to Control+C makes the application work in any locale but any further editing in the designer breaks it again.

Expected behavior:

No crash. The resources generated by the designer should be locale independent.

Minimal repro:

Here's the sample app created using the steps above. It is multi-targeting both .NET Framework and .NET Core so it can run in both variation from inside Visual Studio.

WindowsFormsApp2.zip

[janbaltus]

From comments it seems CTRL being localized as STRG is causing crash. Both Ctrl are localized. If string is not ment to be localized it need to be locked by instruction.

Microsoft.DotNet.Wpf\src\UIAutomation\UIAutomationClientSideProviders\Resources\xlf\Strings.xlf 0; "KeyCtrl" System.Windows.Forms\src\Resources\xlf\SR.xlf 0; "toStringControl"

Can you please confirm which one of above is the functional key?

[filipnavara]

toStringControl is the problematic one.

[janbaltus]

When we compare resource from .Net Framework same string ID was used and localized in the past

LocComment=(SLE)It's used by the KeyConverter to localize display of menu shortcuts, it can be translated into whatever is printed on the keyboard for this key (eg. ALT) in the corresponding language.

So from our point of view this might be localizability issue in .NetCore. @RussKie can you please check?

[janbaltus]

please find xls with all tostring occurrences: .net.xlsx

[weltkante]

Desktop Framework also stores ShortcutKeys localized in the resx file yet doesn't run into this issue. I don't think this is a localization problem as having localized ShortcutKey resx strings works in Desktop Framework, its more likely that this is a tooling issue.

  <data name="booToolStripMenuItem.ShortcutKeys" type="System.Windows.Forms.Keys, System.Windows.Forms">
    <value>Ctrl+C</value>
  </data>

Note the type="System.Windows.Forms.Keys, System.Windows.Forms" which I think means the resx-compiler is supposed to translate this into the enum, not the runtime on the end user machine. With an enum stored in the binary resx resource instead of a string the ShortcutKeys property should be stored language-independent.

I suspect the .NET Core resx compiler ignores this instruction and stores the type as plain string, causing it to be translated at runtime, which fails depending on the users UI culture.

Comparing the binary resx data between a Desktop Framework and .NET Core build may be able to confirm if this assumption is correct.

[weltkante]

Yeah can confirm the tooling is badly designed here, .NET Core uses a new resx format which prefers string serialization over binary serialization.

On the reader side they do ConvertFromInvariantString which shows up in the stack trace as source of the error.

But I guess on the writer side they don't roundtrip through TypeConverter to emit an invariant string, they probably just emit the resx string directly into the resource. Since the whole point of resx files is localization nobody should expect resx resources to already be in invariant culture, so this is probably a bug in corefx tooling. I don't know where the source of the tools is but IMHO they should fix it and do something like

// get the type converter for the type specified in the resx source
var tc = TypeDescriptor.GetConverter(typeof(Keys));

// roundtrip through TypeConverter once to ensure data is invariant culture
serialized_value = tc.ConvertToInvariantString(tc.ConvertFrom(resx_value));

[filipnavara]

The whole point of the CoreFX tooling was to avoid deserialization and reserialization of the resources and loading all the assemblies with the necessary converters (https://github.com/dotnet/corefx/pull/36906). /cc @ericstj

[weltkante]

Never mind, WinForms KeysConverter is also totally broken, in that it doesn't implement ConvertFromInvariantString correctly. It just ignores the culture being asked for and initializes itself with the CurrentCulture.

Sorry for the multiple posts, seems like I was digging down a rabbit hole :-) It may very well be that corefx resx writer already performs the culture-invariant transform described above and the error is on WinForms side because KeysConverter doesn't support ConvertFromString correctly when given a specific culture.

You should still double check that the resx tooling is outputting the resx data in invariant culture, otherwise fixing this in WinForms just for the Keys property is possible, but it will still fail for 3rd party designer properties.

[weltkante]

@filipnavara @ericstj you can't avoid running the TypeConverter on build time if you want a culture-invariant serialization format. Serializer and deserializer must be symmetric in what culture they emit and expect. (Well, you could record the expected culture and operate on it directly when deserializing instead of having a culture-invariant format ...)

[filipnavara]

Yeah, there is problem at multiple levels here and it's not obvious which level[s] need to be fixed.

[filipnavara]

But I guess on the writer side they don't roundtrip through TypeConverter to emit an invariant string, they probably just emit the resx string directly into the resource. Since the whole point of resx files is localization nobody should expect resx resources to already be in invariant culture, so this is probably a bug in corefx tooling.

How would the resource compiler know which culture to convert it from? The .resx file doesn't contain any hint about what culture it was created on. I would totally expect it to be already in the invariant form there. In this case English - my system culture - has the same strings as the invariant culture so it's hard to verify.

Never mind, WinForms KeysConverter is also totally broken, in that it doesn't implement ConvertFromInvariantString correctly.

That looks like an obvious bug to me. For this particular case fixing that would be enough to fix the crash. It should still be verified that on German system the designer still produces invariant values in the .resx file.

[weltkante]

I would totally expect it to be already in the invariant form there. [...] It should still be verified that on German system the designer still produces invariant values in the .resx file.

Your expectations are wrong, resx files are localized (I'm on a german system and get 'Strg' instead of 'Ctrl' in my resx). Note that this behavior is the whole point of this issue, if resx files were invariant to begin with nobody would ever have run into this problem. [edit] Note that it may be possible that the VS designer outputs invariant resources into resx files and we just see KeysConverter being broken again by not listening to the culture its callers ask for. I'd have to run larger tests with custom controls using 3rd party TypeConverters to definitely figure out what happens during design time and build time.

Assuming this isn't just isolated to KeysConverter and resx files are actually culture specific, then you can't just redefine resx files to be invariant unless you both

• update the VS designer to generate resx files invariant for .NET Core (but not for Desktop Framework due to compatibility with existing Desktop projects you need to continue to edit)
• require everyone migrating from Desktop Framework to un-localize every single resx entry

The VS designer is the reference here (and of course you also want to stay compatible with already existing resx files created in .NET Core projects), whatever culture VS uses to write into WinForms resx files is what needs to be used by the resx tooling as well. If VS designer and build tools don't agree then things never will work out.

If VS uses the current culture (which I'd guess because changing the assembly culture itself didn't have any effect on the Desktop Framework designer) then resx files would be incompatible between build machines using different languages, which is very unfortunate, but IMHO redefining resx semantics is not an option. If anything you can extend them by adding a language attribute/tag somewhere and if its not present assume legacy semantics.

[filipnavara]

If VS uses the current culture (which I'd guess because changing the assembly culture itself didn't have any effect on the designer) then resx files would be incompatible between build machines using different languages, which is very unfortunate, but IMHO redefining resx semantics is not an option.

This is exactly the reason why I have real trouble believing that .resx can contain non-invariant strings. Note that the KeysConverter implementation in NetFX is broken in exactly the same way as the NetCore one (https://referencesource.microsoft.com/#system.windows.forms/winforms/managed/system/winforms/KeysConverter.cs), so even if the designer asked for conversion to invariant culture it would still get a wrong answer.

[weltkante]

Yes I realized the mistake in my reasoning and edited my last post. This may all be due to the broken KeysConverter. I'll run some tests with custom TypeConverters to figure out the exact behavior of the VS designer + resx build tools.

[weltkante]

Desktop Framework:

• VS designer interacts with the Property Grid in the Current Culture (OS language) but CurrentUICulture is the VS installation language - in other words the developer usually sees and edits the localized version in the designer (but for KeysConverter he will see keys localized according to his VS language because SR string resources in its implementation use CurrentUICulture not CurrentCulture)
• VS designer stores in resx with invariant culture. This breaks for KeysConverter because it keeps operating on CurrentUICulture which still is the VS installation language.
• msbuild deserializes resx entries with invariant culture. KeysConverter still uses CurrentUICulture which cancels out the bug when building from within VS, but it will fail the build when run outside VS and the OS and VS language does not match (one of my machines has english VS on german OS and could not build the test project outside VS on this machine)
• runtime deserializes with BinaryFormatter (culture is irrelevant) so the bugs above never mattered for end users as long as your build environment had a consistent language (and if it didn't you'd get build errors instead of runtime errors)

.NET Core

• VS designer doesn't support custom controls yet so I couldn't check how CurrentCulture and CurrentUICulture are used
• msbuild does not use TypeConverter, it just takes the string from resx
• runtime deserializes with invariant culture

Asumming VS designer for .NET Core will be implemented analogous to Desktop Framework it will interact with the developer in the Current Culture and serialize to resx with Invariant Culture - then everything will play out fine for a correctly implemented 3rd party TypeConverter.

That leaves the broken WinForms KeysConverter to fix. The new resx writer implementation moves a build-time localization issue (which either canceled out itself or resulted in build failures) into runtime errors, you'll have to fix KeysConverter and support serialization/deserialization with the requested culture, otherwise the new resx format will never work correctly with KeysConverter.

You also might have to consider providing a migration path, because after fixing KeysConverter the VS designer will no longer be able to read resx forms made on non-english VS installations (both made in .NET Core before the fix and those migrated from Desktop Framework will fail to open in the designer). Considering manual fixup of the resx files is possible you may get away without providing an automated resolution in the .NET Core VS designer.

[filipnavara]

Thanks for the detailed analysis!

We have already worked around the problem by manually updating the resources in our application to use locale-independent versions of key resources. However, it gets broken with any edit in designer so the solution is quite error prone. We will add a unit test to verify the problem at compile time but obviously we expect the issue to be tackled upstream at some point.

[RalphLAtGitHub]

@merriemcgaw: Is there still hope that the bug will be fixed? What does your timeline look like?

[filipnavara]

I'd certainly hope that this gets fixed at some point. We have to force our developers to use specific locale on machines running Visual Studio that use the WinForms designer. Additionally we have a unit test that checks all the .resx files to make sure all the shortcuts could still be loaded in different locales.

[merriemcgaw]

@filipnavara agreed, this certainly needs to be fixed. We'll bubble this up to take a look now since we're much further a long in the localized forms story. @Olina-Zhang can you test this scenario for localizing keyboard shortcuts on the form? If there's still and issue, let us know here but also create a bug for the designer repo as well. Thanks so much!

[Jenna-Zheng]

@merriemcgaw this issue is still reproducing and I have create a new bug #3305 for the designer repo as well.

[RalphLAtGitHub]

To whom it may concern at Microsoft: It is extremely annoying that Microsoft is unable to fix this bug after almost two years, let alone provide a timeline for solving the problem. if I did my job like that, I would be bankrupt!

[filipnavara]

At the current pace we will have a 3-year anniversary of this bug report soon.

[merriemcgaw]

@dmitrii-drobotov Can your team take a look at this? I would love to get this addressed soon. FYI @Tanya-Solyanik

[dmitrii-drobotov]

@merriemcgaw sure, we'll look into it.

[elachlan]

Related: #8440, which suggested we remove the localization. Which would fix this issue.

[merriemcgaw]

Good call. @NikitaSemenovAkvelon are you still working on this? Take a look at @elachlan's suggestion above if you need some ideas.

[NikitaSemenovAkvelon]

Good call. @NikitaSemenovAkvelon are you still working on this? Take a look at @elachlan's suggestion above if you need some ideas.

8440 is not about the same case. #8401 already fixes this Issue by supporting provided culture in KeysConverter. Just waiting for review.

[jessekingf]

Is there any timeline on when this fix will be available? Will this be backported to .NET 6.0?

[merriemcgaw]

Thank you for bringing this back up. I can't commit to a specific timeframe, we are working on the best solution. We are definitely open to backporting this to 6.0 given the need we have seen from the community.

FYI - @Tanya-Solyanik

[Philip-Wang01]

Verified this on latest .NET 8.0.100-preview.7.23362.29, issue was fixed. But this issue still repro's on down level targets(net3.1,5.0,6.0,7.0), already an issue tracking it(https://github.com/microsoft/winforms-designer/issues/3305).

[Tanya-Solyanik]

@dmitrii-drobotov - could you please add a custom converter, that is based on this fix, to get registered on startup in designer repo to support downlevel versions of Core.

[Tanya-Solyanik]

@Philip-Wang01 - the fix was implemented only for .NET8

[Philip-Wang01]

Verified with .NET SDK 8.0.100-preview.7.23376.3 test pass build, this issue was fixed with above same verification result.