<AWT Dev> AWT Dev> [9] Review request for 8163979: [macosx] Chinese text shows as Latin w/ openVanilla input method

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

<AWT Dev> AWT Dev> [9] Review request for 8163979: [macosx] Chinese text shows as Latin w/ openVanilla input method

Dmitry Markov
Hello,

Could you review a fix for jdk9, please?

        bug: https://bugs.openjdk.java.net/browse/JDK-8163979
        webrev: http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/

Problem description:
If the current keyboard layout is set to non-default value, (e.g. ABC, ABC Extended, etc) during Java launch, the input method functionality is not initialised correctly and may not work for some input methods, (e.g. OpenVanilla input method). The initialisation problems are caused by nativeGetAvailableLocales() function from CInputMethod.m. The function tries to obtain the list of available input method locales using platform API. If the retrieval is failed for some reason, nativeGetAvailableLocales() returns empty list or null.

Fix:
The function nativeGetAvailableLocales() should return the list contained the current input method locale, if it cannot retrieve the list of available locales from the platform.


Thanks,
Dmitry
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> AWT Dev> [9] Review request for 8163979: [macosx] Chinese text shows as Latin w/ openVanilla input method

Dmitry Markov
I am sorry, I sent the incorrect version of the fix, (i.e. http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/). It releases currenLocale two times.
The correct version is located at http://cr.openjdk.java.net/~dmarkov/8163979/webrev.01/
Could you review the updated fix, please?

Thanks,
Dmitry

> On 13 Feb 2017, at 16:58, Dmitry Markov <[hidden email]> wrote:
>
> Hello,
>
> Could you review a fix for jdk9, please?
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8163979
> webrev: http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/
>
> Problem description:
> If the current keyboard layout is set to non-default value, (e.g. ABC, ABC Extended, etc) during Java launch, the input method functionality is not initialised correctly and may not work for some input methods, (e.g. OpenVanilla input method). The initialisation problems are caused by nativeGetAvailableLocales() function from CInputMethod.m. The function tries to obtain the list of available input method locales using platform API. If the retrieval is failed for some reason, nativeGetAvailableLocales() returns empty list or null.
>
> Fix:
> The function nativeGetAvailableLocales() should return the list contained the current input method locale, if it cannot retrieve the list of available locales from the platform.
>
>
> Thanks,
> Dmitry

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> AWT Dev> [9] Review request for 8163979: [macosx] Chinese text shows as Latin w/ openVanilla input method

Sergey Bylokhov
Hi, Dmitry.
Probably we can reuse Java_sun_lwawt_macosx_CInputMethod_getNativeLocale?

And change the java code in the CInputMethodDescriptor to something like:

    static Object[] getAvailableLocalesInternal() {
        List<?> workList = nativeGetAvailableLocales();
        Locale nativeLocale = CIM.getNativeLocale()

        if (workList == null && nativeLocale == null)) {
                return new Object[] {
           Locale.getDefault()
        };
        }
        if(workList != null && !workList.isEmpty() && nativeLocale == null) {
            return workList.toArray();
        }
        if((workList == null || workList.isEmpty()) && nativeLocale != null) {
                return new Object[] {
           nativeLocale;
        };
        }
        if (!workList.contains(nativeLocale)){
                workList.add(nativeLocale);
        }
        return workList.toArray();
    }


>
> I am sorry, I sent the incorrect version of the fix, (i.e. http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/). It releases currenLocale two times.
> The correct version is located at http://cr.openjdk.java.net/~dmarkov/8163979/webrev.01/
> Could you review the updated fix, please?
>
> Thanks,
> Dmitry
>> On 13 Feb 2017, at 16:58, Dmitry Markov <[hidden email]> wrote:
>>
>> Hello,
>>
>> Could you review a fix for jdk9, please?
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8163979
>> webrev: http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/
>>
>> Problem description:
>> If the current keyboard layout is set to non-default value, (e.g. ABC, ABC Extended, etc) during Java launch, the input method functionality is not initialised correctly and may not work for some input methods, (e.g. OpenVanilla input method). The initialisation problems are caused by nativeGetAvailableLocales() function from CInputMethod.m. The function tries to obtain the list of available input method locales using platform API. If the retrieval is failed for some reason, nativeGetAvailableLocales() returns empty list or null.
>>
>> Fix:
>> The function nativeGetAvailableLocales() should return the list contained the current input method locale, if it cannot retrieve the list of available locales from the platform.
>>
>>
>> Thanks,
>> Dmitry
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> AWT Dev> [9] Review request for 8163979: [macosx] Chinese text shows as Latin w/ openVanilla input method

