On Fri, 14 Jun 2024 10:07:39 GMT, Abhishek Kumar <abhis...@openjdk.org> wrote:

>> In GTK LAF, the menu mnemonics are always displayed which is different from 
>> the native behavior. In native application **(tested with gedit**), the menu 
>> mnemonics toggle on press of `ALT` key. Menu mnemonics are hidden initially 
>> and then toggles between show/hide on `ALT` press. 
>> Proposed fix is to handle the `ALT` key press for GTK LAF and mimic the 
>> native behavior. Fix is similar to the `ALT` key processing in  Windows LAF. 
>> Automated test case is added to verify the fix and tested in Ubuntu and 
>> Oracle linux.
>> 
>> CI testing is green and link attached in JBS.
>
> Abhishek Kumar has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   condition update

Changes requested by aivanov (Reviewer).

src/java.desktop/share/classes/javax/swing/plaf/synth/SynthRootPaneUI.java line 
82:

> 80:             KeyboardFocusManager.getCurrentKeyboardFocusManager().
> 81:                     removeKeyEventPostProcessor(altProcessor);
> 82:         }

I think you have to keep track of whether `altProcessor` was installed or not. 
If the value of `"RootPane.altPress"` changes, the listener may be left 
installed.

src/java.desktop/share/classes/javax/swing/plaf/synth/SynthRootPaneUI.java line 
115:

> 113:             KeyboardFocusManager.getCurrentKeyboardFocusManager().
> 114:                     addKeyEventPostProcessor(altProcessor);
> 115:         }

Can this lead to installing `altProcessor` twice or more?

test/jdk/com/sun/java/swing/plaf/gtk/TestMenuMnemonicOnAltPress.java line 44:

> 42: import javax.swing.plaf.synth.SynthLookAndFeel;
> 43: 
> 44: public class TestMenuMnemonicOnAltPress {

This test seems to repeat the 
[`JMenuBar/TestMenuMnemonic.java`](https://github.com/openjdk/jdk/blob/master/test/jdk/javax/swing/JMenuBar/TestMenuMnemonic.java)
 test that you created for Windows L&F when you worked on 
[JDK-8326458](https://bugs.openjdk.org/browse/JDK-8326458).

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

PR Review: https://git.openjdk.org/jdk/pull/18992#pullrequestreview-2126001490
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1644763217
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1644755969
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1644770126

Reply via email to