Re: RFR: 8213232 Unix/X11 setCompositionEnableNative issue

Sun, 28 Apr 2019 20:47:34 -0700 

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.

Reply via email to