On Mon, 30 Mar 2026 19:31:16 GMT, Phil Race <[email protected]> wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add mnemonic mask test
>
> src/java.desktop/share/classes/javax/swing/SwingUtilities.java line 907:
>
>> 905: /**
>> 906: * Returns the system mnemonic key mask to be used
>> 907: * for shortcut keymask and/or accelerator keymask
>
> IMO that is too vague. I'd like to see more precise and more helpful /
> descriptive text here.
>
> Also I see that SU2 will return ALT_MASK and I wonder if it (and SunToolkit
> of course) should be returning ALT_DOWN_MASK instead ?
>
> And since this is a JDK defined mask, maybe entirely for Swing, isn't the
> word "System" is mis-leading ?
> Meaning if there's a look up anywhere of what the desktop expects, I'm
> missing it.
I guess SU2 should use ALT_DOWN_MASK as AT_MASK is deprecated from jdk9 but I
thought it should be done in separate PR and not for this exposure PR where I
guess we donot touch implementation details.
Regarding "System" I just used the wrapper over SU2 so I didn't change..I can
change the public API to `getSwingShortcutKeyMask` as it caters to accelerator
and mnemonic both, if there is consensus..
> test/jdk/javax/swing/SwingUtilities/TestAcceleratorMask.java line 94:
>
>> 92: if ((mod & (InputEvent.SHIFT_DOWN_MASK | InputEvent.SHIFT_MASK))
>> != 0) {
>> 93: return KeyEvent.VK_SHIFT;
>> 94: }
>
> Since SFAICS we always return VK_ALT, what was the reason for checking the
> other masks ?
getSystemMnemonicKeyMask calls getFocusAcceleratorKeyMask which in LWCToolkit
returns CTRL_MASK|ALT_MASK
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30366#discussion_r3013059291
PR Review Comment: https://git.openjdk.org/jdk/pull/30366#discussion_r3013059386
