<i18n dev> <AWT Dev> [8] Review request for 6789984: JPasswordField can not receive keyboard input

[Naoto Sato]

Looks good to me. Thank you for fixing this IM bug!

Naoto

On 11/13/12 6:54 AM, Anton Litvinov wrote:
> Hello Naoto,
>
> Thank you for reviewing this fix and pointing to a directory with Java
> input method (IM) implementation. Could you review a new version of the
> webrev containing a correction in comments of
> "sun.awt.im.InputMethodAdapter.stopListening()". In comparison with
> previous webrev the new one has additional changes only in
> "InputMethodAdapter.java".
>
> Webrev: http://cr.openjdk.java.net/~alitvinov/6789984/webrev.02
>
> The fix was manually tested with Java input methods (IMs) and native IMs
> loaded simultaneously on Linux and Windows hosts. "CodePointIM.jar" and
> "CityIM.jar" were used as Java IMs. No cases of loosing of ability to
> enter text into JPasswordField were observed after switches from Java to
> native IMs and vice versa. Also the IMs did loose their functional
> capabilities between those switches.
>
> Thank you,
> Anton
>
>
> On 11/13/2012 3:08 AM, Naoto Sato wrote:
>> The fix looks good. I'd suggest modifying the comment for
>> sun.awt.im.InputMethodAdapter.stopListening() too, since the method is
>> now being issued even when Java input method is not active.
>>
>> Also, please make sure there is no issue on switching the input method
>> between the input method adapter and a Java input method, such as
>> CodePointInputMethod which is in the JDK demo directory.
>>
>> Thanks!
>> Naoto
>>
>> On 11/11/12 12:07 PM, Anton Litvinov wrote:
>>> Hello Naoto,
>>>
>>> Could you, please, review a modified version of the fix, where a former
>>> duplicating code "endComposition();disableInputMethod()" was substituted
>>> for one call to "stopListening()". Also larger testing of the fix was
>>> done on Linux and Windows machine.
>>>
>>> Webrev: http://cr.openjdk.java.net/~alitvinov/6789984/webrev.01
>>>
>>> I came to a conclusion that substitution of call to
>>> "disableInputMethod()" for "stopListening()" in
>>> "sun.awt.im.InputContext", which is code shared between different
>>> platforms, will not lead to any changes on platforms other than Linux
>>> and Solaris. In order to prove this the following facts are provided:
>>>
>>>  1. Windows implementation of InputMethodAdapter
>>>     "jdk/src/windows/classes/sun/awt/windows/WInputMethod.java" does
>>>     nothing except calling "disableInputMethod()" from its overridden
>>>     "stopListening()", which means that this fix will not change the
>>>     logic in Windows implementation.
>>>  2. Mac OS implementation of InputMethodAdapter
>>>     "jdk/src/macosx/classes/sun/lwawt/macosx/CInputMethod.java" does not
>>>     override "stopListening()" at all and has just a stub implementation
>>>     of "disableInputMethod()", thus the fix cannot influence behavior of
>>>     InputMethodAdapter on Mac OS.
>>>
>>> The following tests were run on Linux and Windows with enabled input
>>> methods, and no negative changes in tests' results for JDK without/with
>>> fix were observed:
>>>
>>>  1. Manual/automatic jtreg tests from java/awt/im directory.
>>>  2. Automatic jtreg tests from Focus, keyboard, KeyboardFocusmanager
>>>     directories.
>>>  3. JCK tests from "api/java_awt/im" and "api/java_awt/InputMethod".
>>>
>>>
>>> Thank you,
>>> Anton
>>>
>>>
>>> On 11/7/2012 10:03 PM, Naoto Sato wrote:
>>>> Thank you for trying stopListening() out. Later I figured out that
>>>> this method is only used when the input method instance switches the
>>>> underlying input method engine from native OS's InputMethodAdapter
>>>> instance to pure Java input method, so in this problem case it may not
>>>> seem to apply. Let me know your further findings.
>>>>
>>>> Naoto
>>>>
>>>> On 11/7/12 8:11 AM, Anton Litvinov wrote:
>>>>> Hello Naoto,
>>>>>
>>>>> Yes, sure, I tried calling "stopListening()" method. But it does not
>>>>> lead to complete disabling of the input methods and appearance of the
>>>>> opportunity of entering a text into JPasswordField, when some
>>>>> composition started in JTextField before switch of focus from the last
>>>>> one to JPasswordField. Such situation happens, because "resetXIC()" is
>>>>> not called after "disableInputMethod()" inside "stopListening()" of
>>>>> "sun.awt.X11InputMethod" for the reason of unset flag "needResetXIC"
>>>>> after a call to "endComposition()". I think that I need to
>>>>> substitute a
>>>>> part of the fix calling to "endComposition()" and
>>>>> "disableInputMethod()"
>>>>> inside "disableNativeIM()" of "sun.awt.im.InputContext" for one
>>>>> call to
>>>>> "stopListening()". But before providing a next version of the fix
>>>>> for a
>>>>> review I will have to run regression tests connected with IM on
>>>>> platforms other than Linux and it will require some time.
>>>>>
>>>>> Thank you,
>>>>> Anton
>>>>>
>>>>> On 11/6/2012 11:21 PM, Naoto Sato wrote:
>>>>>> Hi Anton,
>>>>>>
>>>>>> In order for native input methods to not grab the native events, I
>>>>>> would guess that calling InputMethodAdapter.stopListening() would be
>>>>>> the right solution to me. Have you tried considering calling this?
>>>>>>
>>>>>> Naoto
>>>>>>
>>>>>> On 11/5/12 3:14 AM, Anton Litvinov wrote:
>>>>>>> Hello Naoto,
>>>>>>>
>>>>>>> Thank you for the review. A call to "endComposition()" was added to
>>>>>>> the
>>>>>>> shared code for resolution of another problem which was additionally
>>>>>>> observed. The problem consists in the fact that some not completely
>>>>>>> composed string inside JTextField can stay in composition state
>>>>>>> after
>>>>>>> the focus is moved from JTextField to JPasswordField. This not
>>>>>>> completed
>>>>>>> string marked with underscore disappears from JTextField at the
>>>>>>> moment
>>>>>>> when JTextField receives focus again. So I decided to make this
>>>>>>> string
>>>>>>> disappear when a component with disabled input methods, in current
>>>>>>> case
>>>>>>> JPasswordField, receives focus by a call to "endComposition()"
>>>>>>> method.
>>>>>>> Perhaps, it would be better to remove this call from the fix at all,
>>>>>>> because absence of end of text composition during switch of focus
>>>>>>> between component with enabled and disabled input methods can be
>>>>>>> another
>>>>>>> separate issue, would not it? But, on the other hand, since this fix
>>>>>>> brings the case with not composed string to a view, it should
>>>>>>> contain
>>>>>>> some solution which is specific to Unix platforms.
>>>>>>>
>>>>>>> Also a new comment concerning a root cause of the bug from my view
>>>>>>> point
>>>>>>> and a proposed solution was added to a page of the bug. Since it is
>>>>>>> not
>>>>>>> visible on "http://bugs.sun.com" yet, it can be seen on Oracle's
>>>>>>> internal bug tracking resource.
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Anton
>>>>>>>
>>>>>>> On 11/2/2012 9:08 PM, Naoto Sato wrote:
>>>>>>>> Hi Anton,
>>>>>>>>
>>>>>>>> What's the reason for adding endComposition() in the shared code?
>>>>>>>> endComposition() is supposed to end the unfinished composed string
>>>>>>>> that is currently in composition, and the bug does not mention
>>>>>>>> anything regarding the composition. Also, it looks like the bug is
>>>>>>>> localized only to SCIM on Unix platforms, and calling resetXIC()
>>>>>>>> looks
>>>>>>>> just enough in XIM handling code to me.
>>>>>>>>
>>>>>>>> Naoto
>>>>>>>>
>>>>>>>> On 11/2/12 5:43 AM, Anthony Petrov wrote:
>>>>>>>>> Hi Anton,
>>>>>>>>>
>>>>>>>>> I'm not an expert in IM code, but generally the changes look good
>>>>>>>>> to me.
>>>>>>>>> I'm also CC'ing i18n-dev@ to take a look at this fix.
>>>>>>>>>
>>>>>>>>> My only concern is that you're changing shared code here. Did you
>>>>>>>>> run
>>>>>>>>> any IM tests on other platforms to make sure no regressions are
>>>>>>>>> introduced?
>>>>>>>>>
>>>>>>>>> Also, could you please add a comment to the bug report explaining
>>>>>>>>> the
>>>>>>>>> root cause of the issue and how the proposed solution helps
>>>>>>>>> resolve
>>>>>>>>> it?
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> best regards,
>>>>>>>>> Anthony
>>>>>>>>>
>>>>>>>>> On 11/1/2012 8:40 PM, Anton Litvinov wrote:
>>>>>>>>>> Hello,
>>>>>>>>>>
>>>>>>>>>> Please review the following fix for a bug.
>>>>>>>>>>
>>>>>>>>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6789984
>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~alitvinov/6789984/webrev.00
>>>>>>>>>>
>>>>>>>>>> This bug consists in inability of JPasswordField to react on any
>>>>>>>>>> keyboard events under certain conditions on Linux OS. The webrev
>>>>>>>>>> does
>>>>>>>>>> not contain any new "jtreg" test, because a particular Linux OS
>>>>>>>>>> distribution with additionally installed packages are required
>>>>>>>>>> for
>>>>>>>>>> reproduction of the bug.
>>>>>>>>>>
>>>>>>>>>> Requirements to the environment:
>>>>>>>>>> 1. Oracle Enterprise Linux (OEL) 5.6 x86 64/Oracle Enterprise
>>>>>>>>>> Linux
>>>>>>>>>> (OEL) 5.5 x86 64
>>>>>>>>>> 2. Package "scim-anthy-1.2.0-6.el5.x86_64" and its all dependent
>>>>>>>>>> packages installed on the OS.
>>>>>>>>>> 3. JDK 8.
>>>>>>>>>> 4. A simple Java Swing application, where a dialog with both
>>>>>>>>>> JTextField and JPasswordField is displayed.
>>>>>>>>>>
>>>>>>>>>> Thank you,
>>>>>>>>>> Anton
>>>>>>>>
>>>>>>
>>>>
>>