On Fri, 5 May 2023 20:00:55 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> Note: the Java-side changes in this PR are also in #694 which fixes the same 
>> issue (and more) on Linux. Unfortunately the Linux Robot code is not working 
>> making it difficult to test on that platform (see #718).
>> 
>> KeyCharacterCombinations allow the specification of accelerators based on 
>> characters whose KeyCodes vary across keyboard layouts. For example, the + 
>> character is on KeyCode.EQUALS on a U.S. English layout, KeyCode.PLUS on a 
>> German layout, and KeyCode.DIGIT1 on a Mac Swiss German layout. 
>> KeyCharacterCombinations finds the correct KeyCode by calling 
>> `Toolkit.getKeyCodeForChar`.
>> 
>> `getKeyCodeForChar` can only return one KeyCode for a given character so it 
>> can't easily handle characters which appear in more than one location, like 
>> + which is on the main keyboard and the numeric keypad. It's also reliant on 
>> KeyCodes which prevents KeyCharacterCombinations from working on keys with 
>> no codes (e.g. the base character contains a diacritic). It also relies on 
>> the platform to map from a character to a key which is the reverse of how 
>> key mapping normally works making it slow and/or imprecise to implement on 
>> Mac and Linux (Windows is the only platform with a system call to do this).
>> 
>> This PR introduces a new way for a platform to pass key information to the 
>> Java core. `View.notifyKeyEx` takes an additional platform-specific 
>> `hardwareCode` which identifies the key and is tracked in a private field in 
>> the KeyEvent. This is opt-in; a platform can continue to call the old 
>> `View.notifyKey` method and allow the `hardwareCode` to default to -1.
>> 
>> On the back-end `KeyCharacterCombination.match` calls the new routine 
>> `Toolkit.getKeyCanGenerateCharacter` which unpacks the KeyEvent information 
>> and sends it on to the Application. This is also opt-in; the default 
>> implementation falls back to the Application's `getKeyCodeForChar` call. 
>> Platforms which call `View.notifyKeyEx` will be handed the `hardwareCode` 
>> for the key in addition to the Java KeyCode.
>> 
>> The new `View.notifyKeyEx` returns a boolean indicating whether the event 
>> was consumed or not. This plays no role here but will be used later to fix 
>> [JDK-8087863](https://bugs.openjdk.org/browse/JDK-8087863).
>> 
>> For testing I've included the manual KeyboardTest app that also appears in 
>> PR #425. Tests with keypad combinations should now work.
>> 
>> Note: this PR only fixes Windows. Fixes for Mac and Linux but can't be 
>> submitted until #425 and #718 are integrated.
>
> modules/javafx.graphics/src/main/java/javafx/scene/input/KeyEvent.java line 
> 388:
> 
>> 386:      * The hardware key code which is private to the implementation.
>> 387:      */
>> 388:     private int hardwareCode;
> 
> Does it need to be private? Events are public, and I think it should be 
> possible to make your own events that act exactly like an event created by 
> the system, which would preclude hidden variables.

At this point I don't have a strong opinion on whether this field is private or 
not. Originally I wanted it to be private because I thought of it as "whatever 
magic value makes KeyCharacterCombinations work" and was concerned we might 
want to tweak that over time. That's less of a concern now that I've prototyped 
the implementation on three different platforms.

This is largely a policy issue that's above my pay grade. This is an 
intrinsically platform-specific bit of information we would be exposing to 
developers.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1126#discussion_r1188927478

Reply via email to