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

PredefinedCulturesOnly=false not respected in new CultureInfo(int) #86878

Closed
OwnageIsMagic opened this issue May 29, 2023 · 8 comments · Fixed by #87002
Closed

PredefinedCulturesOnly=false not respected in new CultureInfo(int) #86878

OwnageIsMagic opened this issue May 29, 2023 · 8 comments · Fixed by #87002
Labels
area-System.Globalization in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@OwnageIsMagic
Copy link
Contributor

OwnageIsMagic commented May 29, 2023

Description

This creates issue for WinForms
dotnet/winforms#9191 (comment)

see

internal static CultureData GetCultureData(int culture, bool bUseUserOverride)
{
CultureData? retVal = null;
if (culture == CultureInfo.LOCALE_INVARIANT)
{
return Invariant;
}
if (GlobalizationMode.Invariant)
{
// LCID is not supported in the InvariantMode
throw new CultureNotFoundException(nameof(culture), culture, SR.Argument_CultureNotSupportedInInvariantMode);

and
internal static CultureData? GetCultureData(string? cultureName, bool useUserOverride)
{
// First do a shortcut for Invariant
if (string.IsNullOrEmpty(cultureName))
{
return CultureData.Invariant;
}
if (GlobalizationMode.PredefinedCulturesOnly)
{
if (GlobalizationMode.Invariant || (GlobalizationMode.UseNls ? !NlsIsEnsurePredefinedLocaleName(cultureName): !IcuIsEnsurePredefinedLocaleName(cultureName)))
return null;
}

Reproduction Steps

dotnet build /p:InvariantGlobalization=true /p:PredefinedCulturesOnly=false && dotnet run --no-build

_ = new CultureInfo(1100);

Expected behavior

Return Invariant culture.

Actual behavior

Exception.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 29, 2023
@ghost
Copy link

ghost commented May 29, 2023

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

This creates issue for WinForms
dotnet/winforms#9191 (comment)

see

internal static CultureData GetCultureData(int culture, bool bUseUserOverride)
{
CultureData? retVal = null;
if (culture == CultureInfo.LOCALE_INVARIANT)
{
return Invariant;
}
if (GlobalizationMode.Invariant)
{
// LCID is not supported in the InvariantMode
throw new CultureNotFoundException(nameof(culture), culture, SR.Argument_CultureNotSupportedInInvariantMode);

and
internal static CultureData? GetCultureData(string? cultureName, bool useUserOverride)
{
// First do a shortcut for Invariant
if (string.IsNullOrEmpty(cultureName))
{
return CultureData.Invariant;
}
if (GlobalizationMode.PredefinedCulturesOnly)
{
if (GlobalizationMode.Invariant || (GlobalizationMode.UseNls ? !NlsIsEnsurePredefinedLocaleName(cultureName): !IcuIsEnsurePredefinedLocaleName(cultureName)))
return null;
}

Reproduction Steps

dotnet build /p:InvariantGlobalization=true /p:PredefinedCulturesOnly=false && dotnet run --no-build

_ = new CultureInfo(1100);

Expected behavior

return Invariant culture

Actual behavior

exception

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author: OwnageIsMagic
Assignees: -
Labels:

area-System.Globalization, untriaged

Milestone: -

@tarekgh tarekgh added this to the Future milestone May 30, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label May 30, 2023
@tarekgh
Copy link
Member

tarekgh commented May 30, 2023

Why would you enable the invariant globalization mode for a WinForm app? I believe this is not the suitable environment for running WinForm apps for several reasons. Additionally, we generally discourage the use of LCID (Locale Identifier) as it is considered obsolete, and we try to avoid investing in outdated technologies. The appropriate course of action in this case is to disable the invariant mode for your WinForm apps.

@tarekgh tarekgh added the needs-author-action An issue or pull request that requires more info or actions from the author. label May 30, 2023
@ghost
Copy link

ghost commented May 30, 2023

This issue has been marked needs-author-action and may be missing some important information.

@OwnageIsMagic
Copy link
Contributor Author

OwnageIsMagic commented May 30, 2023

I'm developing tool to convert data from one well defined format to another, and doesn't have any plans on localization. InvariantGlobaliztion helps me to ensure that I don't accidentally depend on current culture of my dev environment and saves me from scanning WinForms and other dependencies repos in case it does somewhere sets CurrentCulture/CurrentUICulture/DefaultThreadCurrentCulture/DefaultThreadCurrentUICulture or any other global property I'm not aware of. (I once had to deal with a library that used some specific culture instead of format string to format dates. And one day came a user with overridden NLS settings ...)

Usage of this constructor is tied to what we get from WM_INPUTLANGCHANGE Win32 message.

Fix is trivial and will bring in consistency between constructor overloads.

  internal static CultureData GetCultureData(int culture, bool bUseUserOverride)
  {
      CultureData? retVal = null;
  
      if (culture == CultureInfo.LOCALE_INVARIANT)
      {
          return Invariant;
      }
 
      if (GlobalizationMode.Invariant)
      {
+         if (!GlobalizationMode.PredefinedCulturesOnly)
+         {
+              return Invariant;
+         }
          // LCID is not supported in the InvariantMode
          throw new CultureNotFoundException(nameof(culture), culture, SR.Argument_CultureNotSupportedInInvariantMode);
      }

@ghost ghost added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels May 30, 2023
@tarekgh
Copy link
Member

tarekgh commented May 30, 2023

@OwnageIsMagic thanks for the info. in your tool, who is creating the culture using the LCID? Is it your tool or WinFoms? If it is your tool, then you can just avoid creating the culture in the first place and use Invariant Culture. If it is WinForms, then you have something seriously wrong trying to use WinFomrs and expect it will handle the cultural operation correctly while you are enabling Invariant Globalization mode.

Fix is trivial

This is not the point; the point is we need to refrain users from using LCIDs in general. Helping fixing cases to support LCIDs will make it worse to obsolete such APIs moving forward.

@tarekgh tarekgh added needs-author-action An issue or pull request that requires more info or actions from the author. and removed needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration labels May 30, 2023
@ghost
Copy link

ghost commented May 30, 2023

This issue has been marked needs-author-action and may be missing some important information.

@OwnageIsMagic
Copy link
Contributor Author

@tarekgh the culture is created in WinForms as part of creating EventArgs for handling system broadcast message, it's not used internally, but it's a part of public api.
Issue can be worked around on WinForms side, but exception message links to this property as documented workaround.

the point is

In invariant mode LCID is actually unused and ignored, so I don't think that can be considered as support.

@ghost ghost added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels May 31, 2023
@tarekgh tarekgh removed the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Jun 1, 2023
@tarekgh
Copy link
Member

tarekgh commented Jun 1, 2023

@OwnageIsMagic are you interested to submit a PR?

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 1, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Globalization in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants