On Fri, 21 Jun 2024 07:55:24 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 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
