On Tue, 14 May 2024 22:29:18 GMT, Martin Fox <m...@openjdk.org> wrote:

>> On Linux getKeyCodeForChar does not consult the current keyboard layout. For 
>> example, it assumes the “+” character is on KeyCode.PLUS even on layouts 
>> which don’t generate KeyCode.PLUS. The result is that most 
>> KeyCharacterCombinations don’t work.
>> 
>> This PR fixes this using the same approach that Mac glass uses. When the 
>> user types a key we lookup all the characters that key might generate and 
>> put them in a map. getKeyCodeForChar then consults the map to find the Java 
>> key code. This ensures that we only pay attention to keys that are on the 
>> user’s physical keyboard.
>> 
>> (Some Linux layout maps are expansive and include entries for keys that may 
>> be on the user’s keyboard but probably aren’t, like “(“ and “)” on the 
>> numeric keypad. If we simply ask for all entries that generate a given 
>> character we can get multiple hits with no good way to determine which ones 
>> are physically present.)
>> 
>> This PR also contains fixes to the Robot to enable testing. When Glass 
>> consults the GdkKeymap to determine which keys might generate a keyval GDK 
>> returns a list covering every installed layout and shift level. The old 
>> Glass code simply picked the first entry in the list. This PR iterates 
>> through the list looking for one that works for the current layout and 
>> correct shift level.
>
> Martin Fox has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains ten additional commits since 
> the last revision:
> 
>  - Merge remote-tracking branch 'upstream/master' into linuxcharcombo
>  - Comment fixes
>  - Consistency in naming conventions and comment cleanup.
>  - Merge remote-tracking branch 'upstream/master' into linuxcharcombo
>  - Expanded robot lookup table, general cleanup
>  - Merge remote-tracking branch 'upstream/master' into linuxcharcombo
>  - Resolving duplicates for Robot, fallback for getKeyCodeForChar
>  - Merge remote-tracking branch 'upstream/master' into linuxcharcombo
>  - KeyCharacterCombination fixes on Linux

Tested on linux mint, the app in the ticket fails without the fix and works 
with the fix.

The code looks good to me, though I'd like to have a second pair of eyes on it.

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

Marked as reviewed by angorya (Reviewer).

PR Review: https://git.openjdk.org/jfx/pull/1373#pullrequestreview-2125852531

Reply via email to