Hello Christoph.

Our team tested your fixed code on Linux (RHEL7) and AIX (7.1).

resetCompositionState() was missing in src/java.desktop/aix/classes/sun/awt/X11InputMethod.java
============================================================
--- a/src/java.desktop/aix/classes/sun/awt/X11InputMethod.java Wed May 09 09:05:32 2018 +0900 +++ b/src/java.desktop/aix/classes/sun/awt/X11InputMethod.java Mon May 14 21:25:50 2018 +0900
@@ -56,6 +56,21 @@
     }

     /**
+     * Reset the composition state to the current composition state.
+     */
+    protected void resetCompositionState() {
+        if (compositionEnableSupported && haveActiveClient()) {
+            try {
+ /* Restore the composition mode to the last saved composition
+                   mode. */
+                setCompositionEnabled(savedCompositionState);
+            } catch (UnsupportedOperationException e) {
+                compositionEnableSupported = false;
+            }
+        }
+    }
+
+    /**
      * Activate input method.
      */
     public synchronized void activate() {
============================================================

Otherwise,
we could not find out any functional difference on Linux.
we could not find out any functional difference between our modified code
and your code on AIX.

By code check, we found following difference.
============================================================
--- a/src/java.desktop/aix/native/libawt_xawt/awt/awt_InputMethod.c Wed May 09 09:05:32 2018 +0900 +++ b/src/java.desktop/aix/native/libawt_xawt/awt/awt_InputMethod.c Mon May 14 21:25:50 2018 +0900
@@ -248,6 +248,10 @@
                              "flushText",
                              "()V");
         JNU_CHECK_EXCEPTION_RETURN(env, NULL);
+        if ((*env)->ExceptionOccurred(env)) {
+            (*env)->ExceptionDescribe(env);
+            (*env)->ExceptionClear(env);
+        }
         /* IMPORTANT:
The order of the following calls is critical since "imInstance" may point to the global reference itself, if "freeX11InputMethodData" is called
============================================================

Did you remove this code intentionally ?

Otherwise, I think the other changes were acceptable.

Thanks,

On 2018-05-09 00:24, Erik Joelsson wrote:
Build change looks good.

/Erik


On 2018-05-08 02:54, Langer, Christoph wrote:
Hi Eric,

thanks for that excellent suggestion. I already thought there should be some means to do that but was not aware of how that could be accomplished. I updated the webrev in place.

Thanks
Christoph

-----Original Message-----
From: Erik Joelsson [mailto:erik.joels...@oracle.com]
Sent: Freitag, 4. Mai 2018 17:45
To: Langer, Christoph <christoph.lan...@sap.com>; awt-
d...@openjdk.java.net
Cc: build-dev@openjdk.java.net; ppc-aix-port-...@openjdk.java.net
Subject: Re: RFR: 8201429: Support AIX Input Method Editor (IME) for AWT
Input Method Framework (IMF)

Hello,

It looks like what you are trying to achieve is to override
awt_InputMethod.c with an OS specific version of that file. We have a
construct for this in SetupNativeCompilation that should handle it
automatically, if you just list the source dirs in priority order. I
would suggest leveraging this by making this change instead:

First in the list of LIBAWT_XAWT_DIRS (line 272), prepend a line like this:

$(wildcard
$(TOPDIR)/src/java.desktop/$(OPENJDK_TARGET_OS)/native/libawt_xawt)
\

/Erik


On 2018-05-04 07:07, Langer, Christoph wrote:
Hi,

please help reviewing the contribution of the support for the AIX Input
Method Editor (IME) in AWT's Input Method Framework.
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8201429.1/
Bug: https://bugs.openjdk.java.net/browse/JDK-8201429

I took Ichiroh's initial proposal [1] and tried to integrate it better with
existing code. I have split
src/java.desktop/unix/classes/sun/awt/X11InputMethod.java into
a) a base class containing the common code:
src/java.desktop/unix/classes/sun/awt/X11InputMethodBase.java
b) a specific class for the common Linux/Unixes:
src/java.desktop/unix/classes/sun/awt/X11InputMethod.java and
c) a specific class for AIX:
src/java.desktop/aix/classes/sun/awt/X11InputMethod.java
The AIX specific additions to the native code of
src/java.desktop/unix/native/libawt_xawt/awt/awt_InputMethod.c were so
much that I decided to add a specific implementation file for AIX:
src/java.desktop/aix/native/libawt_xawt/awt/awt_InputMethod_aix.c. The
changes to the former file are some cleanups.
I'm still in the process of testing the changes - but maybe you can run
further tests, especially on non-AIX unixes to make sure we didn't break
something.
Thanks & Best regards
Christoph

[1]: http://mail.openjdk.java.net/pipermail/awt-dev/2018-
April/013869.html

Reply via email to