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

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

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jan 20, 2025

Backport of #111032 to release/9.0-staging

/cc @matouskozak

This PR reverts the previous change to preserve the full (specific = language-region) culture locale format.

Customer Impact

  • Customer reported
  • Found internally

This issue is impacting iOS scenarios which rely on device locale region, such as, displaying local currencies or other regional information. Before this change, only the language was retrieved to set CultureInfo.CurrentCulture and the region was omitted.

Some of the issues reported by customers:

(Added after merge):

Regression

  • Yes
  • No

The regression was introduced in #104071 in .NET 9.

Testing

The previous regression change was intentional. This PR adds a test case for asserting that the default current culture is always in specific format. However, we encountered unexpected behavior on iossimulators running on the CI lab machine thus we disabled the new test case until #111501 gets resolved. We verified locally that the behavior is correct.

Risk

Medium

This change might require customers to change their app logic if they made changes to work around this issue introduced in .NET 9. Note, the behavior is correct in .NET 8.

Additionally, we encountered that for some .NET BCL (e.g., SqlString), the default culture set on device might cause crashes in the libraries code due to #111395. However, these crashes are possible to trigger in .NET 8 iOS apps as well as in apps on other platforms if the user's device locale is not part of the https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Globalization/IcuLocaleData.cs.

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

Package authoring no longer needed in .NET 9

IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.

@matouskozak
Copy link
Member

/azp run runtime-extra-platforms

Copy link
Contributor

Tagging subscribers to 'os-ios': @vitek-karas, @kotlarmilos, @ivanpovazan, @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@thomasbach-dk
Copy link

When can we expect an official release of .NET 9 which includes this bugfix?

@filipnavara
Copy link
Member

FWIW I have seen macOS and iOS machines returning non-sense cultures such as en-CZ when the machine is configured with English language and Czech region. Are we sure that none of these code paths return such values?

@filipnavara
Copy link
Member

filipnavara commented Jan 20, 2025

#include <stdio.h>
#import <Foundation/Foundation.h>

int main()
{
    printf("%s\n", [[NSLocale.preferredLanguages objectAtIndex:0] UTF8String]);
    printf("%s\n", [[NSLocale currentLocale].languageCode UTF8String]);
    printf("%s\n", [[NSLocale currentLocale].countryCode UTF8String]);
    printf("%s\n", [[NSLocale currentLocale].localeIdentifier UTF8String]);
    return 0;
}

prints this on my machine:

en-US
en
CZ
en_US@rg=czzzzz

That means the first code path is probably safe to change but the later one would now result in en-CZ locale. Also, countryCode is deprecated.

@matouskozak
Copy link
Member

FWIW I have seen macOS and iOS machines returning non-sense cultures such as en-CZ when the machine is configured with English language and Czech region. Are we sure that none of these code paths return such values?

Are you testing this on macOS or iOS device? I have seen different behavior on these two.

In my testing on iOS devices:
Both NSLocale.currentLocale and NSLocale.preferredLanguages return en-CZ. So, this change would allow that the culture is set to en-CZ if the device is set for English language and Czech region.

@filipnavara
Copy link
Member

filipnavara commented Jan 20, 2025

Are you testing this on macOS or iOS device? I have seen different behavior on these two.

macOS 15.0.

Also, [NSLocale currentLocale].languageIdentifier (iOS 17+, macOS 14+) returns en-US.

@filipnavara
Copy link
Member

Both NSLocale.currentLocale and NSLocale.preferredLanguages return en-CZ. So, this change would allow that the culture is set to en-CZ if the device is set for English language and Czech region.

That's not a valid locale AFAICT.

@matouskozak
Copy link
Member

Both NSLocale.currentLocale and NSLocale.preferredLanguages return en-CZ. So, this change would allow that the culture is set to en-CZ if the device is set for English language and Czech region.

That's not a valid locale AFAICT.

I'm not sure what .NET considers as a valid locale in this case. Based on my understanding, .NET allows creating custom cultures thus we can rely on what Objective-C API reports without custom handling/fallbacks from our side as long as the returned culture is not non-sense.

In the original PR #111032, I implemented a fallback to neutral culture if the retrieved culture was custom (as classified by .NET), but we agreed on using the retrieved culture directly. I verified locally that for en-CZ, the CZ region information looks to be picked up correctly, and for example:

CurrencyEnglishName: Czech Koruna, 
ISOCurrencySymbol: CZK, 
CurrencyNativeName: Czech Koruna, 
CurrencySymbol: CZK, 
NumberFormat.CurrencySymbol: CZK

which is what I would expect.

@filipnavara
Copy link
Member

After a chat with @matouskozak I agree that the change is fine. The weird Apple cultures like en-CZ / de-CZ and so on are already exposed on macOS on .NET 8/9 and iOS on .NET 8 (w/o hybrid globalization). This is just aligning iOS on .NET 9 to follow the same pattern and it's orthogonal to any existing weirdnesses around those cultures.

@tarekgh tarekgh added this to the 10.0.0 milestone Jan 20, 2025
@matouskozak matouskozak added the Servicing-consider Issue for next servicing release review label Jan 21, 2025
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

lgtm. we will take for consideration in 9.0.x

@rbhanda rbhanda modified the milestones: 10.0.0, 9.0.3 Jan 28, 2025
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 28, 2025
@matouskozak
Copy link
Member

/ba -g timeouts on coreclr windows x64 debug not related, the extra-platform failures are known and not-related

@matouskozak
Copy link
Member

/ba-g timeouts on coreclr windows x64 debug not related

@matouskozak matouskozak merged commit 6061b2c into release/9.0-staging Jan 29, 2025
198 of 213 checks passed
@jkotas jkotas deleted the backport/pr-111032-to-release/9.0-staging branch February 2, 2025 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Globalization os-ios Apple iOS Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants