On Mon, 8 Jul 2024 09:39:11 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:
> 
>   wild import expand. SynthGraphicsUtils revert back

Adding a brief details for the changes in PR as it got multiple commits and 
will be hard to track those changes.

Initial proposed fix was to handle the **mnemonic show/hide on Alt press for 
GTK in Synth based classes** where

- `SynthLookAndFeel` class installed the AltProcessor handler after the 
conditional check of current L&F is GTK. public APIs `setMnemonicHidden and 
isMnemonicHidden` were added as part of fix.
- `SynthRootPaneUI` class is having the AltProcessor class to handle the press 
and release of ALT key press.
- `SynthGraphicsUtils` class is responsible for rendering the mnemonics based 
on the `show/hide status of isMnemonicHidden flag` for GTK L&F.

@prrace suggested that checking for GTK L&F is not an ideal way of 
implementation and we should look for other alternative mentioned 
[here](https://github.com/openjdk/jdk/pull/18992#discussion_r1595782003).

Based on the suggestion, `AltPressProperty` was added in `GTKLookAndFeel` to 
ensure the ALT key press event is handled only for GTK and thus moving the 
installation / uninstallation of ALTProcessor handler code in SynthRootPaneUI 
class.
**This fix required a CSR because of few public APIs which were added as a part 
of fix.**

Later on @aivanov-jdk suggested to move the **public APIs in a separate class 
MnemonicHandler (under sun.swing package) as the code was duplicated among 
Windows/Mac/Linux OSes** and a separate `AltProcessor class to handle the Alt 
key press/release for GTK and Aqua L&F` (same logic was implemented for them), 
Windows is having it's own version of AltProcessor.

Current implementation details are:

- Duplicate code removed, helper methods are moved to one common file 
(MnemonicHandler.java)
- Common AltProcessor handler class for GTK and Aqua (AltProcessor.java)
- Refactoring code for Windows and Aqua to access the helper methods from 
MnemonicHandler class and remove unused code.
- Mnemonic hidden condition check moved to `GTKGraphicsUtils` so that only GTK 
L&F will call `isMnemonicHidden` to avoid the performance issue. 
- No need of CSR as [sun.swing package is not 
public](https://github.com/openjdk/jdk/pull/18992#discussion_r1654417685)

**The fix is extended for Labels and Buttons as well along with Menus for GTK 
L&F.**
Further the wild imports were expanded wherever applicable.

Automated test enhanced to test menu mnemonic behavior for Aqua L&F also. Few 
existing regression tests also modified to access the helper methods from 
MnemonicHandler class.

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

PR Comment: https://git.openjdk.org/jdk/pull/18992#issuecomment-2214042122

Reply via email to