<i18n dev> <AWT Dev> RFR: 8213232 Unix/X11 setCompositionEnableNative issue

Mon Apr 29 03:47:35 UTC 2019

Hello.

I built the new fixed code by GCC 8.2.1.
It worked fine.

Thanks,
Ichiroh Takiguchi

On 2019-04-26 23:04, Ichiroh Takiguchi wrote:
> Hello Sergey.
> 
> See "Preedit State Callbacks" section on Xlib manual [1]
> Return code for PreeditStartCallback should be int instead of void:
> int PreeditStartCallback(XIC ic, XPointer client_data, XPointer 
> call_data);
> 
> I'm very sorry, I could not create build environment for GCC 8.2 on 
> today.
> 
> So I just tested GCC 8.2's compilation error with my small testcase.
> 
> Could you review the fix ?
> 
> Bug:    https://bugs.openjdk.java.net/browse/JDK-8213232
> Change: https://cr.openjdk.java.net/~itakiguchi/8213232/webrev.02/
> 
> I modified the code, like this:
> -    (XIMProc)PreeditStartCallback,
> +    (XIMProc)(void *)&PreeditStartCallback,
> 
> [1] 
> https://www.x.org/releases/X11R7.6/doc/libX11/specs/libX11/libX11.html
> 
> Thanks,
> Ichiroh Takiguchi
> 
> On 2019-04-26 08:14, Sergey Bylokhov wrote:
>> Hi, Ichiroh.
>> 
>> I got this build error when I tried to test this patch:
>> 
>> open/src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c:83:5:
>> error: cast between incompatible function types from 'int (*)(struct
>> >..._XIC *, char *, char *)' to 'void (*)(struct _XIM *, char *, char *)' [-Werror=cast-function-type]
>>>      (XIMProc)PreeditStartCallback,
>>>      ^
>>> cc1: all warnings being treated as errors
>> 
>> Please take a look(linux_x64/gcc8.2)
>> 
>> 
>> On 18/04/2019 04:09, Ichiroh Takiguchi wrote:
>>> Hello Sergey.
>>> Sorry for bad response.
>>> 
>>> About "defined(MACOSX)"
>>> I followed Line# 87 usage, because the codes handle IME status 
>>> windows.
>>> 
>>> <src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c>
>>>    82 static XIMProc callback_funcs[NCALLBACKS] = {
>>>    83     (XIMProc)PreeditStartCallback,
>>>    84     (XIMProc)PreeditDoneCallback,
>>>    85     (XIMProc)PreeditDrawCallback,
>>>    86     (XIMProc)PreeditCaretCallback,
>>>    87 #if defined(__linux__) || defined(MACOSX)
>>>    88     (XIMProc)StatusStartCallback,
>>>    89     (XIMProc)StatusDoneCallback,
>>>    90     (XIMProc)StatusDrawCallback,
>>>    91 #endif
>>>    92 };
>>> 
>>> Should I remove "defined(MACOSX)" from awt_InputMethod.c or just 
>>> modified code ?
>>> Please give me your suggestion.
>>> 
>>> About AIX's awt_InputMethod.c
>>> It already has fixed code Line# 2187 - 2189.
>>> 
>>> <src/java.desktop/aix/native/libawt_xawt/awt/awt_InputMethod.c>
>>>    2172 JNIEXPORT jboolean JNICALL 
>>> Java_sun_awt_X11InputMethodBase_setCompositionEnabledNative
>>>    2173   (JNIEnv *env, jobject this, jboolean enable)
>>>    2174 {
>>>    2175     X11InputMethodData *pX11IMData;
>>>    2176     char * ret = NULL;
>>>    2177     XVaNestedList pr_atrb;
>>> ...
>>>    2186
>>>>> 2187     pr_atrb = XVaCreateNestedList(0, XNPreeditState,
>>>>> 2188                   (enable ? XIMPreeditEnable 
>>>>> : XIMPreeditDisable), NULL);
>>>>> 2189     ret = XSetICValues(pX11IMData->current_ic, 
>>>>> XNPreeditAttributes, pr_atrb, NULL);
>>>    2190     XFree((void *)pr_atrb);
>>>    2191     AWT_UNLOCK();
>>> 
>>> Thanks,
>>> Ichiroh Takiguchi
>>> 
>>> On 2019-04-11 11:19, Sergey Bylokhov wrote:
>>>> Hi, Ichiroh.
>>>> 
>>>> Why the fix uses the "defined(MACOSX)" in a few places? I assume 
>>>> this
>>>> code is never executed on macOS.
>>>> Should we update the aix version as well?
>>>> .../java.desktop/aix/native/libawt_xawt/awt/awt_InputMethod.c
>>>> 
>>>> On 25/03/2019 09:28, Ichiroh Takiguchi wrote:
>>>>> Hello.
>>>>> 
>>>>> Could you review the fix and give me your suggestion, please ?
>>>>> I really appreciate your feedback.
>>>>> 
>>>>> Currently, UnsupportedOperationException happens because of invalid 
>>>>> usage
>>>>> by input method operation.
>>>>> 
>>>>> And I'd like to obtain a sponsor for this issue.
>>>>> 
>>>>> Thanks,
>>>>> Ichiroh Takiguchi
>>>>> 
>>>>> On 2019-03-11 22:00, Ichiroh Takiguchi wrote:
>>>>>> Hello.
>>>>>> 
>>>>>> Could you review the fix and give me your suggestion, please ?
>>>>>> 
>>>>>> Thanks,
>>>>>> Ichiroh Takiguchi
>>>>>> 
>>>>>> On 2019-02-26 22:08, Ichiroh Takiguchi wrote:
>>>>>>> Hello.
>>>>>>> 
>>>>>>> Could you review the fix ?
>>>>>>> 
>>>>>>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8213232
>>>>>>> Change: 
>>>>>>> https://cr.openjdk.java.net/~itakiguchi/8213232/webrev.01/
>>>>>>> 
>>>>>>> JDK-8213232 [1] has movies.
>>>>>>> Please check it.
>>>>>>> 
>>>>>>> I applied following changes on webrev.01:
>>>>>>> 1. According to "Xlib - C Language X Interface" [2]
>>>>>>>    See "Preedit State Callbacks" section:
>>>>>>>    PreeditStartCallback() should return int.
>>>>>>> 2. Add 64bit Big Endian CPU support, like s390x
>>>>>>> 3. Change copyright year
>>>>>>> 
>>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8213232
>>>>>>> [2] 
>>>>>>> https://www.x.org/releases/X11R7.6/doc/libX11/specs/libX11/libX11.html
>>>>>>> 
>>>>>>> Thanks,
>>>>>>> Ichiroh Takiguchi
>>>>>>> IBM Japan, Ltd.
>>>>>>> 
>>>>>>> On 2018-12-04 11:22, Ichiroh Takiguchi wrote:
>>>>>>>> Hello.
>>>>>>>> 
>>>>>>>> Could you review the fix ?
>>>>>>>> 
>>>>>>>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8213232
>>>>>>>> Change: 
>>>>>>>> https://cr.openjdk.java.net/~itakiguchi/8213232/webrev.00/
>>>>>>>> 
>>>>>>>> This issue is related by wrong usage of XNPreeditState on 
>>>>>>>> XSetICValues().
>>>>>>>> Because of wrong usage, UnsupportedOperationException exception
>>>>>>>> happened with Xlib bundled input method.
>>>>>>>> Test instructions is in JDK-8213232.
>>>>>>>> 
>>>>>>>> I'd like to obtain a sponsor for this issue.
>>>>>>>> 
>>>>>>>> Thanks,
>>>>>>>> Ichiroh Takiguchi
>>>>>>>> IBM Japan, Ltd.
>>>>>>>> 
>>>>>>>> On 2018-06-19 02:15, Ichiroh Takiguchi wrote:
>>>>>>>>> Hello,
>>>>>>>>> IBM would like to contribute Unix/X11 
>>>>>>>>> setCompositionEnableNative issue
>>>>>>>>> patch to OpenJDK project.
>>>>>>>>> 
>>>>>>>>> Issue:
>>>>>>>>> To  turn on or turn off IME via Java, XNPreeditState should be 
>>>>>>>>> used
>>>>>>>>> with XSetValues().
>>>>>>>>> But it should be nested via XVaCreateNestedList() since 
>>>>>>>>> XNPreeditState
>>>>>>>>> is a part of XNPreeditAttributes.
>>>>>>>>> Current code is, like:
>>>>>>>>> -    ret = XSetICValues(pX11IMData->current_ic, 
>>>>>>>>> XNPreeditState,
>>>>>>>>> -                       (enable ? 
>>>>>>>>> XIMPreeditEnable : XIMPreeditDisable), NULL);
>>>>>>>>> But it should be, like:
>>>>>>>>> +    pr_atrb = XVaCreateNestedList(0,
>>>>>>>>> +                  XNPreeditState, (enable ? 
>>>>>>>>> XIMPreeditEnable :
>>>>>>>>> XIMPreeditDisable),
>>>>>>>>> +                  NULL);
>>>>>>>>> +    ret = XSetICValues(pX11IMData->current_ic, 
>>>>>>>>> XNPreeditAttributes,
>>>>>>>>> pr_atrb, NULL);
>>>>>>>>> 
>>>>>>>>> On Linux platform, the issue can check with following test 
>>>>>>>>> program and
>>>>>>>>> kinput2 XIM server and
>>>>>>>>> Wnn8 for Linux/BSD (htt).
>>>>>>>>> ==================
>>>>>>>>> import java.awt.*;
>>>>>>>>> import java.awt.event.*;
>>>>>>>>> import javax.swing.*;
>>>>>>>>> 
>>>>>>>>> public class CompositionEnabledTest extends JFrame {
>>>>>>>>>   CompositionEnabledTest() {
>>>>>>>>>     Container c = getContentPane();
>>>>>>>>>     c.setLayout(new GridLayout(0,2));
>>>>>>>>>     JButton btn;
>>>>>>>>>     JTextField tf;
>>>>>>>>>     btn = new JButton("Undefined");
>>>>>>>>>     c.add(btn);
>>>>>>>>>     tf = new JTextField("Undefined");
>>>>>>>>>     c.add(tf);
>>>>>>>>>     btn = new JButton("Disabled");
>>>>>>>>>     btn.addFocusListener(new MyFocusListener(false));
>>>>>>>>>     c.add(btn);
>>>>>>>>>     tf = new JTextField("Disabled");
>>>>>>>>>     tf.addFocusListener(new MyFocusListener(false));
>>>>>>>>>     c.add(tf);
>>>>>>>>>     btn = new JButton("Enabled");
>>>>>>>>>     btn.addFocusListener(new MyFocusListener(true));
>>>>>>>>>     c.add(btn);
>>>>>>>>>     tf = new JTextField("Enabled");
>>>>>>>>>     tf.addFocusListener(new MyFocusListener(true));
>>>>>>>>>     c.add(tf);
>>>>>>>>>     setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
>>>>>>>>>     pack();
>>>>>>>>>     setVisible(true);
>>>>>>>>>   }
>>>>>>>>>   class MyFocusListener implements FocusListener {
>>>>>>>>>     boolean state;
>>>>>>>>>     MyFocusListener(boolean state) {
>>>>>>>>>       this.state = state;
>>>>>>>>>     }
>>>>>>>>>     public void focusGained(FocusEvent fe) {
>>>>>>>>> fe.getComponent().getInputContext().setCompositionEnabled(state);
>>>>>>>>>     }
>>>>>>>>>     public void focusLost(FocusEvent fe) { }
>>>>>>>>>   }
>>>>>>>>>   public static void main(String[] args) {
>>>>>>>>>     new CompositionEnabledTest();
>>>>>>>>>   }
>>>>>>>>> }
>>>>>>>>> ==================
>>>>>>>>> 
>>>>>>>>> Click JTextField and JButton, then XIM server status may be 
>>>>>>>>> changed.
>>>>>>>>> If XIM server does not support XNPreeditState, it does not 
>>>>>>>>> work.
>>>>>>>>> 
>>>>>>>>> I'd like contribute following 3 files:
>>>>>>>>> M src/java.desktop/share/classes/sun/awt/im/InputContext.java
>>>>>>>>> M src/java.desktop/unix/classes/sun/awt/X11InputMethod.java
>>>>>>>>> M 
>>>>>>>>> src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c
>>>>>>>>> 
>>>>>>>>> http://cr.openjdk.java.net/~aleonard/set_composition_enabled/webrev.00/
>>>>>>>>> 
>>>>>>>>> I appreciate any feedback please, and how I would go about 
>>>>>>>>> obtaining a
>>>>>>>>> sponsor and contributor?
>>>>>>>>> 
>>>>>>>>> Thanks,
>>>>>>>>> Ichiroh Takiguchi
>>>>>>>>> IBM Japan, Ltd.
>>>>> 
>>> 