Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v18]
On Mon, 8 Jul 2024 15:40:25 GMT, Abhishek Kumar 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:
>
> Typo in copyright header
src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsRootPaneUI.java
line 156:
> 154: path[1] = menu;
> 155: msm.setSelectedPath(path);
> 156: } else if(!MnemonicHandler.isMnemonicHidden()) {
Suggestion:
} else if (!MnemonicHandler.isMnemonicHidden()) {
test/jdk/javax/swing/JMenuBar/TestMenuMnemonicLinuxAndMac.java line 107:
> 105: private static void createAndShowUI() {
> 106: frame = new JFrame("Test Menu Mnemonic Show/Hide");
> 107: JMenuBar menuBar = new JMenuBar();
Suggestion:
JMenuBar menuBar = new JMenuBar();
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1673518878
PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1673519102
Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v18]
On Wed, 10 Jul 2024 23:35:59 GMT, Phil Race wrote: > This looks like it is much better than the first iteration and being able to > unify some code is good. The change looks "big" but seems to be mostly > because of refactoring. I suppose you tracked down all tests that need > updating ? Meaning manual as well as automated. Yeah, many files got changed due to code refactoring . Tests which failed in CI testing due to changes are updated and working fine now. Searched for the helper method reference in other tests (manual or automated) but seems there are no more tests that refer to old implementation. CI link is posted in JBS. - PR Comment: https://git.openjdk.org/jdk/pull/18992#issuecomment-040544
Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v18]
On Mon, 8 Jul 2024 15:40:25 GMT, Abhishek Kumar 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: > > Typo in copyright header This looks like it is much better than the first iteration and being able to unify some code is good. The change looks "big" but seems to be mostly because of refactoring. I suppose you tracked down all tests that need updating ? Meaning manual as well as automated. - PR Review: https://git.openjdk.org/jdk/pull/18992#pullrequestreview-2170570142
Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v18]
On Mon, 8 Jul 2024 15:40:25 GMT, Abhishek Kumar 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: > > Typo in copyright header LGTM - Marked as reviewed by tr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18992#pullrequestreview-2166337666
Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v18]
On Mon, 8 Jul 2024 15:37:04 GMT, Abhishek Kumar 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: > > Typo in copyright header Marked as reviewed by aivanov (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18992#pullrequestreview-2163688858
Re: RFR: 8155030: The Menu Mnemonics are always displayed for GTK LAF [v18]
> 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: Typo in copyright header - Changes: - all: https://git.openjdk.org/jdk/pull/18992/files - new: https://git.openjdk.org/jdk/pull/18992/files/a2265292..b6089477 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18992&range=17 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18992&range=16-17 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18992.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18992/head:pull/18992 PR: https://git.openjdk.org/jdk/pull/18992
