On Fri, 17 Nov 2023 20:05:09 GMT, Martin Fox <m...@openjdk.org> wrote:

> On Windows a common shortcut like Ctrl+'+' could only be invoked from the 
> main keyboard and not the numeric keypad. Toolkit.getKeyCodeForChar did not 
> have enough context to know whether it should return a result from the main 
> keyboard or the keypad.
> 
> This PR alters getKeyCodeForChar to pass in the code of the key the system is 
> trying to match against. Only the Windows platform actually uses this 
> additional information.
> 
> On the Mac the numeric keypad has always worked due to the odd way 
> getKeyCodeForChar is implemented (until PR #1209 the keypad worked more 
> reliably than the main keyboard). On Linux getKeyCodeForChar is a mess; 
> neither the main keyboard or the numeric keypad work reliably. I have an 
> upcoming PR which should make both work correctly.

sorry this PR was out waiting for too long.  could you sync it up with the 
latest master please?

modules/javafx.graphics/src/main/java/com/sun/glass/events/KeyEvent.java line 
255:

> 253:      * character with respect to the currently active keyboard layout or
> 254:      * VK_UNDEFINED if the character isn't present in the current layout.
> 255:      * The hint is the KeyCode of the key the system is attempting to 
> match.

is this clear enough or should we say that it's KeyCode.getCode()?

(also Application:746)

modules/javafx.graphics/src/main/java/com/sun/glass/events/KeyEvent.java line 
257:

> 255:      * The hint is the KeyCode of the key the system is attempting to 
> match.
> 256:      * It can be used to optimize the search or to distinguish between 
> the
> 257:      * main keyboard and the numeric keypad.

should we mention that it is supposed to be `KeyEvent.VK_UNDEFINED` when not 
available?

tests/system/src/test/java/test/robot/com/sun/glass/ui/monocle/MonocleApplicationTest.java
 line 143:

> 141:                 char ch = (char) TEST_CASES[i][0];
> 142:                 int expectedCode = TEST_CASES[i][1];
> 143:                 int code = MonocleApplicationShim._getKeyCodeForChar(ch, 
> KeyEvent.VK_UNDEFINED);

Do you want to add a new test(s) for the cases when the hint is important?

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

PR Review: https://git.openjdk.org/jfx/pull/1289#pullrequestreview-1871084872
PR Review Comment: https://git.openjdk.org/jfx/pull/1289#discussion_r1483488977
PR Review Comment: https://git.openjdk.org/jfx/pull/1289#discussion_r1483493173
PR Review Comment: https://git.openjdk.org/jfx/pull/1289#discussion_r1483495224

Reply via email to