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

Anthony Petrov anthony.petrov at oracle.com
Wed Nov 14 04:55:30 PST 2012 

Hi Anton,

Yes, I followed your discussion with Naoto. The fix looks fine to me. 
Thanks for testing it so thoroughly!

Do you want me to push it to the repository, or you have your own 
OpenJDK id with commit rights already?

--
best regards,
Anthony

On 11/14/2012 2:20 PM, Anton Litvinov wrote:
>   Hello Anthony,
> 
> Could you review the latest version of the fix, which is already 
> approved by Naoto Sato from i18n development team. An e-mail containing 
> all the correspondence with Naoto concerning the fix is attached to this 
> e-mail.
> 
> Webrev: http://cr.openjdk.java.net/~alitvinov/6789984/webrev.02
> 
> The following tests were run on Linux and Windows with enabled input 
> methods (IMs), 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".
>    4. Manual check of a test case provided on a web page of this CR with
>       Java 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/2/2012 4:43 PM, 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
> 
> ------------------------------------------------------------------------
> 
> Subject:
> Re: <i18n dev> <AWT Dev> [8] Review request for 6789984: JPasswordField 
> can not receive keyboard input
> From:
> Naoto Sato <naoto.sato at oracle.com>
> Date:
> Tue, 13 Nov 2012 09:30:28 -0800
> To:
> Anton Litvinov <anton.litvinov at oracle.com>
> 
> To:
> Anton Litvinov <anton.litvinov at oracle.com>
> CC:
> Anthony Petrov <anthony.petrov at oracle.com>, awt-dev at openjdk.java.net, 
> i18n-dev at openjdk.java.net
> 
> 
> 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
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>

More information about the awt-dev mailing list