On Thu, 27 Apr 2023 18:07:53 GMT, Martin Fox <d...@openjdk.org> wrote:
>> The algorithm in `KeyCharacterCombination.match` relies on the call
>> `Toolkit.getKeyCodeForChar` which is difficult to implement correctly. It
>> defies the way most keyboard API’s work and no platform has got it right
>> yet. In particular the Mac and Linux implementations have to resort to a
>> brute-force approach which monitors keystrokes to learn the relationship
>> between keys and characters.
>>
>> This PR introduces an alternative mechanism which directly asks the platform
>> whether a given key can generate a specific character. It also allows the
>> platform to attach identifying key information to each KeyEvent to make it
>> easier to answer the question (much, much easier).
>>
>> This is mostly dumb plumbing. On the front-end there’s a new call
>> `View.notifyKeyEx` that takes an additional platform-specific `hardwareCode`
>> parameter. It also returns a boolean indicating whether the event was
>> consumed or not so I can fix JDK-8087863. If you want to follow the path
>> visit the files in this order:
>>
>> View.java
>> GlassViewEventHandler.java
>> TKSceneListener.java
>> Scene.java
>>
>> The `KeyEvent` class has been expanded with an additional `hardwareCode`
>> member that can only be accessed internally. See KeyEvent.java and
>> KeyEventHelper.java.
>>
>> On the back-end `KeyCharacterCombination.match` calls a new routine
>> `Toolkit.getKeyCanGenerateCharacter` which unpacks the `KeyEvent`
>> information and sends it on to the Application. The default implementation
>> falls back to the old `getKeyCodeForChar` call but platform specific
>> Applications can send it on to the native glass code.
>>
>> KeyCharacterCombination.java
>> Toolkit.java
>> QuantumToolkit.java
>> Application.java
>> GtkApplication.java
>>
>> The glass code can use the `hardwareCode` to answer the question directly.
>> It also has enough information to fall back on the old `getKeyCodeForChar`
>> logic while also enabling the keypad (a common complaint is that Ctrl+’+’
>> only works on the main keyboard and not the keypad, see JDK-8090275).
>>
>> This PR improves the situation for key events generated by keystrokes.
>> Manually constructed key events won’t work any better or worse than they
>> already do. Based on the bug database I don't think this is an issue.
>
> Martin Fox has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Updating classpath for Eclipse users
modules/javafx.graphics/src/main/java/com/sun/glass/ui/View.java line 974:
> 972: }
> 973:
> 974: // Returns true iff event was consumed
There are some places where it's written "iff", I'm not sure if it's
intentional or a typo.
tests/manual/events/KeyCharacterCombinationTest.java line 41:
> 39:
> 40: public static void main(String[] args) {
> 41: Application.launch(args);
Would be nice to add the class to `launch`,
`Application.launch(KeyCharacterCombinationTest.class, args);` so it can be run
as:
`java @build/run.args tests/manual/events/KeyCharacterCombinationTest.java`
tests/manual/events/KeyCharacterCombinationTest.java line 45:
> 43:
> 44: @Override
> 45: public void start(Stage stage) {
I started the test app but was unsure how to test it. Maybe add a header text
that explains how a successful test should go.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/694#discussion_r1223665772
PR Review Comment: https://git.openjdk.org/jfx/pull/694#discussion_r1223667093
PR Review Comment: https://git.openjdk.org/jfx/pull/694#discussion_r1223667647
