Hi Anton, The fix looks good to me.
Just a minor remark: I would recommend using unicode instead of character during array definition inside isCharModifierKeyInUSInternationalPC() method, (i.e. replace ‘\’’ with ‘u\0027’ for apostrophe and so on). That’s quite trivial change and I don’t need a new webrev for it. Thanks, Dmitry > On 4 Jan 2020, at 19:43, Sergey Bylokhov <sergey.bylok...@oracle.com> wrote: > > Looks fine. > > On 12/20/19 6:53 pm, Anton Litvinov wrote: >> Hi Sergey, >> Thank you for review of this fix. I am not sure, that a similar issue does >> not exist for other keyboard layout with same or other modifier keys in JDK >> for macOS, because I do not know all keyboard layouts and do not know all >> their peculiarities. But I am sure that this JDK code related to input >> methods on macOS is very fragile and depends on many flags and "if" >> conditions in Objective-C and Java source code, and that is why all bugs >> related to input methods for last several years were fixed in narrow ways to >> address only those uncovered problematic scenarios. >> This particular bug exists in Oracle JDK from the very first release, the >> issue is reproducible with JDK 7u4 b21, this bug always affected "U.S. >> International - PC" layout in JDK. The issue is also reproducible with Apple >> JDK 1.6.0_65-b14-468. >> To my mind, a source of these problems with input methods is the fact that >> JDK receives explicit direction from macOS to insert certain characters >> through the method "(void) insertText:(id)aString >> replacementRange:(NSRange)replacementRange" in "AWTView.m" file, and in some >> cases JDK generates "java.awt.event.InputMethodEvent" as a result of this >> call and in some cases does not generate "InputMethodEvent" with the >> intention to generate "java.awt.event.KeyEvent" events instead of it as part >> of invocation of, for example, "(void) keyDown: (NSEvent *)event" method in >> "AWTView.m" file. >> In this bug with "U.S. International - PC" layout, after pressing " ' ", " p >> " characters JDK's method "insertText" from "AWTView.m" file is invoked by >> macOS two times: during the first time JDK is asked to insert " ' ", and >> during the second time it is asked to insert " p " character. So if JDK >> followed that direction from macOS, there would not be a bug, but JDK has >> own requirement to generate only key events for Latin characters, and "U.S. >> International - PC" layout relies only on key codes corresponding to Latin >> characters while at the same time provides input method functionality for >> insertion of characters with diacritics through pressing these modifier keys >> " '`"˜ˆ " followed by required regular Latin character keys. >> I had been debugging this bug and looking at it periodically for three >> years, of course I was trying to fix this bug from different sides. But this >> fix is the only well explained and reliable fix for this bug which I managed >> to create and it excludes a possibility of breaking JDK work with any other >> keyboard layouts on macOS. >> Thank you, >> Anton >> On 20/12/2019 06:36, Sergey Bylokhov wrote: >>> Hi, Anton. >>> >>> How we can be sure that this problem exists only in case of "U.S. >>> International - PC"? >>> Did you try to check other layouts and or possible fix variation w/o >>> hardcoded check for the layout? >>> >>> On 12/16/19 7:01 pm, Anton Litvinov wrote: >>>> Hello, >>>> >>>> Could you please review the following fix for the bug. >>>> >>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8230926 >>>> Webrev: http://cr.openjdk.java.net/~alitvinov/8230926/jdk15/webrev.00 >>>> >>>> The bug consists in the fact that if on macOS with currently set "U.S. >>>> International - PC" keyboard layout a user presses any of 5 modifier keys: >>>> ' (APOSTROPHE), " (QUOTATION MARK), ` (ACCENT GRAVE), ˜ (SMALL TILDE), ˆ >>>> (CIRCUMFLEX ACCENT) and then presses any consonant Latin letter key, for >>>> example "p" key, then 2 consecutive modifier keys are entered in AWT or >>>> Swing text field and the consonant letter is not entered. >>>> >>>> CAUSE OF THE BUG: >>>> >>>> The second extra modifier key is entered in the text field instead of the >>>> required letter during execution of the function "(void) keyDown: (NSEvent >>>> *)event" in "src/java.desktop//macosx/native/libawt_lwawt/awt/AWTView.m" >>>> for "NSEvent" instance representing pressing on the consonant letter key, >>>> for example that "p" key, through the next sequence of function/method >>>> calls: >>>> >>>> "(void) keyDown: (NSEvent *)event" - "AWTView.m" file. >>>> |-> "(void) deliverJavaKeyEventHelper: (NSEvent *) event" - "AWTView.m" >>>> file >>>> |-> "sun.lwawt.macosx.CPlatformView.deliverKeyEvent(NSEvent)" - >>>> "CPlatformView.java" file >>>> |-> "sun.lwawt.macosx.CPlatformResponder.handleKeyEvent" - >>>> "CPlatformResponder.java" file >>>> >>>> The instance of "NSEvent" being handled which corresponds to press on the >>>> consonant letter key, for example "p", has the next values of its 2 >>>> important fields: >>>> >>>> [event characters] = "'p" >>>> [event charactersIgnoringModifiers] = "p" >>>> >>>> The method "sun.lwawt.macosx.CPlatformResponder.handleKeyEvent" is >>>> designed in such a way that it selects only the first character of the >>>> string variable "chars" whose value in this case equals "'p" and generates >>>> Java AWT key events for " ' " character while ignoring presence of "p" >>>> character both in "chars" and "charsIgnoringModifiers" string variables. >>>> >>>> FIX: >>>> >>>> To change "sun.lwawt.macosx.CPlatformResponder.handleKeyEvent" method in >>>> such a way to let it generate AWT key events for a character from >>>> "charsIgnoringModifiers" string instead of the modifier key from the >>>> "chars" string, when "U.S. International - PC" keyboard layout is used. >>>> >>>> Thank you, >>>> Anton >>> >>> > > > -- > Best regards, Sergey.
