Skip to content

Conversation

@tarekgh
Copy link
Member

@tarekgh tarekgh commented Aug 19, 2021

#57310

This change is fixing a regression from .NET 5.0. The problem happens when trying to create a CultureInfo object using culture names that Windows don't allow.
This regression occurred during the code refactoring work done to optimize the globalization data size and WASM support.
Although this fix is addressing the regression, it fixes another issue I have discovered during the investigation. Here is what the change is doing:

  • In the file CultureData.Windows.cs, the condition ShouldUseUserOverrideNlsData is wrong to use. The reason is this part of the code is not concerned about the user overrides. Even the user overrides flag is not initialized yet at this stage.
  • In the same file, I added the call to Windows API GetLocaleInfoEx that help validate the culture name. That is what we have done in .NET 5.0 too.
  • In the file IcuLocaleData.cs, there was a bug that caused not finding the proper culture name mapping to our data because the culture name containing characters like underscore character _.
  • Fixing and adding tests.

@ghost
Copy link

ghost commented Aug 19, 2021

Tagging subscribers to this area: @tarekgh, @safern
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #57310

This change is fixing a regression from .NET 5.0. The problem happens when trying to create a CultureInfo object using culture names that Windows don't allow.
This regression occurred during the code refactoring work done to optimize the globalization data size and WASM support.
Although this fix is addressing the regression, it fixes another issue I have discovered during the investigation. Here is what the change is doing:

  • In the file CultureData.Windows.cs, the condition ShouldUseUserOverrideNlsData is wrong to use. The reason is this part of the code is not concerned about the user overrides. Even the user overrides flag is not initialized yet at this stage.
  • In the same file, I added the call to Windows API GetLocaleInfoEx that help validate the culture name. That is what we have done in .NET 5.0 too.
  • In the file IcuLocaleData.cs, there was a bug that caused not finding the proper culture name mapping to our data because the culture name containing characters like underscore _.
  • Fixing and adding tests.
Author: tarekgh
Assignees: -
Labels:

area-System.Globalization

Milestone: -

@tarekgh tarekgh self-assigned this Aug 19, 2021
@tarekgh tarekgh added this to the 6.0.0 milestone Aug 19, 2021
@tarekgh
Copy link
Member Author

tarekgh commented Aug 19, 2021

@ericstj @jeffhandley just FYI. this is a fix I am going to port to rc2 when it is ready. OfCourse, will loop you on the porting PR at that time.

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM

@tarekgh tarekgh merged commit 1313a68 into dotnet:main Aug 20, 2021
@tarekgh tarekgh deleted the FixGlobalizationRegression branch August 20, 2021 16:28
@tarekgh
Copy link
Member Author

tarekgh commented Aug 20, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1151573976

@dotnet dotnet deleted a comment from github-actions bot Aug 20, 2021
@dotnet dotnet deleted a comment from github-actions bot Aug 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants