<i18n dev> [8]Request for review: 8001205) Calendar.getDisplayName(...): Returns null when provider is SPI but there is no SPI implementation

[Masayoshi Okutsu]

Looks good to me.

Masayoshi

On 11/8/2012 4:23 AM, Naoto Sato wrote:
> Thanks. I incorporated your comments and generated a new webrev:
>
> http://cr.openjdk.java.net/~naoto/8001205.8001562/webrev.01/
>
> Naoto
>
> On 11/7/12 1:24 AM, Masayoshi Okutsu wrote:
>> Hi,
>>
>> Here are my comments.
>>
>> - LocaleServiceProviderPool.java:
>>
>> - * provider, including the JRE.
>> + * provider, including the JRE's FormatData locales.
>> *
>> * @return an array of the available locales
>> */
>> public Locale[] getAvailableLocales() {
>> - Set<Locale> locList = getAvailableLocaleList();
>> + Set<Locale> locList = new HashSet<>();
>> + locList.addAll(getAvailableLocaleSet());
>> + // Make sure it all contains JRE's FormatData locales for 
>> compatibility.
>>
>> It's not accurate to say FormatData locales. The FormatData locales list
>> is no longer a superset.
>>
>> - <> should be used where it's applicable.
>>
>> - test/java/util/Locale/Bug8001562.java: line 37 is too long...
>>
>>
>> Otherwise, the fixes look good to me.
>>
>> Masayoshi
>>
>>
>> On 11/7/2012 7:17 AM, Naoto Sato wrote:
>>> Eventually, I merged the fix for 8001562 with 8001205, and here is the
>>> combined webrev:
>>>
>>> http://cr.openjdk.java.net/~naoto/8001205.8001562/webrev.00/
>>>
>>> This version includes the following:
>>>
>>> - Previous fix for 8001205
>>> - Fix for 8001562, where available locales for JRE's FormatData
>>> locales are now included for getAvailableLocales() return values for
>>> all locale sensitive services (e.g. DateFormat)
>>> - Removed the code for getting rid of Locale.ROOT from the available
>>> locales. Returning it will not affect the compatibility with JDK7
>>> (it's a superset).
>>> - Method name change (getAvailableLocaleList() ->
>>> getAvailableLocaleSet()).
>>>
>>> Please review.
>>>
>>> Naoto
>>>
>>> On 11/2/12 12:02 PM, Naoto Sato wrote:
>>>> Will fix 8001562 too, with a separate changeset.
>>>>
>>>> Naoto
>>>>
>>>> On 11/2/12 1:08 AM, Masayoshi Okutsu wrote:
>>>>> Do you plan to fix this one with 8001562? (If you fix only this 
>>>>> one, it
>>>>> may be seen as a regression.)
>>>>>
>>>>> Masayoshi
>>>>>
>>>>> On 10/31/2012 11:55 AM, Naoto Sato wrote:
>>>>>> With this fix, no. But another bug was filed regarding the locale
>>>>>> sensitive service APIs' (e.g., Collator) getAvailableLocales() 
>>>>>> return
>>>>>> value (8001562). I will fix it to keep the returned locale array
>>>>>> compatible with JDK7.
>>>>>>
>>>>>> Naoto
>>>>>>
>>>>>> On 10/30/12 7:43 PM, Masayoshi Okutsu wrote:
>>>>>>> Question. Does the full set of available locales of JRE still 
>>>>>>> become
>>>>>>> available with this fix? For example, when 
>>>>>>> java.locale.providers=SPI,
>>>>>>> all JRE locales are included in the getAvailableLocales (API)? The
>>>>>>> fallback adapter is an interesting idea, but it's a bit 
>>>>>>> confusing...
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Masayoshi
>>>>>>>
>>>>>>> On 10/31/2012 6:44 AM, Naoto Sato wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> Please review the fix for the following bug:
>>>>>>>>
>>>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8001205
>>>>>>>>
>>>>>>>> The fix is simply to supply the fallback locale provider 
>>>>>>>> adapter for
>>>>>>>> the root locale, when there is no explicit "JRE" adapter is
>>>>>>>> specified
>>>>>>>> in the locale provider preference list.
>>>>>>>>
>>>>>>>> The proposed fix is located at:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~naoto/8001205/webrev.00/
>>>>>>>>
>>>>>>>> Naoto
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>