Dmitry Markov
Hi Sergey,

Thank you for the review. You suggested a really good solution for the issue.
I have updated the fix based on your recommendation. The new version is located at http://cr.openjdk.java.net/~dmarkov/8163979/webrev.02/

Thanks,
Dmitry

> On 13 Feb 2017, at 20:22, Sergey Bylokhov <[hidden email]> wrote:
>
> Hi, Dmitry.
> Probably we can reuse Java_sun_lwawt_macosx_CInputMethod_getNativeLocale?
>
> And change the java code in the CInputMethodDescriptor to something like:
>
>    static Object[] getAvailableLocalesInternal() {
>        List<?> workList = nativeGetAvailableLocales();
> Locale nativeLocale = CIM.getNativeLocale()
>
>        if (workList == null && nativeLocale == null)) {
>        return new Object[] {
>            Locale.getDefault()
>         };
> }
> if(workList != null && !workList.isEmpty() && nativeLocale == null) {
>            return workList.toArray();
>        }
> if((workList == null || workList.isEmpty()) && nativeLocale != null) {
>        return new Object[] {
>            nativeLocale;
>         };
>        }
> if (!workList.contains(nativeLocale)){
> workList.add(nativeLocale);
> }
>        return workList.toArray();
>    }
>
>
>>
>> I am sorry, I sent the incorrect version of the fix, (i.e. http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/). It releases currenLocale two times.
>> The correct version is located at http://cr.openjdk.java.net/~dmarkov/8163979/webrev.01/
>> Could you review the updated fix, please?
>>
>> Thanks,
>> Dmitry
>>> On 13 Feb 2017, at 16:58, Dmitry Markov <[hidden email]> wrote:
>>>
>>> Hello,
>>>
>>> Could you review a fix for jdk9, please?
>>>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8163979
>>> webrev: http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/
>>>
>>> Problem description:
>>> If the current keyboard layout is set to non-default value, (e.g. ABC, ABC Extended, etc) during Java launch, the input method functionality is not initialised correctly and may not work for some input methods, (e.g. OpenVanilla input method). The initialisation problems are caused by nativeGetAvailableLocales() function from CInputMethod.m. The function tries to obtain the list of available input method locales using platform API. If the retrieval is failed for some reason, nativeGetAvailableLocales() returns empty list or null.
>>>
>>> Fix:
>>> The function nativeGetAvailableLocales() should return the list contained the current input method locale, if it cannot retrieve the list of available locales from the platform.
>>>
>>>
>>> Thanks,
>>> Dmitry
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> AWT Dev> [9] Review request for 8163979: [macosx] Chinese text shows as Latin w/ openVanilla input method

