You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by varan123 <gi...@git.apache.org> on 2018/08/26 15:20:08 UTC
[GitHub] jmeter pull request #396: Fix to 62336
GitHub user varan123 opened a pull request:
https://github.com/apache/jmeter/pull/396
Fix to 62336
## Description
Keyboard shortcut Ctrl +6 is not working.
Was reproduced for Java 8 on Windows 10.
actionEvent.getActionCommand() in MainFrame unexpected returning null only for CTRL-6. getActionCommand was replaced with alternative code. New version working stable.
## Motivation and Context
All CTRL-{N} working stable, after fix.
https://bz.apache.org/bugzilla/show_bug.cgi?id=62336
## How Has This Been Tested?
Tested on Java 8 and Java 9 on Windows 10. Issue was reproduced. New code working fine.
## Screenshots (if appropriate):
## Types of changes
- Bug fix 62336. Was changed org.apache.jmeter.gui.MainFarme around line 686.
## Checklist:
Path Filename Extension Status Lines added Lines removed Last Modified Size
src/core/org/apache/jmeter/gui/MainFrame.java MainFrame.java .java Modified 12 1
[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/varan123/jmeter Bug_62336
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jmeter/pull/396.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #396
----
commit 1df6d1b56e8e73df58ee68981c7f2bbc19273a94
Author: Michael Pavlov <mi...@...>
Date: 2018-08-26T04:29:28Z
Fix to 62336
----
---
[GitHub] jmeter issue #396: Fix to 62336
Posted by FSchumacher <gi...@git.apache.org>.
Github user FSchumacher commented on the issue:
https://github.com/apache/jmeter/pull/396
Just wanted to let you know, that you are not forgotten. I have now access to a windows 10 system and can reproduce the issue. But before I implement your fix, I would like to understand why `ctrl`+`6`doesn't work, as all other keys seem to work OK.
---
[GitHub] jmeter pull request #396: Fix to 62336
Posted by FSchumacher <gi...@git.apache.org>.
Github user FSchumacher commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/396#discussion_r212836512
--- Diff: src/core/org/apache/jmeter/gui/MainFrame.java ---
@@ -680,7 +683,15 @@ private void addQuickComponentHotkeys(JTree treevar) {
@Override
public void actionPerformed(ActionEvent actionEvent) {
- String propname = "gui.quick_" + actionEvent.getActionCommand();
+ //Bug 62336
+ AWTEvent current_event = EventQueue.getCurrentEvent();
+ String key_text = "";
+ if(current_event instanceof KeyEvent) {
+ KeyEvent key_event = (KeyEvent)current_event;
--- End diff --
Here I would put a space after the closing parenthesis as this is a cast.
---
[GitHub] jmeter pull request #396: Fix to 62336
Posted by FSchumacher <gi...@git.apache.org>.
Github user FSchumacher commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/396#discussion_r212836415
--- Diff: src/core/org/apache/jmeter/gui/MainFrame.java ---
@@ -680,7 +683,15 @@ private void addQuickComponentHotkeys(JTree treevar) {
@Override
public void actionPerformed(ActionEvent actionEvent) {
- String propname = "gui.quick_" + actionEvent.getActionCommand();
+ //Bug 62336
+ AWTEvent current_event = EventQueue.getCurrentEvent();
+ String key_text = "";
+ if(current_event instanceof KeyEvent) {
--- End diff --
I would place a space between `if` and the opening parenthesis as `if` is not a function call.
---
[GitHub] jmeter issue #396: Fix to 62336
Posted by FSchumacher <gi...@git.apache.org>.
Github user FSchumacher commented on the issue:
https://github.com/apache/jmeter/pull/396
Thanks for your contribution.
I still don't know, why we get `null` for `CTRL`+`6` on windows, but the workaround seems to work, so I have included it and hope somebody can explain the real bug to me.
---
[GitHub] jmeter issue #396: Fix to 62336
Posted by varan123 <gi...@git.apache.org>.
Github user varan123 commented on the issue:
https://github.com/apache/jmeter/pull/396
current_event is KeyEvent. action_event is ActionEvent. KeyEvent and
ActonEvent two different types, but both from AWTEvent. Line: String
propname = "gui.quick_" + actionEvent.getActionCommand(); tries to prepare
property name where last part it is pressed key with digit, and not working
with CTRL-6. I just replaced, with code, which take key text directly from
KeyEvent.
np. I will rename variables.
On Sun, Aug 26, 2018 at 3:30 PM Felix Schumacher <no...@github.com>
wrote:
> *@FSchumacher* requested changes on this pull request.
>
> Thanks for your patch.
> I have one question and a few minor notes about code formatting.
> ------------------------------
>
> In src/core/org/apache/jmeter/gui/MainFrame.java
> <https://github.com/apache/jmeter/pull/396#discussion_r212836402>:
>
> > @@ -680,7 +683,15 @@ private void addQuickComponentHotkeys(JTree treevar) {
>
> @Override
> public void actionPerformed(ActionEvent actionEvent) {
> - String propname = "gui.quick_" + actionEvent.getActionCommand();
> + //Bug 62336
> + AWTEvent current_event = EventQueue.getCurrentEvent();
>
> Have you checked what actionEvent is here? If I read the Swing-API
> correctly, it should be the same object as current_event. If actionEvent
> is really null here, is it only sometimes null and the NPE stops the AWT
> thread?
> As a minor note: current_event should be written in camel case as
> currentEvent to match the other names.
> ------------------------------
>
> In src/core/org/apache/jmeter/gui/MainFrame.java
> <https://github.com/apache/jmeter/pull/396#discussion_r212836415>:
>
> > @@ -680,7 +683,15 @@ private void addQuickComponentHotkeys(JTree treevar) {
>
> @Override
> public void actionPerformed(ActionEvent actionEvent) {
> - String propname = "gui.quick_" + actionEvent.getActionCommand();
> + //Bug 62336
> + AWTEvent current_event = EventQueue.getCurrentEvent();
> + String key_text = "";
> + if(current_event instanceof KeyEvent) {
>
> I would place a space between if and the opening parenthesis as if is not
> a function call.
> ------------------------------
>
> In src/core/org/apache/jmeter/gui/MainFrame.java
> <https://github.com/apache/jmeter/pull/396#discussion_r212836472>:
>
> > @@ -680,7 +683,15 @@ private void addQuickComponentHotkeys(JTree treevar) {
>
> @Override
> public void actionPerformed(ActionEvent actionEvent) {
> - String propname = "gui.quick_" + actionEvent.getActionCommand();
> + //Bug 62336
> + AWTEvent current_event = EventQueue.getCurrentEvent();
> + String key_text = "";
> + if(current_event instanceof KeyEvent) {
> + KeyEvent key_event = (KeyEvent)current_event;
> + key_text = KeyEvent.getKeyText( key_event.getKeyCode() );
>
> Again a minor nit: No space needed after the opening and before the
> closing parenthesis as this is a function call.
> And as above variable names should be written in camel case: keyEvent
> instead of key_event.
> ------------------------------
>
> In src/core/org/apache/jmeter/gui/MainFrame.java
> <https://github.com/apache/jmeter/pull/396#discussion_r212836512>:
>
> > @@ -680,7 +683,15 @@ private void addQuickComponentHotkeys(JTree treevar) {
>
> @Override
> public void actionPerformed(ActionEvent actionEvent) {
> - String propname = "gui.quick_" + actionEvent.getActionCommand();
> + //Bug 62336
> + AWTEvent current_event = EventQueue.getCurrentEvent();
> + String key_text = "";
> + if(current_event instanceof KeyEvent) {
> + KeyEvent key_event = (KeyEvent)current_event;
>
> Here I would put a space after the closing parenthesis as this is a cast.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/jmeter/pull/396#pullrequestreview-149548048>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AglOptrf5wGhMvsxG1TeD4dhKPCM5dU3ks5uUvdcgaJpZM4WM0kk>
> .
>
---
[GitHub] jmeter issue #396: Fix to 62336
Posted by FSchumacher <gi...@git.apache.org>.
Github user FSchumacher commented on the issue:
https://github.com/apache/jmeter/pull/396
Right, the events will never be the same, but still... I wonder why `ActionEvent` is `null` in the first place. This seems to be a bug in the JDK, it doesn't make sense to call an action listener without a valid event.
Can you post a stacktrace when the event is `null` (and maybe return early from the function and see, if the next event in the action listener is the *correct* one)?
---
[GitHub] jmeter issue #396: Fix to 62336
Posted by varan123 <gi...@git.apache.org>.
Github user varan123 commented on the issue:
https://github.com/apache/jmeter/pull/396
I based my solution on discussion: https://stackoverflow.com/questions/32978936/why-actionevent-getactioncommand-returns-null
If we want to receive key value, have sense to receive it directly from KeyEvent.
Debug results:
Ctrl-6: propname value "gui.quick_null"
Ctrl-3: propname value "gui.quick_3"
Stacktrace:
at org.apache.jmeter.gui.MainFrame$2.actionPerformed(MainFrame.java:696)
at javax.swing.SwingUtilities.notifyAction(SwingUtilities.java:1663)
at javax.swing.JComponent.processKeyBinding(JComponent.java:2882)
at javax.swing.KeyboardManager.fireBinding(KeyboardManager.java:307)
at javax.swing.KeyboardManager.fireKeyboardAction(KeyboardManager.java:250)
at javax.swing.JComponent.processKeyBindingsForAllComponents(JComponent.java:2974)
at javax.swing.JComponent.processKeyBindings(JComponent.java:2966)
at javax.swing.JComponent.processKeyEvent(JComponent.java:2845)
at java.awt.Component.processEvent(Component.java:6310)
at java.awt.Container.processEvent(Container.java:2237)
at java.awt.Component.dispatchEventImpl(Component.java:4889)
at java.awt.Container.dispatchEventImpl(Container.java:2295)
at java.awt.Component.dispatchEvent(Component.java:4711)
at java.awt.KeyboardFocusManager.redispatchEvent(KeyboardFocusManager.java:1954)
at java.awt.DefaultKeyboardFocusManager.dispatchKeyEvent(DefaultKeyboardFocusManager.java:806)
at java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(DefaultKeyboardFocusManager.java:1074)
at java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(DefaultKeyboardFocusManager.java:945)
at java.awt.DefaultKeyboardFocusManager.dispatchEvent(DefaultKeyboardFocusManager.java:771)
at java.awt.Component.dispatchEventImpl(Component.java:4760)
at java.awt.Container.dispatchEventImpl(Container.java:2295)
at java.awt.Window.dispatchEventImpl(Window.java:2746)
at java.awt.Component.dispatchEvent(Component.java:4711)
at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758)
at java.awt.EventQueue.access$500(EventQueue.java:97)
at java.awt.EventQueue$3.run(EventQueue.java:709)
at java.awt.EventQueue$3.run(EventQueue.java:703)
at java.security.AccessController.doPrivileged(AccessController.java)
at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:80)
at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:90)
at java.awt.EventQueue$4.run(EventQueue.java:731)
at java.awt.EventQueue$4.run(EventQueue.java:729)
at java.security.AccessController.doPrivileged(AccessController.java)
at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:80)
at java.awt.EventQueue.dispatchEvent(EventQueue.java:728)
at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:201)
at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)
---
[GitHub] jmeter pull request #396: Fix to 62336
Posted by FSchumacher <gi...@git.apache.org>.
Github user FSchumacher commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/396#discussion_r212836402
--- Diff: src/core/org/apache/jmeter/gui/MainFrame.java ---
@@ -680,7 +683,15 @@ private void addQuickComponentHotkeys(JTree treevar) {
@Override
public void actionPerformed(ActionEvent actionEvent) {
- String propname = "gui.quick_" + actionEvent.getActionCommand();
+ //Bug 62336
+ AWTEvent current_event = EventQueue.getCurrentEvent();
--- End diff --
Have you checked what `actionEvent` is here? If I read the Swing-API correctly, it should be the same object as `current_event`. If `actionEvent` is really `null` here, is it only sometimes `null` and the NPE stops the AWT thread?
As a minor note: `current_event` should be written in camel case as `currentEvent` to match the other names.
---
[GitHub] jmeter pull request #396: Fix to 62336
Posted by FSchumacher <gi...@git.apache.org>.
Github user FSchumacher commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/396#discussion_r212836472
--- Diff: src/core/org/apache/jmeter/gui/MainFrame.java ---
@@ -680,7 +683,15 @@ private void addQuickComponentHotkeys(JTree treevar) {
@Override
public void actionPerformed(ActionEvent actionEvent) {
- String propname = "gui.quick_" + actionEvent.getActionCommand();
+ //Bug 62336
+ AWTEvent current_event = EventQueue.getCurrentEvent();
+ String key_text = "";
+ if(current_event instanceof KeyEvent) {
+ KeyEvent key_event = (KeyEvent)current_event;
+ key_text = KeyEvent.getKeyText( key_event.getKeyCode() );
--- End diff --
Again a minor nit: No space needed after the opening and before the closing parenthesis as this is a function call.
And as above variable names should be written in camel case: `keyEvent` instead of `key_event`.
---
[GitHub] jmeter issue #396: Fix to 62336
Posted by codecov-io <gi...@git.apache.org>.
Github user codecov-io commented on the issue:
https://github.com/apache/jmeter/pull/396
# [Codecov](https://codecov.io/gh/apache/jmeter/pull/396?src=pr&el=h1) Report
> Merging [#396](https://codecov.io/gh/apache/jmeter/pull/396?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/jmeter/commit/f8fb20aa9a711aa351a976098890bfedb97b235d?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/jmeter/pull/396?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #396 +/- ##
============================================
- Coverage 58.62% 58.61% -0.01%
Complexity 10602 10602
============================================
Files 1193 1193
Lines 75837 75842 +5
Branches 7330 7331 +1
============================================
Hits 44457 44457
- Misses 28869 28874 +5
Partials 2511 2511
```
| [Impacted Files](https://codecov.io/gh/apache/jmeter/pull/396?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [src/core/org/apache/jmeter/gui/MainFrame.java](https://codecov.io/gh/apache/jmeter/pull/396/diff?src=pr&el=tree#diff-c3JjL2NvcmUvb3JnL2FwYWNoZS9qbWV0ZXIvZ3VpL01haW5GcmFtZS5qYXZh) | `1.53% <0%> (-0.03%)` | `1 <0> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/jmeter/pull/396?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/jmeter/pull/396?src=pr&el=footer). Last update [f8fb20a...1df6d1b](https://codecov.io/gh/apache/jmeter/pull/396?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
---
[GitHub] jmeter pull request #396: Fix to 62336
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jmeter/pull/396
---