Looks fine.
On 23/06/2018 09:32, Pankaj Bansal wrote:
Hi Sergey,
<< This looks fine to me, the only suggestion is to try to extract the common
code to some new method in SwingUtil2.
Please have a look at the following code. I have added a function to set the
ALT_GRAPH mask on any modifier passed to it.
Webrev: http://cr.openjdk.java.net/~pbansal/8194873/webrev.06/
Regards
Pankaj Bansal
-----Original Message-----
From: Sergey Bylokhov
Sent: Friday, June 22, 2018 5:13 AM
To: Pankaj Bansal; Prasanta Sadhukhan
Cc: swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> [11] JDK-8194873: right ALT key hotkeys no longer work
in Swing components
This looks fine to me, the only suggestion is to try to extract the common code
to some new method in SwingUtil2.
On 21/06/2018 05:25, Pankaj Bansal wrote:
Hi Prasanta/Sergey
@Prasanta
<< The test passed for me in windows 10 even without your fix and the test
frames and dialogs are kept visible even after the test finished. Please have a look.
Sorry I think I was testing by printing log on console instead of throwing
exceptions and I made webrev with that version. I have corrected it now. The
test fails without fix and passes with fix on Windows 10.
@Sergey
<< Is this bug applicable only to windows? can we make the test non-windows
specific?
Yes, I think this bug addresses the regression introduced when the fix was made
for https://bugs.openjdk.java.net/browse/JDK-8041928. Both ALT and ALT_GRAPH
keys work on Windows. The test added fails on linux and mac both with and
without fix. I think the ALT_GRAPH key does not work on these platforms and it
is not related to the regression being addressed here. We can file separate bug
to investigate that. Please let me know what do you think regarding this.
I have run possible affected jck tests on Windows and all the automated event
tests in awt/swing jtreg tests on Windows, Linux and Mac. The fix is not
producing any regression.
webrev:
http://cr.openjdk.java.net/~pbansal/8194873/webrev.04/
Thanks,
Pankj Bansal
-----Original Message-----
From: Prasanta Sadhukhan
Sent: Thursday, June 21, 2018 11:58 AM
To: Pankaj Bansal
Cc: swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> [11] JDK-8194873: right ALT key hotkeys no
longer work in Swing components
Hi Pankaj,
The test passed for me in windows 10 even without your fix and the test frames
and dialogs are kept visible even after the test finished. Please have a look.
Regards
Prasanta
On 6/20/2018 8:22 PM, Pankaj Bansal wrote:
Hi Prasanta,
I looked at documentation of KeyStroke.getKeyStroke and InputEvent.ALT_MASK
should be used instead of ActionEvent.ALT_MASK. The corresponding
InputEvent.ALT_GRAPH_MASK is already present. I used it by mistake last time.
I have made changes for JMenuItem I have changed BasicJMenuItemUI for JMenuItem
and updated the test case too. Other changes are same as webrev.02. Please have
a look.
Webrev: http://cr.openjdk.java.net/~pbansal/8194873/webrev.03/
Regards,
Pankaj Bansal
-----Original Message-----
From: Prasanta Sadhukhan
Sent: Friday, April 20, 2018 5:04 PM
To: Pankaj Bansal
Cc: swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> [11] JDK-8194873: right ALT key hotkeys no
longer work in Swing components
Hi Pankaj,
On 4/20/2018 3:48 PM, Pankaj Bansal wrote:
Hi Prasanta,
In case of JMenuItem, there are two ways to add key shortcuts
1.
JMenuItem newMenuItem = new JMenuItem("New");
newMenuItem.setMnemonic(KeyEvent.VK_N);
In this case, the event is triggered if the N key is pressed. The modifiers are
not used in this and pressing only N will work. So ALT or ALT_GRAPH is not
required. These shortcut only work if the menu containing the given menu item
is expanded.
2.
JMenuItem newMenuItem = new JMenuItem("New");
newMenuItem.setAccelerator(KeyStroke.getKeyStroke( KeyEvent.VK_N,
ActionEvent.ALT_MASK)); In this case we are setting the accelerator key. In
this case the key N will work with the modifier passed here. So the user is
explicitly telling whether to use ALT, ALT_GRAPH or ALT+ALT+GRAPH. So I think
we don’t need to change anything here. The menu containing the menu item does
not have to be expanded in this case.
But, as far I see in spec, ActionEvent does not have anyway to specify Right
ALT mask or ALT_GRAPH_MASK so how the user can tell to use ALT_GRAPH.
Regards
Prasanta
Please let me know what do you think about this.
Regards,
Pankaj Bansal
-----Original Message-----
From: Prasanta Sadhukhan
Sent: Thursday, April 19, 2018 12:41 PM
To: Pankaj Bansal; Andrej Golovnin
Cc: Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> [11] JDK-8194873: right ALT key hotkeys no
longer work in Swing components
Hi Pankaj,
looks good. but it still does not test JMenuItem as I can see. Did you check if
you have some menu items inside JMenu and set mnemonic, does Right Alt key
works?
Regards
Prasanta
On 4/10/2018 3:15 PM, Pankaj Bansal wrote:
Hello Andrej,
Thanks for the quick review. Yes, it does not sense to apply || on same value.
It was a typo. Thanks for pointing it out.
Webrev:
http://cr.openjdk.java.net/~pbansal/8194873/webrev.02/
Regards,
Pankaj Bansal
-----Original Message-----
From: Andrej Golovnin [mailto:andrej.golov...@gmail.com]
Sent: Tuesday, April 10, 2018 2:18 PM
To: Pankaj Bansal
Cc: Prasanta Sadhukhan; Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> [11] JDK-8194873: right ALT key hotkeys no
longer work in Swing components
Hi Pankaj,
Webrev:
http://cr.openjdk.java.net/~pbansal/8194873/webrev.01/
src/java.desktop/windows/native/libawt/windows/awt_Component.cpp
3540 BOOL altIsDown = ((modifiers &
java_awt_event_InputEvent_ALT_DOWN_MASK) ||
3541 (modifiers &
java_awt_event_InputEvent_ALT_DOWN_MASK));
Applying '||' on the same value does not make sense. I think the
line
3541 should use 'java_awt_event_InputEvent_ALT_GRAPH_DOWN_MASK':
3541 (modifiers &
java_awt_event_InputEvent_ALT_GRAPH_DOWN_MASK));
Best regards,
Andrej Golovnin
--
Best regards, Sergey.
--
Best regards, Sergey.
