Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@GaryQian
Copy link
Contributor

iOS's preferredLanguages may not always include the region/country code. To ensure the default locale is always complete, we use the [NSLocale currentLocale] only for the default locale which does not strip countryCode.

Also, we are able to handle language-only locales, so it should be safe to not enforce countryCode being non-null.

@tvolkert
Copy link
Contributor

/cc @cbracken @dnfield

// Use a different API for first/default locale as preferredLanguages
// may strip the region/country code from the locales.
currentLocale = [NSLocale currentLocale];
first = false;
Copy link
Contributor

@tvolkert tvolkert Nov 28, 2018

Choose a reason for hiding this comment

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

Couldn't this end up skipping a preferred locale? Take the following list:

[NSLocale preferredLanguages] => ["ru_US", "en"]
[NSLocale currentLocale].localeIdentifier => "en_US"

In this case, we'll end up setting the locales to ["en_US", "en"] and skip "ru_US" altogether.

It seems like we should always add [NSLocale currentLocale] to the head of the list, then walk [NSLocale preferredLanguages], adding them to the list if they're not already covered by the current locale.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I can tell from the iOS docs, preferredLanguages is an ordered list, and currentLocale should be the first locale in it. (https://developer.apple.com/documentation/foundation/nslocale/1415614-preferredlanguages)

This implementation prevents cases where the same locale is represented twice in the list.

Choose a reason for hiding this comment

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

The first item isn't always necessarily the same as currentLocale. I believe currentLocale makes a best determination as to what language the app should use based on system preferred languages and languages declared in the app bundle.

Also iOS allows setting preferred languages which the system itself does not support, like Filipino. So the phone system language will be something else like English, but if your app declares it supports Filipino then currentLocale does indeed return Filipino.

Since our app does support Filipino, with phone language English and Filipino as top preferred language we get:
currentLocale fil_US
preferredLanguages ("fil-US", "en-US", "pl-US", "ru-US", "es-US")

But if I then add Armenian (hy) as the top preferred language, which neither the system UI nor our app support, then we get:
currentLocale fil_US
preferredLanguages ("hy-US", "fil-US", "en-US", "pl-US", "ru-US", "es-US")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh thanks for that clarification. In that case, prepending currentLocale should be consistent with what we are trying to achieve

@GaryQian GaryQian requested a review from cbracken November 28, 2018 20:20

// Add any secondary locales/languages to the list.
for (NSString* localeID in preferredLocales) {
NSLocale* currentLocale = [[NSLocale alloc] initWithLocaleIdentifier:localeID];
Copy link
Member

Choose a reason for hiding this comment

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

NSLocale* currentLocale = [[[NSLocale alloc] initWithLocaleIdentifier:localeID] autorelease];

Copy link
Member

Choose a reason for hiding this comment

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

(or call [currentLocale release] right before the continue)


- (void)onLocaleUpdated:(NSNotification*)notification {
NSArray<NSString*>* preferredLocales = [NSLocale preferredLanguages];
NSMutableArray<NSString*>* data = [NSMutableArray new];
Copy link
Contributor

Choose a reason for hiding this comment

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

So long as you're here, this should be NSMutableArray<NSString*>* data = [[NSMutableArray new] autorelease];. I have another PR that fixes it, but might as well get it now.

@GaryQian GaryQian merged commit 72c7a75 into flutter:master Nov 28, 2018
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 28, 2018
…nsure countryCode exists. Allow language-only locales. (flutter/engine#6995)
GaryQian added a commit to GaryQian/engine that referenced this pull request Dec 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants