[iOS] Retrieve device locale in full (specific) format from ObjectiveC APIs #111032

matouskozak · 2025-01-02T12:08:36Z

Problem Description

On iOS (and other apple mobile platforms), we are taking the system default culture and using it inside .NET as CultureInfo.CurrentCulture.

Currently, the system default culture is truncated into neutral culture format (only language). This behavior was set in #104071, where we had an incorrect assumption that other ICU platforms also return just neutral locale name. However, that isn't true because the ICU uloc_getBaseName

runtime/src/native/libs/System.Globalization.Native/pal_locale.c

Line 232 in 117c4ab

    
           uloc_getBaseName(defaultLocale, localeNameBuffer, ULOC_FULLNAME_CAPACITY, &status);

only strips collation information as described in https://github.com/unicode-org/icu/blob/d9d09db2a7167eea2b5749836ef32cdd45303fdd/icu4c/source/common/unicode/uloc.h#L948.

The problem of using all Apple's system default locales is that some of the retrieved identifiers are not part of https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Globalization/IcuLocaleData.cs (e.g., en-CZ), causing the culture LCID number to be 0x1000 (LOCALE_CUSTOM_UNSPECIFIED). This can cause issues where the default CultureInfo.CurrentCulture causes crashes when using certain CultureInfo APIs that accept LCID instead of culture name, such as,

runtime/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs

Line 993 in 94c356d

public static CultureInfo GetCultureInfo(int culture)

~~used by e.g. SqlString.~~

Proposed Change

This PR reverts the previous change and preserves the full (specific = language-region) culture locale format when the retrieved locale is part of IcuLocaleData.cs. Upon retrieving the system locale name, we verify that the culture name is part of the ICU data and if not, we use the parent culture (neutral) instead.

Previous behavior (.net 9):

Apple system culture	`CurrentCulture`	IsCustomCulture?
`en-US`	`en`	no
`en-CZ`	`en`	no

New behavior:

Apple system culture	`CurrentCulture`	IsCustomCulture?
`en-US`	`en-US`	no
`en-CZ`	`en-CZ`	yes

This PR addresses the following issues:

dotnet-policy-service · 2025-01-02T12:09:06Z

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

Copilot

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

Files not reviewed (1)

src/native/libs/System.Globalization.Native/pal_locale.m: Language not supported

matouskozak · 2025-01-02T13:35:47Z

/azp run runtime-ioslike,runtime-ioslikesimulator,runtime-maccatalyst

azure-pipelines · 2025-01-02T13:36:09Z

Azure Pipelines successfully started running 3 pipeline(s).

vitek-karas · 2025-01-02T14:29:15Z

src/native/libs/System.Globalization.Native/pal_locale.m

-static NSString* GetBaseName(NSString *localeIdentifier)
-{
-    NSLocale *locale = [[NSLocale alloc] initWithLocaleIdentifier:localeIdentifier];
-    NSString *languageCode = [locale objectForKey:NSLocaleLanguageCode];
-    return languageCode;
-}
-


Do we know what this was done before?
The PR description should be updated to describe this - what we did before (has that) and why, and what we're changing it to (and why it's OK to change given the previous reasoning).

Indeed, I was trying this PR to see which test suit breaks if we switch to specific name because there are other dependencies such as SqlStrings that were previously affected. I'll update the issue description when I finalize the code changes.

Brief history what I found:

The switch to neutral locale names was done in [ios][HybridGlobalization] Fix default culture #104071

In that PR, there was an assumption that our ICU also returns just neutral locale name. However, that isn't true as the ICU uloc_getBaseName

runtime/src/native/libs/System.Globalization.Native/pal_locale.c

Line 232 in 117c4ab

uloc_getBaseName(defaultLocale, localeNameBuffer, ULOC_FULLNAME_CAPACITY, &status);

only strips collation information as described in https://github.com/unicode-org/icu/blob/d9d09db2a7167eea2b5749836ef32cdd45303fdd/icu4c/source/common/unicode/uloc.h#L948.

The problem with using Apple's system default locale is that if it's not part of https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Globalization/IcuLocaleData.cs (e.g., en-CZ), the culture LCID number is 0x1000 (LOCALE_CUSTOM_UNSPECIFIED). While it seems that using LCID number to create new cultures is not recommended and using culture name is preferred, e.g., SqlString implementation still relies on using it, thus when LCID doesn't match with valid culture it causes failures when using some CultureInfo APIs, e.g.,

runtime/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs

Line 993 in 94c356d

public static CultureInfo GetCultureInfo(int culture)

I'll try to make a middle way out of this, where we use system locale if it's recognized by .NET, otherwise we use parent culture which is neutral.

matouskozak · 2025-01-03T13:47:41Z

/azp run runtime-ioslike,runtime-ioslikesimulator,runtime-maccatalyst

azure-pipelines · 2025-01-03T13:47:57Z

Azure Pipelines successfully started running 3 pipeline(s).

matouskozak · 2025-01-06T08:31:19Z

/azp run runtime-ioslike,runtime-ioslikesimulator,runtime-maccatalyst

azure-pipelines · 2025-01-06T08:31:34Z

