Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix System.Windows.Forms.InputLanguage.FromCulture() for languages without LANGID value #8573

Merged
merged 2 commits into from
Jun 14, 2023

Conversation

DJm00n
Copy link
Contributor

@DJm00n DJm00n commented Feb 6, 2023

Since Windows 8 many locale languages do not have a unique pre-assigned LANGID/LCID to identify them and in these cases we can have transient keyboard locale identifier values in the HKL lowerword:

  • 0x2000 - LOCALE_TRANSIENT_KEYBOARD1
  • 0x2400 - LOCALE_TRANSIENT_KEYBOARD2
  • 0x2800 - LOCALE_TRANSIENT_KEYBOARD3
  • 0x2C00 - LOCALE_TRANSIENT_KEYBOARD4

This is done to distingush up to 4 0x1000 (LOCALE_CUSTOM_UNSPECIFIED) or 0x0C00 (LOCALE_CUSTOM_DEFAULT) languages via old Win32 APIs.

But these values can't be used to identify the input language directly (we must convert it to BCP 47 language tag as soon as possible and use language tags where possible), and in result we can't get the InputLanguage correctly in the FromCulture method and may have wrong Culture in some cases..

More info about transient lang ids.

PS: Some background info on CultureInfo.KeyboardLayoutId that was used in FromCulture() method before my changes. Long story short - it should be avoided. :)

Proposed changes

Customer Impact

  • InputLanguage.Culture and InputLanguage.FromCulture() now should properly work for these locales:
Language tag Language display name Default KLID Keyboard Layout Name
got-Goth Gothic 000C0C00 Gothic
ff-Adlm Fulfulde (ADLaM) 00140C00 ADLaM
jv-Java Javanese (Javanese) 00110C00 Javanese
jv-Latn Javanese (Latin) 00000409 US
mg Malagasy 0000040C French
nqo N’ko 00090C00 N’Ko
sn-Latn Shona 00000409 US
vro-Latn Võro 00000409 US
zgh-Tfng Standard Moroccan Tamazight 0000105F Tifinagh (Basic)

Regression?

  • No

Risk

Screenshots

Before

image

Get-WinUserLanguageList PowerShell cmdlet output:

image

After

image

Test methodology

Autotesting:

  • InputLanguage_FromCulture_SupplementalInputLanguages_Expected (new)

Manually:

  • Install "N’Ko" input language (or any other from the list above) in Windows settings
  • Set it as default input language in Windows settings
  • Try to run InputLanguage_FromCulture_Roundtrip_Success test and see the results

Accessibility testing

Test environment(s)

dotnet --info
.NET SDK:
 Version:   8.0.100-alpha.1.23061.8
 Commit:    c8d103ed3c

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19044
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   G:\project\winforms\.dotnet\sdk\8.0.100-alpha.1.23061.8\

Host:
  Version:      8.0.0-alpha.1.23079.4
  Architecture: x64
  Commit:       a34291586e

.NET SDKs installed:
  8.0.100-alpha.1.22512.5 [G:\project\winforms\.dotnet\sdk]
  8.0.100-alpha.1.22622.3 [G:\project\winforms\.dotnet\sdk]
  8.0.100-alpha.1.23061.8 [G:\project\winforms\.dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 8.0.0-alpha.1.22510.12 [G:\project\winforms\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.0-alpha.1.22615.1 [G:\project\winforms\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.0-alpha.1.23058.7 [G:\project\winforms\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 8.0.0-alpha.1.22507.5 [G:\project\winforms\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0-alpha.1.22605.1 [G:\project\winforms\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0-alpha.1.22627.5 [G:\project\winforms\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0-alpha.1.23057.5 [G:\project\winforms\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0-alpha.1.23058.2 [G:\project\winforms\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0-alpha.1.23079.4 [G:\project\winforms\.dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 8.0.0-alpha.1.22507.3 [G:\project\winforms\.dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.0-alpha.1.22614.1 [G:\project\winforms\.dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.0-alpha.1.23057.1 [G:\project\winforms\.dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  DOTNET_ROOT       [G:\project\winforms\.dotnet]

global.json file:
  G:\project\winforms\global.json

Related issue in Runtime: dotnet/runtime#81633

@DJm00n DJm00n requested a review from a team as a code owner February 6, 2023 15:35
@ghost ghost assigned DJm00n Feb 6, 2023
@elachlan
Copy link
Contributor

elachlan commented Feb 6, 2023

There are a couple of test failures. Great job in tracking this down!

@DJm00n
Copy link
Contributor Author

DJm00n commented Feb 18, 2023

I tried to use CultureInfo constructor with LANGID parameter but it turns out that the LCIDToLocaleName API, which is used inside CultureInfo, may return incorrect language tags for transient language identifiers. For example, it returns "nqo-GN" and "jv-Java-ID" instead of the "nqo" and "jv-Java"(as seen in the Get-WinUserLanguageList PowerShell cmdlet).

I had to use undocumented Bcp47FromHkl API that returns valid values in these cases.

I still need to add some proper code to test these locales with transient LCIDs.

@DJm00n

This comment was marked as resolved.

@elachlan
Copy link
Contributor

@merriemcgaw I think @JeremyKuhne will need to review this because of the addition of a DLL Import?

Does Microsoft have some documentation around Bcp47FromHkl? If its undocumented, then it might not be a great idea to use it.

As far as I can tell bcp47langs.dll is a kernel DLL and wouldn't be included in Win32Metadata for inclusion in CsWin32. Although it might make it into wdkmetadata, which might mean CsWin32 could include it if it decided to support that as well.

@mikebattista please correct me if I way off track here and it isn't a kernel DLL and just missing from Win32Metadata.

@DJm00n
Copy link
Contributor Author

DJm00n commented Feb 28, 2023

@dotnet-policy-service agree

@DJm00n

This comment was marked as resolved.

@DJm00n DJm00n requested a review from elachlan February 28, 2023 21:24
@elachlan
Copy link
Contributor

Thanks for the heads up. We had something similar in other PRs.

@dmitrii-drobotov there is another ARM test failure in this issue. Is it related to #8672 (comment)?

elachlan
elachlan previously approved these changes Feb 28, 2023
Copy link
Contributor

@elachlan elachlan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! I imagine the failures are flaky tests. But the team will be able to tell you after reviewing logs.

@dmitrii-drobotov
Copy link
Contributor

dmitrii-drobotov commented Mar 1, 2023

@dmitrii-drobotov there is another ARM test failure in this issue. Is it related to #8672 (comment)?

Yes, it looks like a flaky test not related to this PR.

Stack trace
 System.Windows.Forms.Tests.TabControlTests.TabControl_OnHandleCreated_InvokeWithHandle_CallsHandleCreated(eventArgs: EventArgs { }) [FAIL]
       System.IndexOutOfRangeException : Index was outside the bounds of the array.
      Stack Trace:
           at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
           at System.Collections.Generic.Dictionary`2.set_Item(TKey key, TValue value)
        /_/src/System.Windows.Forms/src/System/Windows/Forms/NativeWindow.cs(301,0): at System.Windows.Forms.NativeWindow.CreateWindowId(IHandle`1 handle)
        /_/src/System.Windows.Forms/src/System/Windows/Forms/TabControl.cs(1259,0): at System.Windows.Forms.TabControl.OnHandleCreated(EventArgs e)
        /_/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/TabControlTests.cs(5804,0): at System.Windows.Forms.Tests.TabControlTests.SubTabControl.OnHandleCreated(EventArgs e)
        /_/src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/TabControlTests.cs(4363,0): at System.Windows.Forms.Tests.TabControlTests.TabControl_OnHandleCreated_InvokeWithHandle_CallsHandleCreated(EventArgs eventArgs)
           at InvokeStub_TabControlTests.TabControl_OnHandleCreated_InvokeWithHandle_CallsHandleCreated(Object, Object, IntPtr*)
           at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
 
Unhandled exception. System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.Collections.Generic.Dictionary`2.set_Item(TKey key, TValue value)
   at System.Windows.Forms.NativeWindow.CreateWindowId(IHandle`1 handle) in /_/src/System.Windows.Forms/src/System/Windows/Forms/NativeWindow.cs:line 301
   at System.Windows.Forms.TabControl.OnHandleCreated(EventArgs e) in /_/src/System.Windows.Forms/src/System/Windows/Forms/TabControl.cs:line 1259
   at System.Windows.Forms.Control.WmCreate(Message& m) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 12047
   at System.Windows.Forms.Control.WndProc(Message& m) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 13056
   at System.Windows.Forms.TabControl.WndProc(Message& m) in /_/src/System.Windows.Forms/src/System/Windows/Forms/TabControl.cs:line 2133
   at System.Windows.Forms.Control.ControlNativeWindow.OnMessage(Message& m) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.ControlNativeWindow.cs:line 65
   at System.Windows.Forms.Control.ControlNativeWindow.WndProc(Message& m) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.ControlNativeWindow.cs:line 114
   at System.Windows.Forms.NativeWindow.Callback(HWND hWnd, WM msg, WPARAM wparam, LPARAM lparam) in /_/src/System.Windows.Forms/src/System/Windows/Forms/NativeWindow.cs:line 375
--- End of stack trace from previous location ---
   at System.Windows.Forms.Application.ThreadContext.OnThreadException(Exception ex) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Application.ThreadContext.cs:line 869
   at System.Windows.Forms.Application.OnThreadException(Exception t) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Application.cs:line 1086
   at System.Windows.Forms.Control.WndProcException(Exception e) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs:line 13297
   at System.Windows.Forms.Control.ControlNativeWindow.OnThreadException(Exception e) in /_/src/System.Windows.Forms/src/System/Windows/Forms/Control.ControlNativeWindow.cs:line 59
   at System.Windows.Forms.NativeWindow.Callback(HWND hWnd, WM msg, WPARAM wparam, LPARAM lparam) in /_/src/System.Windows.Forms/src/System/Windows/Forms/NativeWindow.cs:line 389

It may be worth rebasing on the latest main to get the fix from #8695.

@DJm00n
Copy link
Contributor Author

DJm00n commented Mar 2, 2023

I found another way to convert the transient LANGID into a language tag (without a bug for transient LANGID language tags), which does not depend on the undocumented Bcp47FromHkl method, but depends on an undocumented registry key:

int langId = PARAM.LOWORD(_handle);
using RegistryKey? key = Registry.CurrentUser.OpenSubKey(@"Control Panel\International\User Profile");
if (key is not null && key.GetValue("Languages") is string[] languages)
{
    foreach (string language in languages)
    {
        using RegistryKey? subKey = key.OpenSubKey(language);
        if (subKey is not null && subKey.GetValue("TransientLangId") is int transientLangId &&
            transientLangId == langId)
        {
            return language;
        }
    }
}

return CultureInfo.GetCultureInfo(langId).Name;

I have tested this locally and everything looks good.
I can push this version if using Bcp47FromHkl is not allowed.

@elachlan @RussKie what do you think about this approach?

@elachlan
Copy link
Contributor

I found another way to convert the transient LANGID into a language tag (without a bug for transient LANGID language tags), which does not depend on the undocumented Bcp47FromHkl method, but depends on an undocumented registry key:

int langId = PARAM.LOWORD(_handle);
using RegistryKey? key = Registry.CurrentUser.OpenSubKey(@"Control Panel\International\User Profile");
if (key is not null && key.GetValue("Languages") is string[] languages)
{
    foreach (string language in languages)
    {
        using RegistryKey? subKey = key.OpenSubKey(language);
        if (subKey is not null && subKey.GetValue("TransientLangId") is int transientLangId &&
            transientLangId == langId)
        {
            return language;
        }
    }
}

return CultureInfo.GetCultureInfo(langId).Name;

I have tested this locally and everything looks good. I can push this version if using Bcp47FromHkl is not allowed.

@elachlan @RussKie what do you think about this approach?

Anything undocumented is probably a no-go. Since I believe Microsoft wouldn't consider it a "public API". We will have to wait for the response from someone in Microsoft working on Windows to let us know what is possible. It might just mean Bcp47FromHkl gets documented.

@DJm00n DJm00n requested a review from RussKie March 28, 2023 13:51
@JeremyKuhne
Copy link
Member

@elachlan yes, we cannot use undocumented Windows APIs in the shipping binaries. There are long standing legal reasons, so there isn't a lot of flexibility. We'll try to get things documented so we can close this gap properly.

This sort of thing does come up from time to time, but it is infrequent enough that it is always a little rocky. Thanks for your patience on this everyone.

@DJm00n
Copy link
Contributor Author

DJm00n commented May 22, 2023

Added some comments. Thanks for the patience here.

@JeremyKuhne updated PR. please take a look

@DJm00n DJm00n requested a review from JeremyKuhne May 22, 2023 13:40
DJm00n added a commit to DJm00n/winforms that referenced this pull request May 29, 2023
Many supported Windows languages do not have a unique pre-assigned LANGID to
identify them and instead use a value 0x1000 (LOCALE_CUSTOM_UNSPECIFIED) since
Windows 8.

Windows tries to allocate and provide a unique transient value (from a pool four
values: 0x2000, 0x2400, 0x2800, 0x2C00) in these cases in the HKL lowerword to
distinguish between different languages via the old Win32 APIs, but these
values can't be used to identify the keyboard layout directly, and in result we
can't get the InputLanguage correctly in the FromCulture method.

Aquire corresponging IETF BCP 47 language tag with a call to a proper API
and compare locale names instead of LANGID to fix these corner cases.

I tried to use CultureInfo constructor with LANGID parameter but it turns out
that the LCIDToLocaleName API, which is used inside CultureInfo, may return
incorrect language tags for transient language identifiers. For example, it
returns "nqo-GN" and "jv-Java-ID" instead of the "nqo" and "jv-Java"(as seen in
the Get-WinUserLanguageList PowerShell cmdlet).

I had to use undocumented registry keys to to extract proper language tag from a
LANGID. This workaround was approved by a Windows team.
dotnet#8573 (comment)
@DJm00n
Copy link
Contributor Author

DJm00n commented Jun 2, 2023

@JeremyKuhne @dreddy-work any updates on this?

Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about the delays in responses, been very busy. Just a few more comments. Please tag me when you've responded.

@@ -180,6 +172,62 @@ internal string LayoutId
}
}

private static readonly int[] s_transientLangIds =
{
0x2000, // LOCALE_TRANSIENT_KEYBOARD1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be added to the NativeMethods.txt file in System.Windows.Forms.Primitives.

0x2C00 // LOCALE_TRANSIENT_KEYBOARD4
};

private static string s_userProfileRegistryPath => @"Control Panel\International\User Profile";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a constant, not a static.

@ghost ghost added the 📭 waiting-author-feedback The team requires more information from the author label Jun 2, 2023
Many supported Windows languages do not have a unique pre-assigned LANGID to
identify them and instead use a value 0x1000 (LOCALE_CUSTOM_UNSPECIFIED) since
Windows 8.

Windows tries to allocate and provide a unique transient value (from a pool four
values: 0x2000, 0x2400, 0x2800, 0x2C00) in these cases in the HKL lowerword to
distinguish between different languages via the old Win32 APIs, but these
values can't be used to identify the keyboard layout directly, and in result we
can't get the InputLanguage correctly in the FromCulture method.

Aquire corresponging IETF BCP 47 language tag with a call to a proper API
and compare locale names instead of LANGID to fix these corner cases.

It turns out that the LCIDToLocaleName API, which is used inside CultureInfo, may
return incorrect language tags for transient language identifiers. For example, it
returns "nqo-GN" and "jv-Java-ID"  instead of the "nqo" and "jv-Java" (as seen in
the Get-WinUserLanguageList PowerShell cmdlet).

I had to use undocumented registry keys to to extract proper language tag from a
LANGID. This workaround was approved by a Windows team.
dotnet#8573 (comment)
@ghost ghost removed the 📭 waiting-author-feedback The team requires more information from the author label Jun 5, 2023
@DJm00n
Copy link
Contributor Author

DJm00n commented Jun 5, 2023

@JeremyKuhne please take another look at this. I've made corrections.

JeremyKuhne
JeremyKuhne previously approved these changes Jun 5, 2023
Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience!

@DJm00n
Copy link
Contributor Author

DJm00n commented Jun 14, 2023

@JeremyKuhne any updates?

@dreddy-work dreddy-work enabled auto-merge (squash) June 14, 2023 17:08
@dreddy-work dreddy-work merged commit 84a94ae into dotnet:main Jun 14, 2023
@ghost ghost added this to the 8.0 Preview6 milestone Jun 14, 2023
@elachlan
Copy link
Contributor

Great work @DJm00n!

@Olina-Zhang
Copy link
Member

Verified it in the Release/8.0-Preview6 branch from this repo, result of running InputLanguage_FromCulture_Roundtrip_Success test is failed as following. Is there some wrong from my setting? @DJm00n
image

@DJm00n
Copy link
Contributor Author

DJm00n commented Jun 29, 2023

Hi @Olina-Zhang!
Can you please run Get-WinUserLanguageList in PowerShell and provide output on your system?

@Olina-Zhang
Copy link
Member

@DJm00n Here is Get-WinUserLanguageList PowerShell cmdlet output:
image

@DJm00n
Copy link
Contributor Author

DJm00n commented Jun 30, 2023

@Olina-Zhang interesting...

  • 0xf0402000 is 0x040 "Layout Id" under HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Keyboard Layouts\00090C00 - which looks okay
  • 0xf03e2000 is 0x03e "Layout Id" under HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Keyboard Layouts\00070C00 - which looks okay

0x2000 (LOCALE_TRANSIENT_KEYBOARD1) in both cases means that these layouts are installed under one input language.. but Get-WinUserLanguageList shows only one of them under 0x2000 langID.

This looks like some kind of configuration issue on your PC.

If it is an RDP connection then it could be related too (since it can install keyboard layouts from the remote system):
https://superuser.com/questions/483086/how-do-i-remove-automatically-added-keyboard-layouts

You can also check these registry paths for this suspicious "00070C00" keyboard layout:

HKEY_USERS\.DEFAULT\Control Panel\International\User Profile
HKEY_USERS\.DEFAULT\Keyboard Layout\Preload
HKEY_USERS\.DEFAULT\Keyboard Layout\Substitutes
HKEY_USERS\S-1-5-18\Control Panel\International\User Profile
HKEY_USERS\S-1-5-18\Keyboard Layout\Preload
HKEY_USERS\S-1-5-18\Keyboard Layout\Substitutes
HKEY_CURRENT_USER\Control Panel\International\User Profile
HKEY_CURRENT_USER\Keyboard Layout\Preload
HKEY_CURRENT_USER\Keyboard Layout\Substitutes

Or attach files generated from these commands in PowerShell:

REG EXPORT "HKEY_USERS\.DEFAULT\Control Panel\International\User Profile" d:\def-userprofile.reg
REG EXPORT "HKEY_USERS\.DEFAULT\Keyboard Layout" d:\def-keyboard.reg
REG EXPORT "HKEY_USERS\S-1-5-18\Control Panel\International\User Profile" d:\sys-userprofile.reg
REG EXPORT "HKEY_USERS\S-1-5-18\Keyboard Layout" d:\sys-keyboard.reg
REG EXPORT "HKEY_CURRENT_USER\Control Panel\International\User Profile" d:\cur-userprofile.reg
REG EXPORT "HKEY_CURRENT_USER\Keyboard Layout" d:\cur-keyboard.reg

@Olina-Zhang
Copy link
Member

@DJm00n Thanks for your comment! The testing machine I used is connected by remoting. It may be related some machine setting. Today re-tested that test case: InputLanguage_FromCulture_Roundtrip_Success, it passed. And checked machine with more input languages automatically.
Input languages:
image
Case passed:
image
image

@ghost ghost locked as resolved and limited conversation to collaborators Aug 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants