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

Alexey Ivanov aivanov at openjdk.org
Fri Jul 5 10:57:27 UTC 2024


On Thu, 4 Jul 2024 18:44:52 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:
> 
>   wild imports expand, imports sorting

> Imports are expanded and sorted as per recent review comments. I guess everything is fine now.

Nearly there.



> > This changeset enables hiding / showing mnemonics on JMenuBar only. Do you plan to update ButtonUI and LabelUI for GTK Look-and-Feel too?
> 
> Not as a part of this PR. It can be taken up as other bug.

Does it make sense to handle mnemonics in buttons and labels as a separate bug? I doesn't seem worth it.

You've done 98% of the job in this PR. Refactoring touched all the classes which hide mnemonics.

The only thing left is to add handling into `SynthButtonUI` and `SynthLabelUI` where you need to replace `b.getDisplayedMnemonicIndex()` and `label.getDisplayedMnemonicIndex()` with a conditional display based on `MnemonicHandler.isMnemonicHidden()`.

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

> 42: import javax.swing.JMenuBar;
> 43: import javax.swing.JMenuItem;
> 44: import javax.swing.JMenu;

Suggestion:

import javax.swing.JMenu;
import javax.swing.JMenuBar;
import javax.swing.JMenuItem;

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

> 1: /*

There are still unused imports in the list: `File`, `PrivilegedAction`, `Locale` and `OSInfo`.

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

> 44: import sun.swing.MenuItemLayoutHelper;
> 45: import sun.swing.SwingUtilities2;
> 46: import sun.swing.MnemonicHandler;

Suggestion:

import sun.swing.MenuItemLayoutHelper;
import sun.swing.MnemonicHandler;
import sun.swing.SwingUtilities2;

src/java.desktop/share/classes/sun/swing/MnemonicHandler.java line 75:

> 73:     /*
> 74:      * Repaints all the components with the mnemonics in the given window and all its owned windows.
> 75:      */

I suggest converting the comment to javadoc comment:
Suggestion:

    /**
     * Repaints all the components with the mnemonics in the given window and all its owned windows.
     */

src/java.desktop/share/classes/sun/swing/MnemonicHandler.java line 92:

> 90:      * Repaints all the components with the mnemonics in container.
> 91:      * Recursively searches for all the subcomponents.
> 92:      */

Convert this one to javadoc comment too.

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

> 36: import javax.swing.SwingUtilities;
> 37: import javax.swing.UIManager;
> 38: import javax.swing.plaf.synth.SynthLookAndFeel;

`Point` and `SynthLookAndFeel` aren't used in the test.

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18992#pullrequestreview-2160433537
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1666634817
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1666637877
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1666640228
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1666641169
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1666642504
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1666650816


More information about the client-libs-dev mailing list