Bug 152715 – TextBreakIterator uses an internal implementation detail of NSLocale

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

TextBreakIterator uses an internal implementation detail of NSLocale

[Myles C. Maxfield]

Created attachment 268241 [details]
Patch

[Myles C. Maxfield]

Created attachment 268242 [details]
Patch

[Myles C. Maxfield]

<rdar://problem/23775121>

[WebKit Commit Bot]

Comment on attachment 268242 [details]
Patch

Clearing flags on attachment: 268242

Committed r194566: <http://trac.webkit.org/changeset/194566>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 268242 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=268242&action=review

> Source/WebCore/platform/text/mac/TextBreakIteratorInternalICUMac.mm:41
> +    NSArray *languagesArray = [NSLocale preferredLanguages];

To be consistent with the usage in the rest of this file it would be better to call CFLocaleCopyPreferredLanguages rather than [NSLocale preferredLanguages].

There is a change in behavior here: the NSLocale/CFLocale functions create a new array with the result of CFLocaleCreateCanonicalLanguageIdentifierFromString on each item in the array. That means that:

1) The code in this function that checks if the item from the array is an NSString is no longer needed since the NSLocale/CFLocale function guarantees it will return an array of strings, with no objects of other types.
2) The code below in two different functions that calls canonicalLanguageIdentifier on the result of this function is no longer needed; the result of this function is already guaranteed to be a canonical language identifier. I suggest moving the call to CFLocaleCreateCanonicalLanguageIdentifierFromString into the textBreakLocalePreference function and removing the canonicalLanguageIdentifier function entirely.

[Myles C. Maxfield]

Comment on attachment 268242 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=268242&action=review

>> Source/WebCore/platform/text/mac/TextBreakIteratorInternalICUMac.mm:41
>> +    NSArray *languagesArray = [NSLocale preferredLanguages];
> 
> To be consistent with the usage in the rest of this file it would be better to call CFLocaleCopyPreferredLanguages rather than [NSLocale preferredLanguages].
> 
> There is a change in behavior here: the NSLocale/CFLocale functions create a new array with the result of CFLocaleCreateCanonicalLanguageIdentifierFromString on each item in the array. That means that:
> 
> 1) The code in this function that checks if the item from the array is an NSString is no longer needed since the NSLocale/CFLocale function guarantees it will return an array of strings, with no objects of other types.
> 2) The code below in two different functions that calls canonicalLanguageIdentifier on the result of this function is no longer needed; the result of this function is already guaranteed to be a canonical language identifier. I suggest moving the call to CFLocaleCreateCanonicalLanguageIdentifierFromString into the textBreakLocalePreference function and removing the canonicalLanguageIdentifier function entirely.

http://trac.webkit.org/changeset/195001

[Myles C. Maxfield]

Comment on attachment 268242 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=268242&action=review

>>> Source/WebCore/platform/text/mac/TextBreakIteratorInternalICUMac.mm:41
>>> +    NSArray *languagesArray = [NSLocale preferredLanguages];
>> 
>> To be consistent with the usage in the rest of this file it would be better to call CFLocaleCopyPreferredLanguages rather than [NSLocale preferredLanguages].
>> 
>> There is a change in behavior here: the NSLocale/CFLocale functions create a new array with the result of CFLocaleCreateCanonicalLanguageIdentifierFromString on each item in the array. That means that:
>> 
>> 1) The code in this function that checks if the item from the array is an NSString is no longer needed since the NSLocale/CFLocale function guarantees it will return an array of strings, with no objects of other types.
>> 2) The code below in two different functions that calls canonicalLanguageIdentifier on the result of this function is no longer needed; the result of this function is already guaranteed to be a canonical language identifier. I suggest moving the call to CFLocaleCreateCanonicalLanguageIdentifierFromString into the textBreakLocalePreference function and removing the canonicalLanguageIdentifier function entirely.
> 
> http://trac.webkit.org/changeset/195001

http://trac.webkit.org/changeset/195006