RFR: 4850101: Setting mnemonic to VK_F4 underlines the letter S in a button. [v2]

[Prasanta Sadhukhan]

On Sat, 13 Aug 2022 13:39:08 GMT, Karl T <duke at openjdk.org> wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Modify check for lowercase character
>
> src/java.desktop/share/classes/javax/swing/SwingUtilities.java line 2089:
> 
>> 2087:            return -1;
>> 2088:         }
>> 2089: 
> 
> There are some issues with this implementation IMHO:
> - why invoke `KeyEvent.getKeyText(mnemonic)` eleven times? This creates eleven temporary strings. The result is always the same.
> - `KeyEvent.getKeyText()` may return localized strings, which would break the fix
> - why not simply compare `mnemonic` with ` KeyEvent.VK_F*`?
> 
> Following should do the same:
> ~~~java
> if (mnemonic >= KeyEvent.VK_F1 && mnemonic <= KeyEvent.VK_F11) {
>     return -1;
> }
> ~~~
> 
> But IMO **all lowercase ascii characters** should be excluded because there are more `VK_` keys in this range (e.g. `setMnemonic(VK_NUMPAD1)` would underline the 'a' character, but `Alt+A` does not click the button):
> 
> ~~~java
> if (mnemonic >= 'a' && mnemonic <= 'z') {
>     return -1;
> }
> ~~~

Yes, calling KeyEvent.getKeyText(mnemonic) so many times is a mistake, which could have been done once.

I was also sceptical about checking for 'a' to 'z' as spec of method says "This method is only designed to handle character values which fall between 'a' and 'z' or 'A' and 'Z'." so 'a', 'z' are valid characters but this method is for displaying Mnemonic index which is not affected and it prevents the action keys underlining lowercase characters.

Thanks for your suggestion which is incorporated.

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

PR: https://git.openjdk.org/jdk/pull/9712