Fixing runtime crash if an application contains ToolStripMenuItem with configured shortcut and launched in other locales.

[NikitaSemenovAkvelon]

Fixes #2886

Proposed changes

• Added support for the culture parameter in ConvertFrom and ConvertTo methods.
• Dictionaries with localization are loaded per request.
• KeysConverter becomes more independent and doesn't contain the logic of choosing the "right" locale.

Customer Impact

• No crashes when an application is run in different locales.
• A developer's locale doesn't impact values in the .resx file.
• The placeholder when the shortcut isn't set becomes more apparent.

Regression?

• Yes (after migration to .net core).

Risk

• Old applications that contain localized values of ShortcutKeys (e.g. Strg for de-DE) are crashed.
• ShortcutKeysEditor seems inconsistent due to KeysConverter receiving CultureInfo.CurrentCulture while ShortcutKeysUI is localized with CultureInfo.CurrentUICulture.

Screenshots

Before

After

Test methodology

• Manually

Test environment(s)

.NET SDK:
Version: 8.0.100-alpha.1.22512.5
Commit: 1b80461e45

Runtime Environment:
OS Name: Windows
OS Version: 10.0.22621

Microsoft Reviewers: Open in CodeFlow

[NikitaSemenovAkvelon]

Important notes:

• In net481 it looks this way:
  
  

• Resource files contain value depending on version:
  net481: Ctrl+D
  net8 (before the fix): Strg+D
  net8 (after the fix): Ctrl+D

A few suggestions:

1. To fix the crashes of old applications (the risk mentioned in the PR's description), add a workaround in ShortcutKeysEditor that would also check a parameter in the current locale in the ConvertFrom method.
2. Make a difference in resources of ShortcutKeysEditor for looking as in the net481 version.
3. Cover KeysConverter with tests when a final version would be applied.

[v-elnovikova]

LGTM

[dmitrii-drobotov]

1. Let's say there is a .NET 7 app with "Strg+D" shortcut in resources, which is later updated to .NET 8 with this fix. As I understand the app will crash on runtime and designer won't load the form. I'm not sure it will be clear for developers what exactly needs to be changed in order to fix it. Is it possible to provide more details in this case? Maybe a breaking change page in docs could be useful?
2. Does it also fix scenario from #5036?

[Tanya-Solyanik]

What happens if a NET 8application is retargetted to NET7 in VS?

[NikitaSemenovAkvelon]

Let's say there is a .NET 7 app with "Strg+D" shortcut in resources, which is later updated to .NET 8 with this fix. As I understand the app will crash on runtime and designer won't load the form.

Yes, an error would look like this:

Is it possible to provide more details in this case? Maybe a breaking change page in docs could be useful?

Yes, we could handle this case and throw an exception with a detailed message. Also, we can modify the docs.

Does it also fix scenario from #5036?

I guess, it looks like the reason is the same.

What happens if a NET 8application is retargetted to NET7 in VS?

It depends on the user's locale. For example, if the user has de-DE, the behavior will be exactly the same as described in this issue. But in other locales localized value of toStringControl is the same as in invariant culture.
I published this fix because in my humble opinion it covered most of the cases without workarounds. Even if we add a second dictionary that contains localized values it will crash when the resource file will have localized value in one locale while the application is opened in another non-invariant locale (e.g. toStringBack has different values in each locale).
To prevent all crashes we have to check an incoming value in all locales and return only in the invariant culture. Whereas, it would a workaround for supporting previous versions and it also would reduce speed a bit.

[Tanya-Solyanik]

In addition to this change we could add a custom TypeConverter to the designer to handle localization for downlevel versions of the framework.

[NikitaSemenovAkvelon]

I've decided to rework PR to support provided culture in ConvertFrom and ConvertTo methods. According to a comment (#2886 (comment)) it was really broken. And it works as expected, the bug has gone but localization in runtime is still working.

[elachlan]

Maybe we should merge PR #8485 with this?

[elachlan]

Are the keys serialized to their non-localized values in the designer code? What happens if the translation changes? Can a form be designed okay between culture changes?

[dreddy-work]

Are the keys serialized to their non-localized values in the designer code? What happens if the translation changes? Can a form be designed okay between culture changes?

Localized Forms serialize into resx for the properties that are language dependent. May be i didn't get your question.

[Tanya-Solyanik]

1. What controls besides MenuItems should be tested(i.e. have shortcut keys)?

2.Scenario:

• on EN OS and EN VS, create a form with toolstrip
• add a menu item with a shortcut key >>> Form1.resx contains Ctrl+A for a shortcut
• Make the form localizable, set language to DE-DE
• Change the shortcut key to Ctrl+B->from1.de-DE.resx contains Ctrl+B as a shortcut, i.e. localized resource file has an invariant key name - this matches the .NET FX
  Question: run that app on a DE machine with this change - which resource file is the shortcut read from in a Core app and in a FX app?

[riverar]

What controls besides MenuItems should be tested(i.e. have shortcut keys)?

Our app uses the KeysConverter directly and round trips text in the control. We are satisified that the tests in this PR cover our scenario.

[dmitrii-drobotov]

What controls besides MenuItems should be tested(i.e. have shortcut keys)?

I don't see any other controls with Keys properties, other than ToolStripMenuItem. But I guess it's possible to create a custom control with Keys property, and it would be used there. There's also ShortcutKeysEditor.ShortcutKeysUI which will be opened in a PropertyGrid entry for a Keys property:

[dmitrii-drobotov]

2.Scenario:

• on EN OS and EN VS, create a form with toolstrip
• add a menu item with a shortcut key >>> Form1.resx contains Ctrl+A for a shortcut
• Make the form localizable, set language to DE-DE
• Change the shortcut key to Ctrl+B->from1.de-DE.resx contains Ctrl+B as a shortcut, i.e. localized resource file has an invariant key name - this matches the .NET FX
  Question: run that app on a DE machine with this change - which resource file is the shortcut read from in a Core app and in a FX app?

@Tanya-Solyanik
The shortcut will be read from DE resource file both in .NET Fx and .NET 8 with or without this PR. But Form1.de-DE.resx needs to be created manually, for me it wasn't created automatically even on all DE settings. And the DE shortcut value also needs to be set manually, but it will override invariant value correctly.

[Tanya-Solyanik]

But Form1.de-DE.resx needs to be created manually, for me it wasn't created automatically even on all DE settings.

This is strange. When I set Localizable property on the form to true and Language to de-DE, Form1.de-DE.resx is created

[dmitrii-drobotov]

This is strange. When I set Localizable property on the form to true and Language to de-DE, Form1.de-DE.resx is created

Sorry, I initially misunderstood the steps, now Form1.de-DE.resx is created, and I confirmed that results are the same as with manually created resource file.