Azure Pipelines successfully started running 3 pipeline(s).

tarekgh · 2025-01-06T17:27:35Z

I don't think we need to fix this. We encourage users to move away from LCIDs. Any users still using LCID should start migrating to use culture names instead. The change here is trying to work around the issue of using the parent which may benefit using the LCID but it can cause other issues when requesting any region-specific data from current culture.

This issue is not specific to iOS or mobile platforms. This can occur on Windows for example doing CultureInfo.CurrentCulture = new CultureInfo("en-AE");. Whoever is using LCID needs to fix their code and start using culture names or they need to make their code tolerant when getting custom LCID values.

Last, if IcuLocaleData.cs missing any data, we can try to update it as needed.

matouskozak · 2025-01-06T18:34:21Z

Any users still using LCID should start migrating to use culture names instead.

@tarekgh I know that using LCID is not recommended. What I was trying to work around here by using the parent is that any default culture set by .NET would be compatible with all .NET libraries code. We are currently relying on LCID inside SqlString codebase, causing failures if LCID is custom culture.

Note, this PR not only does the fallback to parent culture but also fixes the behavior that we didn't return specific system culture. I can remove the parent culture workaround and disable the failing tests until the affected library code migrates from LCID. However, because we need to backport this fix to .NET 9, we could be risking a breaking change to some customers using e.g., SqlStrings.

tarekgh · 2025-01-06T18:51:31Z

What I was trying to work around here by using the parent is that any default culture set by .NET would be compatible with all .NET libraries code. We are currently relying on LCID inside SqlString codebase, causing failures if LCID is custom culture.

