RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v8]

[Alexey Ivanov]

On Fri, 21 Jun 2024 07:55:24 GMT, Abhishek Kumar <abhiscxk at 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 for normal buttons and tested with libreoffice for menu**), 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:
> 
>   Remove access modifier from method declaration

Changes requested by aivanov (Reviewer).

src/java.desktop/share/classes/javax/swing/plaf/synth/SynthGraphicsUtils.java line 751:

> 749:      * Repaints all the components with the mnemonics in the given window and all its owned windows.
> 750:      */
> 751:     static void repaintMnemonicsInWindow(final Window w) {

The `repaintMnemonicsInWindow` and `repaintMnemonicsInContainer` are exactly the same as methods in `WindowsGraphicsUtils`. And `AquaMnemonicHandler` has yet another copy of the same code.

Is it possible to move these methods to a utility class that's available for all Look-and-Feels to avoid duplicating code?

src/java.desktop/share/classes/javax/swing/plaf/synth/SynthGraphicsUtils.java line 779:

> 777:                 c.repaint();
> 778:                 continue;
> 779:             } else if (c instanceof Container){

You said you were handling menu mnemonics only, yet this is more than menus. However, this would make the UI look consistent.

src/java.desktop/share/classes/javax/swing/plaf/synth/SynthLookAndFeel.java line 1053:

> 1051:      * This method is a non operation if the underlying operating system
> 1052:      * does not support the mnemonic hiding feature.
> 1053:      */

You can still write proper javadocs for members with any visibility, and IDE will show you the description of a method or a field when you hover over its usage anywhere in the code.

You already wrote the javadoc, leave them.

A regular comment won't be shown this way.

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

> 43:     private SynthStyle style;
> 44:     static final AltProcessor altProcessor = new AltProcessor();
> 45:     static boolean altProcessorInstalledFlag;

Wouldn't it be easier to install `altProcessor` always in `SynthLookAndFeel.initialize` and to uninstall it in `SynthLookAndFeel.uninitialize` like it's done in `WindowsLookAndFeel`:

https://github.com/openjdk/jdk/blob/master/src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsLookAndFeel.java#L197-L198

Then `altProcessor` would do nothing if the `RootPane.altPress` property isn't set or is `false`.

Requesting the value of `RootPane.altPress` from `UIManager` each time `postProcessKeyEvent` is called is inefficient, so you can store the value in `altProcessor` when look and feel is installed.

If `RootPane.altPress` can be changed dynamically, you can install a `PropertyChangeListener` to `UIManager`.

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

PR Review: https://git.openjdk.org/jdk/pull/18992#pullrequestreview-2133329175
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1649390460
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1649377154
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1649372792
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1649386106