8389582: JavaFX MenuItem accelerator for NumPad keys is displayed incorrectly

[andy-goryachev-oracle]

Updated KeyCodeCombination.getDisplayText() to return "NumPad *" text for all numpad keys:

            Arguments.of("NumPad 0", KeyCode.NUMPAD0),
            Arguments.of("NumPad 1", KeyCode.NUMPAD1),
            Arguments.of("NumPad 2", KeyCode.NUMPAD2),
            Arguments.of("NumPad 3", KeyCode.NUMPAD3),
            Arguments.of("NumPad 4", KeyCode.NUMPAD4),
            Arguments.of("NumPad 5", KeyCode.NUMPAD5),
            Arguments.of("NumPad 6", KeyCode.NUMPAD6),
            Arguments.of("NumPad 7", KeyCode.NUMPAD7),
            Arguments.of("NumPad 8", KeyCode.NUMPAD8),
            Arguments.of("NumPad 9", KeyCode.NUMPAD9),
            Arguments.of("NumPad *", KeyCode.MULTIPLY),
            Arguments.of("NumPad +", KeyCode.ADD),
            Arguments.of("NumPad -", KeyCode.SUBTRACT),
            Arguments.of("NumPad .", KeyCode.DECIMAL),
            Arguments.of("NumPad /", KeyCode.DIVIDE)

Added test for numpad and also modified the test case where we have platform-specific differences (Backspace, Delete, ...)

NOTE: noticed the auto-generated text shows weird names - "Back Space" instead of "Backspace". We might want to double check and fix these as well.

some names are weird, perhaps these should also be fixed:

KeyCode.BACK_SPACE: Back Space
KeyCode.QUOTEDBL: Quotedbl
KeyCode.EJECT_TOGGLE: Eject Toggle
KeyCode.KP_DOWN: Kp Down
KeyCode.KP_LEFT: Kp Left
KeyCode.KP_RIGHT: Kp Right
KeyCode.KP_UP: Kp Up

Also, there is difference in naming certain keys between macOS keyboards and the rest of the world:

esc - Esc
backspace == delete
return - Enter
caps lock - Caps Lock
shift - Shift

The use of symbols for macOS is questionable in my opinion, maybe the keyboard have changed since then:

KeyCode.BACK_SPACE: ⌫
KeyCode.DELETE: ⌦
KeyCode.ESCAPE: ⎋

---

• I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8389582: JavaFX MenuItem accelerator for NumPad keys is displayed incorrectly (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/2257/head:pull/2257
$ git checkout pull/2257

Update a local copy of the PR:
$ git checkout pull/2257
$ git pull https://git.openjdk.org/jfx.git pull/2257/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2257

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/2257.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back angorya! 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.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

The total number of required reviews for this PR has been set to 2 based on the presence of this label: rfr. This can be overridden with the /reviewers command.

[mlbridge]

Webrevs

• 01: Full - Incremental (76dfd885)
• 00: Full (1b2f5dae)

[beldenfox]

It's uncommon to create menu items with most of these shortcuts so I'm not sure how much effort we should put into this. But if we're going to clean this up we should consider doing it all.

To avoid distraction most keyboard shortcut text is as brief as possible. I would recommend "Num 4" instead of "NumPad 4". For that matter "Page Down" should be "PgDn", "Escape" just "Esc", and "Back Space" just "Backspace". That would also align better with what's printed on most keyboards.

You mention the distinction between "esc" and "Esc". Keyboards don't generally capitalize "esc" but it's still correct to use "Esc" in the menu item text the same way we write "Ctrl" instead of "ctrl".

The Mac has a long tradition of using only symbols in the shortcut text instead of spelling out the key's name so it's correct for JavaFX to use ⎋ for escape even though I don't think that symbol has been printed on any keyboard.

Don't worry about KP_LEFT, KP_RIGHT, KP_UP, or KP_DOWN. They've never been implemented (even in AWT) and never can be. See JDK-8316307.

[andy-goryachev-oracle]

You bring a good point, though getDisplayText() might be used by the application in other scenarios (shortcut preferences come to mind).

"Num": I am not sure about this. Swing does "NumPad-0" or "NumPad .", though on mac it uses ⌨ = \u2328 symbol which is total nonsense. I think "NumPad" is still a better choice for consistency reasons.

There is an option of using ⊞ (U+229E, SQUARED PLUS) on mac, but it's debatable.

Do agree with capitalization - lowercase looks more like a stylistic choice (I have keyboards with both capitalized and all-lowercase labels.

PgUp/PgDn is probably a reasonable abbreviation.