I think the right thing to do is have SqlString get fixed. Either they migrate to use the culture name or to be tolerant to do the fallback themselves. As I mentioned, this issue is not specific to iOS. What happens if someone sets en-AE as a default culture on Windows? They will run into the exact same problem. Also, your fix can cause some different problems. Like someone is using the CurrentCulture name to create a RegionInfo object. If you are using the parent (which most likely be a neutral culture, RegionInfo creation will throw exception.

but also fixes the behavior that we didn't return specific system culture.

I am fine with the change in pal_locale.m if other reviewers see this is a correct fix.

ivanpovazan · 2025-01-07T11:17:21Z

We encourage users to move away from LCIDs

Is there an official documentation about it?

We are currently relying on LCID inside SqlString codebase, causing failures if LCID is custom culture

Is SqlString codebase the only problem? Could we detect this in some way and produce a build warning, so that internally we can fix all the places relying on this?

tarekgh · 2025-01-07T17:12:06Z

Is there an official documentation about it?

https://learn.microsoft.com/en-us/globalization/locale/other-locale-names#lcid

matouskozak · 2025-01-13T09:07:46Z

We are currently relying on LCID inside SqlString codebase, causing failures if LCID is custom culture

Is SqlString codebase the only problem? Could we detect this in some way and produce a build warning, so that internally we can fix all the places relying on this?

I took a look and there are a few other places where LCID is used but I'm unsure if they would cause the crash. I would need to run each library test suit locally to verify because the test might not fail on CI as the default CurrentCulture on lab device is likely en-US (non-custom, not causing the before-mentioned crash).

To trigger build warnings, we would need to mark the APIs obsolete/depracated. Not sure if this is something that is planned based on https://learn.microsoft.com/en-us/globalization/locale/other-locale-names#lcid @tarekgh ?

matouskozak · 2025-01-16T12:20:22Z

/azp run runtime-ioslikesimulator

azure-pipelines · 2025-01-16T12:20:34Z

Azure Pipelines successfully started running 1 pipeline(s).

matouskozak · 2025-01-16T16:52:27Z

/azp run runtime-ioslike,runtime-ioslikesimulator,runtime-maccatalyst

azure-pipelines · 2025-01-16T16:52:49Z

Azure Pipelines successfully started running 3 pipeline(s).

matouskozak · 2025-01-17T12:22:47Z

I removed the fallback to neutral culture, thus retrieving the locale in full (specific) locale format.

I also added a test case to assert this behavior. However, for some reason the lab simulators are reporting only en instead of expected en-US. I verified locally that on simulators (down to iOS 15.5), the full locale identifier is retrieved. It is not the first time I've seen different behavior on lab simulators in comparison to local runs, so I opened an issue to investigate this further #111501.

Finally, I opened an issue to address the removal of LCID dependencies inside SqlString codebase issue for removal of LCID inside SqlString codebase #111395.

Please take a look and review the changes @tarekgh @ivanpovazan @akoeplinger @rolfbjarne @steveisok

ivanpovazan

LGTM, nice work!
I left a small comment.

...ies/System.Runtime/tests/System.Globalization.Tests/CultureInfo/CultureInfoCurrentCulture.cs

matouskozak · 2025-01-17T17:44:51Z

/azp run runtime-ioslikesimulator

azure-pipelines · 2025-01-17T17:45:01Z

Azure Pipelines successfully started running 1 pipeline(s).

matouskozak · 2025-01-20T09:38:38Z

/backport to release/9.0-staging

github-actions · 2025-01-20T09:38:53Z

Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/12865114650

Fix CurrentCulture to be in specific format

858ff2f

matouskozak added area-System.Globalization os-ios Apple iOS labels Jan 2, 2025

matouskozak self-assigned this Jan 2, 2025

matouskozak requested a review from Copilot January 2, 2025 12:10

Copilot AI reviewed Jan 2, 2025

View reviewed changes

vitek-karas reviewed Jan 2, 2025

View reviewed changes

build-analysis bot mentioned this pull request Jan 3, 2025

MSBuild crashing in the build #92290

Open

matouskozak changed the title ~~[iOS] Fix CurrentCulture locale to be in specific format~~ [iOS] Set CurrentCulture locale to specific format based on system default when possible Jan 6, 2025

matouskozak marked this pull request as ready for review January 6, 2025 11:05

matouskozak requested review from ivanpovazan, steveisok, tarekgh and vitek-karas January 6, 2025 11:05

matouskozak force-pushed the iOS-ICU-return-full-locale branch from 4dd8721 to b65a72f Compare January 16, 2025 12:20

disable the test for now

2cb6ca3

matouskozak force-pushed the iOS-ICU-return-full-locale branch from b65a72f to 2cb6ca3 Compare January 16, 2025 16:51

build-analysis bot mentioned this pull request Jan 16, 2025

Assertion failed 'HBtab->ebdTryBeg == block' during 'Optimize layout' #111347

Closed

matouskozak changed the title ~~[iOS] Set CurrentCulture locale to specific format based on system default when possible~~ [iOS] Retrieve device locale in full (specific) format from preferredLanguages API Jan 17, 2025

matouskozak marked this pull request as ready for review January 17, 2025 12:18

matouskozak requested a review from rolfbjarne January 17, 2025 12:23

matouskozak changed the title ~~[iOS] Retrieve device locale in full (specific) format from preferredLanguages API~~ [iOS] Retrieve device locale in full (specific) format from ObjectiveC APIs Jan 17, 2025

tarekgh approved these changes Jan 17, 2025

View reviewed changes

ivanpovazan approved these changes Jan 17, 2025

View reviewed changes

...ies/System.Runtime/tests/System.Globalization.Tests/CultureInfo/CultureInfoCurrentCulture.cs Outdated Show resolved Hide resolved

move test to separate method

b24acb7

build-analysis bot mentioned this pull request Jan 17, 2025

ProcessTests.LongProcessNamesAreSupported test failure #111460

Closed

matouskozak merged commit ffc68b8 into dotnet:main Jan 20, 2025
161 of 163 checks passed

github-actions bot mentioned this pull request Jan 20, 2025

[release/9.0-staging] [iOS] Retrieve device locale in full (specific) format from ObjectiveC APIs #111612

Merged

4 tasks

matouskozak mentioned this pull request Jan 16, 2025

[iOS][globalization] Investigate different Globalization behavior on iOS simulators on the CI #111501

Open

matouskozak mentioned this pull request Feb 3, 2025

iOS CultureInfo.CurrentCulture.DateTimeFormat.ShortDatePattern not getting OS settings #112099

Closed

github-actions bot locked and limited conversation to collaborators Feb 20, 2025

[iOS] Retrieve device locale in full (specific) format from ObjectiveC APIs #111032

[iOS] Retrieve device locale in full (specific) format from ObjectiveC APIs #111032

Uh oh!

Conversation

matouskozak commented Jan 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem Description

Proposed Change

Uh oh!

dotnet-policy-service bot commented Jan 2, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

matouskozak commented Jan 2, 2025

Uh oh!

azure-pipelines bot commented Jan 2, 2025

Uh oh!

vitek-karas Jan 2, 2025

Choose a reason for hiding this comment

Uh oh!

matouskozak Jan 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matouskozak commented Jan 3, 2025

Uh oh!

azure-pipelines bot commented Jan 3, 2025

Uh oh!

matouskozak commented Jan 6, 2025

Uh oh!

azure-pipelines bot commented Jan 6, 2025

Uh oh!

tarekgh commented Jan 6, 2025

Uh oh!

matouskozak commented Jan 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tarekgh commented Jan 6, 2025

Uh oh!

ivanpovazan commented Jan 7, 2025

Uh oh!

tarekgh commented Jan 7, 2025

Uh oh!

matouskozak commented Jan 13, 2025

Uh oh!

matouskozak commented Jan 16, 2025

Uh oh!

azure-pipelines bot commented Jan 16, 2025

Uh oh!

matouskozak commented Jan 16, 2025

Uh oh!

azure-pipelines bot commented Jan 16, 2025

Uh oh!

matouskozak commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ivanpovazan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matouskozak commented Jan 17, 2025

Uh oh!

azure-pipelines bot commented Jan 17, 2025

Uh oh!

Uh oh!

matouskozak commented Jan 20, 2025

Uh oh!

github-actions bot commented Jan 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matouskozak commented Jan 2, 2025 •

edited

Loading

matouskozak Jan 3, 2025 •

edited

Loading

matouskozak commented Jan 6, 2025 •

edited

Loading

matouskozak commented Jan 17, 2025 •

edited

Loading