8278938: [Win] Robot can target wrong key for punctuation and symbols

[beldenfox]

When processing a WM_CHAR event on an OEM key (punctuation, symbol, dead key) the glass code will dynamically query the key's unshifted character to determine the Java code to assign to it. This is necessary since the relationship between OEM key codes and the characters they generate varies from layout to layout.

The Robot implementation was consulting a table which assumed a fixed relationship between Java codes and Windows key codes even for the OEM keys. The table was also missing entries for any Java code not on a US QWERTY layout, like PLUS.

In this PR if we don't find the Java code in the table or if it maps to an OEM key (which may be wrong) we sweep through all the OEM keys looking for the matching Java code.

---

Progress

• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue
• [ ] Change must be properly reviewed

Issue

• JDK-8278938: [Win] Robot can target wrong key for punctuation and symbols

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jfx pull/702/head:pull/702
$ git checkout pull/702

Update a local copy of the PR:
$ git checkout pull/702
$ git pull https://git.openjdk.java.net/jfx pull/702/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 702

View PR using the GUI difftool:
$ git pr show -t 702

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jfx/pull/702.diff

[bridgekeeper[bot]]

:wave: Welcome back beldenfox! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[mlbridge[bot]]

Webrevs

• 03: Full - Incremental (d6063597)
• 02: Full - Incremental (d0d7acab)
• 01: Full - Incremental (e9dfaa75)
• 00: Full (dcd4c98c)

[kevinrushforth]

/reviewers 2

[openjdk[bot]]

@kevinrushforth The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

[bridgekeeper[bot]]

@beldenfox This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[kevinrushforth]

@andy-goryachev-oracle or @jperedadnr would one of you also be able to review this?

[beldenfox]

I wonder if RobotKeySanityTest could be part of the PR as a manual test? Same as in #694

The RobotKeySanityTest is tricky, it requires the user to press a lot of keys to get good coverage but can produce confusing results if the user presses certain keys, like AltGr. Windows treats AltGr as if you pressed Ctrl and Alt at the same time. This means the OS sends two events when you press the key and two more when you release it. This is confusing the RobotKeySanityTest which can only handle one press/release pair at a time. I'm not sure I can fix the test but will think about it.

The KeyboardTest app that I recently added to PR #425 covers most of the same ground and avoids the problematic keys. The downside of KeyboardTest is that it only provides good coverage for specific layouts (I'll try to add Spanish) and provides the best coverage when run against multiple layouts which takes effort to set up.

I've written an app that just logs key events as the user types and that would probably be a better candidate to attach to some PR. When something confusing happens with RobotKeySanityTest or KeyboardTest that's the app I pull up to diagnose what's going on (like I just did with AltGr).

[jperedadnr]

Okay, makes sense. I'll approve this PR as is then, if you modify it for any reason, I'll review again.

[kevinrushforth]

@beldenfox One more comment: since your branch is quite out of date, can you please merge in the latest upstream master?

[openjdk[bot]]

@beldenfox This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8278938: [Win] Robot can target wrong key for punctuation and symbols

Reviewed-by: jpereda, kcr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 2 new commits pushed to the master branch:

• fb63b26fc5fe277e5c95d16aedd7703b64fe2253: 8224260: ChangeListener not triggered when adding a new listener in invalidated method
• 5395942174073f25a7fa8f6ccf2fb4dc6604133a: 8304933: BitSet (used for CSS pseudo class states) listener management is incorrect

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch. As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@jperedadnr, @kevinrushforth) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[beldenfox]

/integrate

[openjdk[bot]]

@beldenfox Your change (at version d6063597a2fe1505dd4c98fb55d802712ed3cc54) is now ready to be sponsored by a Committer.

[kevinrushforth]

/sponsor

[openjdk[bot]]

Going to push as commit 3d6f3288ace658900e9af11792d7dd444ad55277. Since your change was applied there have been 2 commits pushed to the master branch:

• fb63b26fc5fe277e5c95d16aedd7703b64fe2253: 8224260: ChangeListener not triggered when adding a new listener in invalidated method
• 5395942174073f25a7fa8f6ccf2fb4dc6604133a: 8304933: BitSet (used for CSS pseudo class states) listener management is incorrect

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@kevinrushforth @beldenfox Pushed as commit 3d6f3288ace658900e9af11792d7dd444ad55277.

:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.