-
Notifications
You must be signed in to change notification settings - Fork 6k
Use a [NSLocale currentLocale] for first locale on iOS to ensure countryCode exists. Allow language-only locales. #6995
Conversation
| // Use a different API for first/default locale as preferredLanguages | ||
| // may strip the region/country code from the locales. | ||
| currentLocale = [NSLocale currentLocale]; | ||
| first = false; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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")
There was a problem hiding this comment.
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
|
|
||
| // Add any secondary locales/languages to the list. | ||
| for (NSString* localeID in preferredLocales) { | ||
| NSLocale* currentLocale = [[NSLocale alloc] initWithLocaleIdentifier:localeID]; |
There was a problem hiding this comment.
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];
There was a problem hiding this comment.
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]; |
There was a problem hiding this comment.
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.
…nsure countryCode exists. Allow language-only locales. (flutter/engine#6995)
…untryCode exists. Allow language-only locales. (flutter#6995)
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.