On Tue, 21 Feb 2023 17:09:32 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 with a new target base due to a merge
> or a rebase. The pull request now contains three commits:
>
> - Merge remote-tracking branch 'upstream/master' into scancode
> - Added manual test for KeyCharacterCombination matching
> - New KeyCharacterCombination implementation
Changes requested by angorya (Committer).
For some reason, Command-+ on Mac did not generate console output in
KeyCharComboTest (in the ticket).
Using Windows+L (on the attached IBM keyboard) did generate stdout, but not
Windows-+.
Am I doing something wrong?
modules/javafx.graphics/src/main/java/com/sun/javafx/tk/TKSceneListener.java
line 67:
> 65: * Pass a key event to the scene to handle
> 66: */
> 67: public boolean keyEvent(KeyEvent keyEvent);
could we update javadoc to explain when it returns true?
modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 2178:
> 2176: }
> 2177:
> 2178: boolean processKeyEvent(KeyEvent e) {
would be nice to have a one-line javadoc explaining when it returns true
modules/javafx.graphics/src/main/native-glass/gtk/glass_key.cpp line 433:
> 431:
> 432: if (hardwareCode < 0) {
> 433: if (vkCode != com_sun_glass_events_KeyEvent_VK_UNDEFINED)
please add curly braces
modules/javafx.graphics/src/main/native-glass/gtk/glass_key.cpp line 462:
> 460: }
> 461: if (allAreZero)
> 462: searchGroup = 0;
{
tests/manual/events/KeyCharacterCombinationTest.java line 86:
> 84: {
> 85: if (down)
> 86: return KeyCombination.ModifierValue.DOWN;
{
tests/manual/events/KeyCharacterCombinationTest.java line 151:
> 149: // Replace 'invisible' characters with their names.
> 150: if (!combinationDescription.isEmpty())
> 151: combinationDescription =
> combinationDescription.substring(0, combinationDescription.length() - 1);
{
-------------
PR Review: https://git.openjdk.org/jfx/pull/694#pullrequestreview-1396267822
PR Comment: https://git.openjdk.org/jfx/pull/694#issuecomment-1518242039
PR Review Comment: https://git.openjdk.org/jfx/pull/694#discussion_r1174082802
PR Review Comment: https://git.openjdk.org/jfx/pull/694#discussion_r1174078238
PR Review Comment: https://git.openjdk.org/jfx/pull/694#discussion_r1174080814
PR Review Comment: https://git.openjdk.org/jfx/pull/694#discussion_r1174080930
PR Review Comment: https://git.openjdk.org/jfx/pull/694#discussion_r1174081157
PR Review Comment: https://git.openjdk.org/jfx/pull/694#discussion_r1174081471
