Hi Anton,
Thank you for the detailed explanation. The initial version of the fix, (i.e.
http://cr.openjdk.java.net/~alitvinov/8230926/jdk15/webrev.00
<http://cr.openjdk.java.net/~alitvinov/8230926/jdk15/webrev.00> ) looks good to
me.
—Dmitry
> On 13 Jan 2020, at 17:01, Anton Litvinov <anton.litvi...@oracle.com> wrote:
>
> Hello Dmitry,
>
> Thank you for review of this fix. I tried to address your remark and suggest
> not to encode all 5 modifier keys and leave the fix as is, because the
> defined character array will be not readable in comparison with its variant
> in the first fix version.
>
> Current variant:
> "final char[] modifierKeys = {‘\’‘, ‘“’, ‘`’, ‘\u02DC’, ‘\u02C6’};"
>
> New variant which is compilable and working:
> "final char[] modifierKeys = {‘\u005C\u0027’, ‘\u0022’, ‘\u0060’, ‘\u02DC’,
> ‘\u02C6’};"
>
> Not readable character in this new representation of the array is
> ‘\u005C\u0027’. Apostrophe cannot be specified as '\u0027’, because the
> compilation fails with error, from which it is clear that apostrophe must be
> shielded with backslash character. '\\u0027' also cannot be compiled, because
> the compiler states that backslash is not followed by the character that can
> be shielded. In order to make the code compilable it is necessary to specify
> both backslash and apostrophe in hexadecimal Unicode values what is
> ‘\u005C\u0027’.
>
> In the first fix version I decided to represent small tilde ˜ and circumflex
> accent ˆ through their hexadecimal Unicode values, because they are not in
> ASCII code table and are not plain tilde ~ and plain circumflex accent ^. On
> macOS simple tilde ~ and simple circumflex accent ^ are not modifier keys in
> "U.S. International - PC" keyboard layout, where instead of them as modifier
> keys those small tilde ˜ and circumflex accent ˆ are used, which are located
> far beyond the ASCII table. Java source code files in JDK are saved in ASCII
> encoding, therefore in order to store ˜ and ˆ not encoded through hexadecimal
> Unicode values, I would need to save the whole file "LWCToolkit.java" in
> UTF-8 encoding and I did not want to change the encoding of the source file.
> Thus I encoded just those 2 problem symbols in the array "modifierKeys".
>
> Therefore I suggest to stay with the first fix version. Dmitry, what is your
> opinion about this suggestion?
>
> Thank you,
> Anton
>
> On 07/01/2020 13:07, Dmitry Markov wrote:
>> 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.
>