Phil Race
 63         if (workList == null || (workList != null && workList.isEmpty())) {


I think we can safely write just :

if (workList == null || workList.isEmpty()) {

Other than that it seems fine.

-phil.

On 02/13/2017 11:43 AM, Dmitry Markov wrote:
Hi Sergey,

Thank you for the review. You suggested a really good solution for the issue.
I have updated the fix based on your recommendation. The new version is located at http://cr.openjdk.java.net/~dmarkov/8163979/webrev.02/

Thanks,
Dmitry
On 13 Feb 2017, at 20:22, Sergey Bylokhov [hidden email] wrote:

Hi, Dmitry.
Probably we can reuse Java_sun_lwawt_macosx_CInputMethod_getNativeLocale?

And change the java code in the CInputMethodDescriptor to something like:

   static Object[] getAvailableLocalesInternal() {
       List<?> workList = nativeGetAvailableLocales();
	Locale nativeLocale = CIM.getNativeLocale()

       if (workList == null && nativeLocale == null)) {
	        return new Object[] {
       	    Locale.getDefault()
       	};
	}
	if(workList != null && !workList.isEmpty() && nativeLocale == null) {
           return workList.toArray();
       }
	if((workList == null || workList.isEmpty()) && nativeLocale != null) {
	        return new Object[] {
       	    nativeLocale;
       	};
       }
	if (!workList.contains(nativeLocale)){
		workList.add(nativeLocale);
	}
       return workList.toArray();
   }


I am sorry, I sent the incorrect version of the fix, (i.e. http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/). It releases currenLocale two times.
The correct version is located at http://cr.openjdk.java.net/~dmarkov/8163979/webrev.01/
Could you review the updated fix, please?

Thanks,
Dmitry
On 13 Feb 2017, at 16:58, Dmitry Markov [hidden email] wrote:

Hello,

Could you review a fix for jdk9, please?

	bug: https://bugs.openjdk.java.net/browse/JDK-8163979
	webrev: http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/

Problem description:
If the current keyboard layout is set to non-default value, (e.g. ABC, ABC Extended, etc) during Java launch, the input method functionality is not initialised correctly and may not work for some input methods, (e.g. OpenVanilla input method). The initialisation problems are caused by nativeGetAvailableLocales() function from CInputMethod.m. The function tries to obtain the list of available input method locales using platform API. If the retrieval is failed for some reason, nativeGetAvailableLocales() returns empty list or null.

Fix:
The function nativeGetAvailableLocales() should return the list contained the current input method locale, if it cannot retrieve the list of available locales from the platform.


Thanks,
Dmitry

        

      

    

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> AWT Dev> [9] Review request for 8163979: [macosx] Chinese text shows as Latin w/ openVanilla input method

Dmitry Markov
Thank you, Phil. I have updated the fix based on your suggestion. Please find the new version here: http://cr.openjdk.java.net/~dmarkov/8163979/webrev.03/

Thanks,
Dmitry
On 14 Feb 2017, at 01:54, Phil Race <[hidden email]> wrote:

 63         if (workList == null || (workList != null && workList.isEmpty())) {


I think we can safely write just :

if (workList == null || workList.isEmpty()) {

Other than that it seems fine.

-phil.

On 02/13/2017 11:43 AM, Dmitry Markov wrote:
Hi Sergey,

Thank you for the review. You suggested a really good solution for the issue.
I have updated the fix based on your recommendation. The new version is located at http://cr.openjdk.java.net/~dmarkov/8163979/webrev.02/

Thanks,
Dmitry
On 13 Feb 2017, at 20:22, Sergey Bylokhov [hidden email] wrote:

Hi, Dmitry.
Probably we can reuse Java_sun_lwawt_macosx_CInputMethod_getNativeLocale?

And change the java code in the CInputMethodDescriptor to something like:

   static Object[] getAvailableLocalesInternal() {
       List<?> workList = nativeGetAvailableLocales();
	Locale nativeLocale = CIM.getNativeLocale()

       if (workList == null && nativeLocale == null)) {
	        return new Object[] {
       	    Locale.getDefault()
       	};
	}
	if(workList != null && !workList.isEmpty() && nativeLocale == null) {
           return workList.toArray();
       }
	if((workList == null || workList.isEmpty()) && nativeLocale != null) {
	        return new Object[] {
       	    nativeLocale;
       	};
       }
	if (!workList.contains(nativeLocale)){
		workList.add(nativeLocale);
	}
       return workList.toArray();
   }


I am sorry, I sent the incorrect version of the fix, (i.e. http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/). It releases currenLocale two times.
The correct version is located at http://cr.openjdk.java.net/~dmarkov/8163979/webrev.01/
Could you review the updated fix, please?

Thanks,
Dmitry
On 13 Feb 2017, at 16:58, Dmitry Markov [hidden email] wrote:

Hello,

Could you review a fix for jdk9, please?

	bug: https://bugs.openjdk.java.net/browse/JDK-8163979
	webrev: http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/

Problem description:
If the current keyboard layout is set to non-default value, (e.g. ABC, ABC Extended, etc) during Java launch, the input method functionality is not initialised correctly and may not work for some input methods, (e.g. OpenVanilla input method). The initialisation problems are caused by nativeGetAvailableLocales() function from CInputMethod.m. The function tries to obtain the list of available input method locales using platform API. If the retrieval is failed for some reason, nativeGetAvailableLocales() returns empty list or null.

Fix:
The function nativeGetAvailableLocales() should return the list contained the current input method locale, if it cannot retrieve the list of available locales from the platform.


Thanks,
Dmitry

        

      

    


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: <AWT Dev> AWT Dev> [9] Review request for 8163979: [macosx] Chinese text shows as Latin w/ openVanilla input method

Sergey Bylokhov
Looks fine.


Thank you, Phil. I have updated the fix based on your suggestion. Please find the new version here: http://cr.openjdk.java.net/~dmarkov/8163979/webrev.03/

Thanks,
Dmitry
On 14 Feb 2017, at 01:54, Phil Race <[hidden email]> wrote:

 63         if (workList == null || (workList != null && workList.isEmpty())) {


I think we can safely write just :

if (workList == null || workList.isEmpty()) {

Other than that it seems fine.

-phil.

On 02/13/2017 11:43 AM, Dmitry Markov wrote:
Hi Sergey,

Thank you for the review. You suggested a really good solution for the issue.
I have updated the fix based on your recommendation. The new version is located at http://cr.openjdk.java.net/~dmarkov/8163979/webrev.02/

Thanks,
Dmitry
On 13 Feb 2017, at 20:22, Sergey Bylokhov [hidden email] wrote:

Hi, Dmitry.
Probably we can reuse Java_sun_lwawt_macosx_CInputMethod_getNativeLocale?

And change the java code in the CInputMethodDescriptor to something like:

   static Object[] getAvailableLocalesInternal() {
       List<?> workList = nativeGetAvailableLocales();
	Locale nativeLocale = CIM.getNativeLocale()

       if (workList == null && nativeLocale == null)) {
	        return new Object[] {
       	    Locale.getDefault()
       	};
	}
	if(workList != null && !workList.isEmpty() && nativeLocale == null) {
           return workList.toArray();
       }
	if((workList == null || workList.isEmpty()) && nativeLocale != null) {
	        return new Object[] {
       	    nativeLocale;
       	};
       }
	if (!workList.contains(nativeLocale)){
		workList.add(nativeLocale);
	}
       return workList.toArray();
   }


I am sorry, I sent the incorrect version of the fix, (i.e. http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/). It releases currenLocale two times.
The correct version is located at http://cr.openjdk.java.net/~dmarkov/8163979/webrev.01/
Could you review the updated fix, please?

Thanks,
Dmitry
On 13 Feb 2017, at 16:58, Dmitry Markov [hidden email] wrote:

Hello,

Could you review a fix for jdk9, please?

	bug: https://bugs.openjdk.java.net/browse/JDK-8163979
	webrev: http://cr.openjdk.java.net/~dmarkov/8163979/webrev.00/

Problem description:
If the current keyboard layout is set to non-default value, (e.g. ABC, ABC Extended, etc) during Java launch, the input method functionality is not initialised correctly and may not work for some input methods, (e.g. OpenVanilla input method). The initialisation problems are caused by nativeGetAvailableLocales() function from CInputMethod.m. The function tries to obtain the list of available input method locales using platform API. If the retrieval is failed for some reason, nativeGetAvailableLocales() returns empty list or null.

Fix:
The function nativeGetAvailableLocales() should return the list contained the current input method locale, if it cannot retrieve the list of available locales from the platform.


Thanks,
Dmitry

        

      

    



Loading...