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

[Alexey Ivanov]

On Wed, 3 Jul 2024 11:17:56 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:
> 
>   Test update to refer MnemonicHandler class

This changeset enables hiding / showing mnemonics on `JMenuBar` only. Do you plan to update `ButtonUI` and `LabelUI` for GTK Look-and-Feel too?

src/java.desktop/macosx/classes/com/apple/laf/AquaLabelUI.java line 37:

> 35: import javax.swing.plaf.UIResource;
> 36: 
> 37: import javax.swing.plaf.basic.BasicLabelUI;

I would prefer not to add blank lines between these imports.

Having a blank line between `java.*` and `javax.*` above is fine.

src/java.desktop/macosx/classes/com/apple/laf/AquaLabelUI.java line 43:

> 41: 
> 42: import com.apple.laf.AquaUtils.RecyclableSingleton;
> 43: import com.apple.laf.AquaUtils.RecyclableSingletonFromDefaultConstructor;

I'd sort them alphabetically too: `sun.swing` follows `com.apple.laf`. I'd not add a blank line between these imports.

src/java.desktop/macosx/classes/com/apple/laf/AquaLookAndFeel.java line 58:

> 56: import apple.laf.JRSUIControl;
> 57: import apple.laf.JRSUIUtils;
> 58: 

Remove the blank line?

src/java.desktop/macosx/classes/com/apple/laf/AquaMenuPainter.java line 37:

> 35: import java.awt.Rectangle;
> 36: 
> 37: import java.awt.event.InputEvent;

No blank line here.

src/java.desktop/macosx/classes/com/apple/laf/AquaMenuPainter.java line 52:

> 50: import javax.swing.UIManager;
> 51: 
> 52: import javax.swing.border.Border;

No blank line here.

src/java.desktop/macosx/classes/com/apple/laf/AquaMenuPainter.java line 62:

> 60: import apple.laf.JRSUIConstants.Widget;
> 61: 
> 62: import com.apple.laf.AquaIcon.InvertableIcon;

Maybe preserve the blank lines in this section, yet it could be better to remove these too.

src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKLookAndFeel.java line 46:

> 44: import java.util.Map;
> 45: import sun.awt.SunToolkit;
> 46: import sun.awt.UNIXToolkit;

Optimize imports in this file too, to expand the wildcard imports.

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsGraphicsUtils.java line 43:

> 41: import javax.swing.JMenu;
> 42: import javax.swing.JMenuItem;
> 43: import javax.swing.UIManager;

Suggestion:

import javax.swing.AbstractButton;
import javax.swing.ButtonModel;
import javax.swing.JButton;
import javax.swing.JCheckBox;
import javax.swing.JMenu;
import javax.swing.JMenuItem;
import javax.swing.JRadioButton;
import javax.swing.JToggleButton;
import javax.swing.UIManager;

Keep the imports sorted.

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsGraphicsUtils.java line 49:

> 47: import sun.swing.SwingUtilities2;
> 48: 
> 49: import com.sun.java.swing.plaf.windows.WindowsButtonUI;

The `WindowsButtonUI` import is unused.

test/jdk/javax/swing/JMenuBar/TestMenuMnemonicLinuxAndMac.java line 63:

> 61:                 System.out.println("Testing: "+laf.getName());
> 62:                 UIManager.setLookAndFeel(laf.getClassName());
> 63:             }

Suggestion:

            if (laf.getName().contains("GTK") || laf.getName().contains("Aqua")) {
                System.out.println("Testing: " + laf.getName());
                UIManager.setLookAndFeel(laf.getClassName());
                break;
            }

Since we expect one and only one L&F, break from the loop as soon as you found a match.

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18992#pullrequestreview-2159270954
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1665920495
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1665921960
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1665923019
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1665924234
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1665924574
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1665925751
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1665926565
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1665928024
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1665927518
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1665934285