[rfc][icedtea-web] Move Translation Responsibility from JNLPRuntime to Translator

[Jiri Vanek]

On 09/11/2014 02:15 AM, Andrew Azores wrote:
> On 09/10/2014 02:31 PM, Jie Kang wrote:
>>
>> Hello,
>>
>>
>> Good points. I have made Translator an enum with a single instance where the public static String
>> R still remains and added basic unit tests for Translator.
>>
>>
>
> Overall I definitely like the idea behind this patch. Just a few minor implementation details I have
> some questions about.
>
>>      public static String R(String message) {
>> -        return JNLPRuntime.getMessage(message);
>> +        return getInstance().getMessage(message);
>>      }
>
> Can this be defined in terms of Translator.R(String, Object...) instead? Not a big deal but it seems
> natural to me.
>
>> +    public static void loadResourceBundle(ResourceBundle bundle) {
>> +        getInstance().resources = bundle;
>> +    }
>
> Why is this still static? I think this makes sense as a method belonging to the singleton instance,
> and I only see one call site (TranslatorTest) that would need to be refactored. I would also rather
> see a (package-private?) setter method rather than direct field assignment here, but that's not a
> huge deal.

  good catch

>> +        } catch (Exception ex) {
>> +            throw new IllegalStateException("Missing resource bundle in
>> netx.jar:net/sourceforge/jnlp/resource/Messages.properties");
>> +        }
>
> Is this path correct even for non-default (English) locales? Eg for Czech shouldn't it be attempting

Afaik the patch is correct. The JVM is handling the locales rename.

Anyway Jies responsibility to try vith various de_DE or de_DE..UTF8 or DE... and similar...

> (and failing) to load net/sourceforge/jnlp/resource/Messages_cs.properties? I'm not sure how easy it
> is to get the name of the ResourceBundle that the runtime would attempt to load however, it may not
> be worth the effort - this is a pretty close approximation anyway even for other locales I suppose.
>
> Thanks,