Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Tue, 26 Nov 2019 13:06:38 GMT, Florian Kirmaier
wrote:
> On Tue, 26 Nov 2019 09:24:01 GMT, Kevin Rushforth wrote:
>
>> On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier
>> wrote:
>>
>>> Original PR: https://github.com/javafxports/openjdk-jfx/pull/591
>>>
>>> A side effect of JDK-8200224 is, that the first click on a JFXPanel is
>>> always a double click.
>>> The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in
>>> JFXPanel ignored if another swing component has focus).
>>> This fix introduced synthesized press-events, which is an unsafe fix for
>>> the problem.
>>>
>>> The pull request introduces a new fix for the problem.
>>> Instead of simulating new input-events, we set JavaFX Scene/Window to
>>> focused.
>>> We do so, by using setFocused.
>>> When the original Swing-Focus event is called slightly later, it won't have
>>> any side-effects,
>>> because calling setFocused just sets the value of a boolean property.
>>>
>>> I've now also added a unit-test for the problem.
>>>
>>>
>>>
>>> Commits:
>>> - 314e423c: JDK-8200224
>>> - 725e5fef: JDK-8200224
>>>
>>> Changes: https://git.openjdk.java.net/jfx/pull/25/files
>>> Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8200224
>>> Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod
>>> Patch: https://git.openjdk.java.net/jfx/pull/25.diff
>>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25
>>
>> I left a couple additional comments about the test changes. Namely:
>>
>> 1. You didn't fully revert the changes to `SwingFXUtilsTest`
>> 2. Your new test should be put in its own class in `test.javafx.embed.swing`
>> (and not in the exist mac-only Robot test)
>>
>> tests/system/src/test/java/test/javafx/embed/swing/SwingFXUtilsTest.java
>> line 87:
>>
>>> 86: testFromFXImg("opaque.gif");
>>> 87: testFromFXImg("opaque.jpg");
>>> 88: testFromFXImg("opaque.png");
>>
>> You didn't fully revert your change to this file. The above needs to be
>> restored such that when you are done, this file is unchanged versus master,
>> and not part of the PR.
>>
>> tests/system/src/test/java/test/robot/javafx/embed/swing/JFXPanelTest.java
>> line 25:
>>
>>> 24: */
>>> 25: package test.robot.javafx.embed.swing;
>>> 26:
>>
>> Since you aren't using `Robot` I'll repeat my earlier recommendation to put
>> your new test in `test.javafx.embed.swing` (as a new test class) rather than
>> modifying this existing test class. This class isn't suitable anyway, since
>> it is only run on Mac.
>>
>> tests/system/src/test/java/test/robot/javafx/embed/swing/JFXPanelTest.java
>> line 40:
>>
>>> 39: import javax.swing.*;
>>> 40: import java.awt.*;
>>> 41: import java.awt.event.InputEvent;
>>
>> The use of wild card imports is discouraged (except for static imports).
>>
>> tests/system/src/test/java/test/robot/javafx/embed/swing/JFXPanelTest.java
>> line 129:
>>
>>> 128: JPanel jpanel = new JPanel();
>>> 129: jpanel.add(fxPnl);
>>> 130: jframe.setContentPane(jpanel);
>>
>> You didn't add an initial dummy `JFXPanel` so I suspect it will still not
>> catch the bug (meaning it will still pass even without your patch).
>>
>>
>>
>> Changes requested by kcr (Lead).
>
> 1. I've restored the test. But the git history is now very chaotic.
> Especially the moves might cause problems. Do the commits get squashed after
> merging? Otherwise, it might make sense, to redo the changes on a fresh
> branch and create a new pull request.
> 2. The test works now on windows. : )
Yes, the commits are squashed, so don't worry about the history (in worst case
I would ask you to do a squash-rebase / force push rather than create a new PR,
but that isn't needed here).
At first glance the changes you made look good. I'll take a closer look later.
@prsadhuk please also review this.
PR: https://git.openjdk.java.net/jfx/pull/25
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Tue, 26 Nov 2019 09:24:01 GMT, Kevin Rushforth wrote:
> On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier
> wrote:
>
>> Original PR: https://github.com/javafxports/openjdk-jfx/pull/591
>>
>> A side effect of JDK-8200224 is, that the first click on a JFXPanel is
>> always a double click.
>> The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in
>> JFXPanel ignored if another swing component has focus).
>> This fix introduced synthesized press-events, which is an unsafe fix for the
>> problem.
>>
>> The pull request introduces a new fix for the problem.
>> Instead of simulating new input-events, we set JavaFX Scene/Window to
>> focused.
>> We do so, by using setFocused.
>> When the original Swing-Focus event is called slightly later, it won't have
>> any side-effects,
>> because calling setFocused just sets the value of a boolean property.
>>
>> I've now also added a unit-test for the problem.
>>
>>
>>
>> Commits:
>> - 314e423c: JDK-8200224
>> - 725e5fef: JDK-8200224
>>
>> Changes: https://git.openjdk.java.net/jfx/pull/25/files
>> Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8200224
>> Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod
>> Patch: https://git.openjdk.java.net/jfx/pull/25.diff
>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25
>
> I left a couple additional comments about the test changes. Namely:
>
> 1. You didn't fully revert the changes to `SwingFXUtilsTest`
> 2. Your new test should be put in its own class in `test.javafx.embed.swing`
> (and not in the exist mac-only Robot test)
>
> tests/system/src/test/java/test/javafx/embed/swing/SwingFXUtilsTest.java line
> 87:
>
>> 86: testFromFXImg("opaque.gif");
>> 87: testFromFXImg("opaque.jpg");
>> 88: testFromFXImg("opaque.png");
>
> You didn't fully revert your change to this file. The above needs to be
> restored such that when you are done, this file is unchanged versus master,
> and not part of the PR.
>
> tests/system/src/test/java/test/robot/javafx/embed/swing/JFXPanelTest.java
> line 25:
>
>> 24: */
>> 25: package test.robot.javafx.embed.swing;
>> 26:
>
> Since you aren't using `Robot` I'll repeat my earlier recommendation to put
> your new test in `test.javafx.embed.swing` (as a new test class) rather than
> modifying this existing test class. This class isn't suitable anyway, since
> it is only run on Mac.
>
> tests/system/src/test/java/test/robot/javafx/embed/swing/JFXPanelTest.java
> line 40:
>
>> 39: import javax.swing.*;
>> 40: import java.awt.*;
>> 41: import java.awt.event.InputEvent;
>
> The use of wild card imports is discouraged (except for static imports).
>
> tests/system/src/test/java/test/robot/javafx/embed/swing/JFXPanelTest.java
> line 129:
>
>> 128: JPanel jpanel = new JPanel();
>> 129: jpanel.add(fxPnl);
>> 130: jframe.setContentPane(jpanel);
>
> You didn't add an initial dummy `JFXPanel` so I suspect it will still not
> catch the bug (meaning it will still pass even without your patch).
>
>
>
> Changes requested by kcr (Lead).
1. I've restored the test. But the git history is now very chaotic. Especially
the moves might cause problems. Do the commits get squashed after merging?
Otherwise, it might make sense, to redo the changes on a fresh branch and
create a new pull request.
2. The test works now on windows. : )
PR: https://git.openjdk.java.net/jfx/pull/25
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier
wrote:
> Original PR: https://github.com/javafxports/openjdk-jfx/pull/591
>
> A side effect of JDK-8200224 is, that the first click on a JFXPanel is always
> a double click.
> The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in
> JFXPanel ignored if another swing component has focus).
> This fix introduced synthesized press-events, which is an unsafe fix for the
> problem.
>
> The pull request introduces a new fix for the problem.
> Instead of simulating new input-events, we set JavaFX Scene/Window to focused.
> We do so, by using setFocused.
> When the original Swing-Focus event is called slightly later, it won't have
> any side-effects,
> because calling setFocused just sets the value of a boolean property.
>
> I've now also added a unit-test for the problem.
>
>
>
> Commits:
> - 314e423c: JDK-8200224
> - 725e5fef: JDK-8200224
>
> Changes: https://git.openjdk.java.net/jfx/pull/25/files
> Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00
> Issue: https://bugs.openjdk.java.net/browse/JDK-8200224
> Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod
> Patch: https://git.openjdk.java.net/jfx/pull/25.diff
> Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25
I left a couple additional comments about the test changes. Namely:
1. You didn't fully revert the changes to `SwingFXUtilsTest`
2. Your new test should be put in its own class in `test.javafx.embed.swing`
(and not in the exist mac-only Robot test)
tests/system/src/test/java/test/javafx/embed/swing/SwingFXUtilsTest.java line
87:
> 86: testFromFXImg("opaque.gif");
> 87: testFromFXImg("opaque.jpg");
> 88: testFromFXImg("opaque.png");
You didn't fully revert your change to this file. The above needs to be
restored such that when you are done, this file is unchanged versus master, and
not part of the PR.
tests/system/src/test/java/test/robot/javafx/embed/swing/JFXPanelTest.java line
25:
> 24: */
> 25: package test.robot.javafx.embed.swing;
> 26:
Since you aren't using `Robot` I'll repeat my earlier recommendation to put
your new test in `test.javafx.embed.swing` (as a new test class) rather than
modifying this existing test class. This class isn't suitable anyway, since it
is only run on Mac.
tests/system/src/test/java/test/robot/javafx/embed/swing/JFXPanelTest.java line
40:
> 39: import javax.swing.*;
> 40: import java.awt.*;
> 41: import java.awt.event.InputEvent;
The use of wild card imports is discouraged (except for static imports).
tests/system/src/test/java/test/robot/javafx/embed/swing/JFXPanelTest.java line
129:
> 128: JPanel jpanel = new JPanel();
> 129: jpanel.add(fxPnl);
> 130: jframe.setContentPane(jpanel);
You didn't add an initial dummy `JFXPanel` so I suspect it will still not catch
the bug (meaning it will still pass even without your patch).
Changes requested by kcr (Lead).
PR: https://git.openjdk.java.net/jfx/pull/25
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Tue, 26 Nov 2019 09:23:59 GMT, Florian Kirmaier
wrote:
> On Wed, 13 Nov 2019 22:11:14 GMT, Kevin Rushforth wrote:
>
>> On Wed, 13 Nov 2019 01:17:44 GMT, Kevin Rushforth wrote:
>>
>>> On Tue, 12 Nov 2019 10:35:36 GMT, Florian Kirmaier
>>> wrote:
>>>
On Wed, 6 Nov 2019 08:30:47 GMT, Kevin Rushforth wrote:
> On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier
> wrote:
>
>> Original PR: https://github.com/javafxports/openjdk-jfx/pull/591
>>
>> A side effect of JDK-8200224 is, that the first click on a JFXPanel is
>> always a double click.
>> The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu
>> in JFXPanel ignored if another swing component has focus).
>> This fix introduced synthesized press-events, which is an unsafe fix for
>> the problem.
>>
>> The pull request introduces a new fix for the problem.
>> Instead of simulating new input-events, we set JavaFX Scene/Window to
>> focused.
>> We do so, by using setFocused.
>> When the original Swing-Focus event is called slightly later, it won't
>> have any side-effects,
>> because calling setFocused just sets the value of a boolean property.
>>
>> I've now also added a unit-test for the problem.
>>
>>
>>
>> Commits:
>> - 314e423c: JDK-8200224
>> - 725e5fef: JDK-8200224
>>
>> Changes: https://git.openjdk.java.net/jfx/pull/25/files
>> Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8200224
>> Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod
>> Patch: https://git.openjdk.java.net/jfx/pull/25.diff
>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25
>
> modules/javafx.swing/src/main/java/javafx/embed/swing/JFXPanel.java line
> 457:
>
>> 456:
>> 457: sendMouseEventToFX(e);
>> 458: super.processMouseEvent(e);
>
> typo: save --> safe
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 2:
>
>> 1: /*
>> 2: * Copyright (c) 2014, 2016 Oracle and/or its affiliates. All rights
>> reserved.
>> 3: * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>
> replace `2014, 2016` with `2019,`
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 28:
>
>> 27:
>> 28: import javafx.application.Application;
>> 29: import javafx.application.Platform;
>
> Remove unused import (several of the below imports are similarly unused).
> Also, since this is a new test, please sort the imports.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 48:
>
>> 47:
>> 48: import static junit.framework.Assert.assertEquals;
>> 49: import static org.junit.Assert.assertNotNull;
>
> This method should be imported from `org.junit` package.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 116:
>
>> 115: MouseEvent e = new MouseEvent(fxPnl,
>> MouseEvent.MOUSE_PRESSED, 0, MouseEvent.BUTTON1_DOWN_MASK,
>> 116: 5, 5, 1, false, MouseEvent.BUTTON1);
>> 117:
>
> This doesn't appear to trigger the bug -- at least on Windows. The test
> passes for me with or without your fix. You might consider moving it to
> the system tests project, under `tests/system/src/test/java/test/robot`
> and using `Robot` to effect the mouse press.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 57:
>
>> 56:
>> 57:
>> 58: @BeforeClass
>
> You can remove the extra blank line.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 65:
>
>> 64:
>> 65:
>> 66: try {
>
> You can remove the extra blank line.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 79:
>
>> 78:
>> 79: class TestFXPanel extends JFXPanel {
>> 80: protected void processMouseEventPublic(MouseEvent e) {
>
> I think this can be a static class.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 88:
>
>> 87:
>> 88: CountDownLatch firstPressedEventLatch = new
>> CountDownLatch(1);
>> 89:
>
> This can be final.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 91:
>
>> 90: // It's an array, so we can mutate it inside of lambda
>> statement
>> 91: int[] pressedEventCounter = {0};
>> 92:
>
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Wed, 13 Nov 2019 01:17:44 GMT, Kevin Rushforth wrote:
> On Tue, 12 Nov 2019 10:35:36 GMT, Florian Kirmaier
> wrote:
>
>> On Wed, 6 Nov 2019 08:30:47 GMT, Kevin Rushforth wrote:
>>
>>> On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier
>>> wrote:
>>>
Original PR: https://github.com/javafxports/openjdk-jfx/pull/591
A side effect of JDK-8200224 is, that the first click on a JFXPanel is
always a double click.
The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in
JFXPanel ignored if another swing component has focus).
This fix introduced synthesized press-events, which is an unsafe fix for
the problem.
The pull request introduces a new fix for the problem.
Instead of simulating new input-events, we set JavaFX Scene/Window to
focused.
We do so, by using setFocused.
When the original Swing-Focus event is called slightly later, it won't
have any side-effects,
because calling setFocused just sets the value of a boolean property.
I've now also added a unit-test for the problem.
Commits:
- 314e423c: JDK-8200224
- 725e5fef: JDK-8200224
Changes: https://git.openjdk.java.net/jfx/pull/25/files
Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00
Issue: https://bugs.openjdk.java.net/browse/JDK-8200224
Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod
Patch: https://git.openjdk.java.net/jfx/pull/25.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25
>>>
>>> modules/javafx.swing/src/main/java/javafx/embed/swing/JFXPanel.java line
>>> 457:
>>>
456:
457: sendMouseEventToFX(e);
458: super.processMouseEvent(e);
>>>
>>> typo: save --> safe
>>>
>>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>>> line 2:
>>>
1: /*
2: * Copyright (c) 2014, 2016 Oracle and/or its affiliates. All rights
reserved.
3: * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>
>>> replace `2014, 2016` with `2019,`
>>>
>>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>>> line 28:
>>>
27:
28: import javafx.application.Application;
29: import javafx.application.Platform;
>>>
>>> Remove unused import (several of the below imports are similarly unused).
>>> Also, since this is a new test, please sort the imports.
>>>
>>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>>> line 48:
>>>
47:
48: import static junit.framework.Assert.assertEquals;
49: import static org.junit.Assert.assertNotNull;
>>>
>>> This method should be imported from `org.junit` package.
>>>
>>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>>> line 116:
>>>
115: MouseEvent e = new MouseEvent(fxPnl,
MouseEvent.MOUSE_PRESSED, 0, MouseEvent.BUTTON1_DOWN_MASK,
116: 5, 5, 1, false, MouseEvent.BUTTON1);
117:
>>>
>>> This doesn't appear to trigger the bug -- at least on Windows. The test
>>> passes for me with or without your fix. You might consider moving it to the
>>> system tests project, under `tests/system/src/test/java/test/robot` and
>>> using `Robot` to effect the mouse press.
>>>
>>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>>> line 57:
>>>
56:
57:
58: @BeforeClass
>>>
>>> You can remove the extra blank line.
>>>
>>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>>> line 65:
>>>
64:
65:
66: try {
>>>
>>> You can remove the extra blank line.
>>>
>>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>>> line 79:
>>>
78:
79: class TestFXPanel extends JFXPanel {
80: protected void processMouseEventPublic(MouseEvent e) {
>>>
>>> I think this can be a static class.
>>>
>>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>>> line 88:
>>>
87:
88: CountDownLatch firstPressedEventLatch = new CountDownLatch(1);
89:
>>>
>>> This can be final.
>>>
>>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>>> line 91:
>>>
90: // It's an array, so we can mutate it inside of lambda
statement
91: int[] pressedEventCounter = {0};
92:
>>>
>>> This can be final.
>>>
>>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>>> line 132:
>>>
131:
132:
133: }
>>>
>>> You can remove the extra blank line.
>>>
>>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>>> line 123:
>>>
122:
123: if(!firstPressedEventLatch.await(5000,
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Tue, 12 Nov 2019 10:35:36 GMT, Florian Kirmaier
wrote:
> On Wed, 6 Nov 2019 08:30:47 GMT, Kevin Rushforth wrote:
>
>> On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier
>> wrote:
>>
>>> Original PR: https://github.com/javafxports/openjdk-jfx/pull/591
>>>
>>> A side effect of JDK-8200224 is, that the first click on a JFXPanel is
>>> always a double click.
>>> The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in
>>> JFXPanel ignored if another swing component has focus).
>>> This fix introduced synthesized press-events, which is an unsafe fix for
>>> the problem.
>>>
>>> The pull request introduces a new fix for the problem.
>>> Instead of simulating new input-events, we set JavaFX Scene/Window to
>>> focused.
>>> We do so, by using setFocused.
>>> When the original Swing-Focus event is called slightly later, it won't have
>>> any side-effects,
>>> because calling setFocused just sets the value of a boolean property.
>>>
>>> I've now also added a unit-test for the problem.
>>>
>>>
>>>
>>> Commits:
>>> - 314e423c: JDK-8200224
>>> - 725e5fef: JDK-8200224
>>>
>>> Changes: https://git.openjdk.java.net/jfx/pull/25/files
>>> Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8200224
>>> Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod
>>> Patch: https://git.openjdk.java.net/jfx/pull/25.diff
>>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25
>>
>> modules/javafx.swing/src/main/java/javafx/embed/swing/JFXPanel.java line 457:
>>
>>> 456:
>>> 457: sendMouseEventToFX(e);
>>> 458: super.processMouseEvent(e);
>>
>> typo: save --> safe
>>
>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>> line 2:
>>
>>> 1: /*
>>> 2: * Copyright (c) 2014, 2016 Oracle and/or its affiliates. All rights
>>> reserved.
>>> 3: * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>
>> replace `2014, 2016` with `2019,`
>>
>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>> line 28:
>>
>>> 27:
>>> 28: import javafx.application.Application;
>>> 29: import javafx.application.Platform;
>>
>> Remove unused import (several of the below imports are similarly unused).
>> Also, since this is a new test, please sort the imports.
>>
>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>> line 48:
>>
>>> 47:
>>> 48: import static junit.framework.Assert.assertEquals;
>>> 49: import static org.junit.Assert.assertNotNull;
>>
>> This method should be imported from `org.junit` package.
>>
>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>> line 116:
>>
>>> 115: MouseEvent e = new MouseEvent(fxPnl,
>>> MouseEvent.MOUSE_PRESSED, 0, MouseEvent.BUTTON1_DOWN_MASK,
>>> 116: 5, 5, 1, false, MouseEvent.BUTTON1);
>>> 117:
>>
>> This doesn't appear to trigger the bug -- at least on Windows. The test
>> passes for me with or without your fix. You might consider moving it to the
>> system tests project, under `tests/system/src/test/java/test/robot` and
>> using `Robot` to effect the mouse press.
>>
>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>> line 57:
>>
>>> 56:
>>> 57:
>>> 58: @BeforeClass
>>
>> You can remove the extra blank line.
>>
>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>> line 65:
>>
>>> 64:
>>> 65:
>>> 66: try {
>>
>> You can remove the extra blank line.
>>
>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>> line 79:
>>
>>> 78:
>>> 79: class TestFXPanel extends JFXPanel {
>>> 80: protected void processMouseEventPublic(MouseEvent e) {
>>
>> I think this can be a static class.
>>
>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>> line 88:
>>
>>> 87:
>>> 88: CountDownLatch firstPressedEventLatch = new CountDownLatch(1);
>>> 89:
>>
>> This can be final.
>>
>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>> line 91:
>>
>>> 90: // It's an array, so we can mutate it inside of lambda statement
>>> 91: int[] pressedEventCounter = {0};
>>> 92:
>>
>> This can be final.
>>
>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>> line 132:
>>
>>> 131:
>>> 132:
>>> 133: }
>>
>> You can remove the extra blank line.
>>
>> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
>> line 123:
>>
>>> 122:
>>> 123: if(!firstPressedEventLatch.await(5000, TimeUnit.MILLISECONDS))
>>> {
>>> 124: throw new Exception();
>>
>> Add a space after the `if`.
>>
>> The fix seems fine. Have you tested it on all three platforms?
>>
>> I have several comments on the test.
>>
>>
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Wed, 6 Nov 2019 08:30:47 GMT, Kevin Rushforth wrote:
> On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier
> wrote:
>
>> Original PR: https://github.com/javafxports/openjdk-jfx/pull/591
>>
>> A side effect of JDK-8200224 is, that the first click on a JFXPanel is
>> always a double click.
>> The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in
>> JFXPanel ignored if another swing component has focus).
>> This fix introduced synthesized press-events, which is an unsafe fix for the
>> problem.
>>
>> The pull request introduces a new fix for the problem.
>> Instead of simulating new input-events, we set JavaFX Scene/Window to
>> focused.
>> We do so, by using setFocused.
>> When the original Swing-Focus event is called slightly later, it won't have
>> any side-effects,
>> because calling setFocused just sets the value of a boolean property.
>>
>> I've now also added a unit-test for the problem.
>>
>>
>>
>> Commits:
>> - 314e423c: JDK-8200224
>> - 725e5fef: JDK-8200224
>>
>> Changes: https://git.openjdk.java.net/jfx/pull/25/files
>> Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8200224
>> Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod
>> Patch: https://git.openjdk.java.net/jfx/pull/25.diff
>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25
>
> modules/javafx.swing/src/main/java/javafx/embed/swing/JFXPanel.java line 457:
>
>> 456:
>> 457: sendMouseEventToFX(e);
>> 458: super.processMouseEvent(e);
>
> typo: save --> safe
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 2:
>
>> 1: /*
>> 2: * Copyright (c) 2014, 2016 Oracle and/or its affiliates. All rights
>> reserved.
>> 3: * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>
> replace `2014, 2016` with `2019,`
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 28:
>
>> 27:
>> 28: import javafx.application.Application;
>> 29: import javafx.application.Platform;
>
> Remove unused import (several of the below imports are similarly unused).
> Also, since this is a new test, please sort the imports.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 48:
>
>> 47:
>> 48: import static junit.framework.Assert.assertEquals;
>> 49: import static org.junit.Assert.assertNotNull;
>
> This method should be imported from `org.junit` package.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 116:
>
>> 115: MouseEvent e = new MouseEvent(fxPnl,
>> MouseEvent.MOUSE_PRESSED, 0, MouseEvent.BUTTON1_DOWN_MASK,
>> 116: 5, 5, 1, false, MouseEvent.BUTTON1);
>> 117:
>
> This doesn't appear to trigger the bug -- at least on Windows. The test
> passes for me with or without your fix. You might consider moving it to the
> system tests project, under `tests/system/src/test/java/test/robot` and using
> `Robot` to effect the mouse press.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 57:
>
>> 56:
>> 57:
>> 58: @BeforeClass
>
> You can remove the extra blank line.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 65:
>
>> 64:
>> 65:
>> 66: try {
>
> You can remove the extra blank line.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 79:
>
>> 78:
>> 79: class TestFXPanel extends JFXPanel {
>> 80: protected void processMouseEventPublic(MouseEvent e) {
>
> I think this can be a static class.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 88:
>
>> 87:
>> 88: CountDownLatch firstPressedEventLatch = new CountDownLatch(1);
>> 89:
>
> This can be final.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 91:
>
>> 90: // It's an array, so we can mutate it inside of lambda statement
>> 91: int[] pressedEventCounter = {0};
>> 92:
>
> This can be final.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 132:
>
>> 131:
>> 132:
>> 133: }
>
> You can remove the extra blank line.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 123:
>
>> 122:
>> 123: if(!firstPressedEventLatch.await(5000, TimeUnit.MILLISECONDS)) {
>> 124: throw new Exception();
>
> Add a space after the `if`.
>
> The fix seems fine. Have you tested it on all three platforms?
>
> I have several comments on the test.
>
>
>
> Disapproved by kcr (Lead).
Maybe try `./gradlew -PFULL_TEST=true swing:clean swing:test`.
I'm sure, it can be reproduced on windows.
If you still can not reproduce it, then I will add a test for the Robot.
PR:
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Wed, 6 Nov 2019 08:30:47 GMT, Kevin Rushforth wrote:
> On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier
> wrote:
>
>> Original PR: https://github.com/javafxports/openjdk-jfx/pull/591
>>
>> A side effect of JDK-8200224 is, that the first click on a JFXPanel is
>> always a double click.
>> The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in
>> JFXPanel ignored if another swing component has focus).
>> This fix introduced synthesized press-events, which is an unsafe fix for the
>> problem.
>>
>> The pull request introduces a new fix for the problem.
>> Instead of simulating new input-events, we set JavaFX Scene/Window to
>> focused.
>> We do so, by using setFocused.
>> When the original Swing-Focus event is called slightly later, it won't have
>> any side-effects,
>> because calling setFocused just sets the value of a boolean property.
>>
>> I've now also added a unit-test for the problem.
>>
>>
>>
>> Commits:
>> - 314e423c: JDK-8200224
>> - 725e5fef: JDK-8200224
>>
>> Changes: https://git.openjdk.java.net/jfx/pull/25/files
>> Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8200224
>> Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod
>> Patch: https://git.openjdk.java.net/jfx/pull/25.diff
>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25
>
> modules/javafx.swing/src/main/java/javafx/embed/swing/JFXPanel.java line 457:
>
>> 456:
>> 457: sendMouseEventToFX(e);
>> 458: super.processMouseEvent(e);
>
> typo: save --> safe
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 2:
>
>> 1: /*
>> 2: * Copyright (c) 2014, 2016 Oracle and/or its affiliates. All rights
>> reserved.
>> 3: * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>
> replace `2014, 2016` with `2019,`
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 28:
>
>> 27:
>> 28: import javafx.application.Application;
>> 29: import javafx.application.Platform;
>
> Remove unused import (several of the below imports are similarly unused).
> Also, since this is a new test, please sort the imports.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 48:
>
>> 47:
>> 48: import static junit.framework.Assert.assertEquals;
>> 49: import static org.junit.Assert.assertNotNull;
>
> This method should be imported from `org.junit` package.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 116:
>
>> 115: MouseEvent e = new MouseEvent(fxPnl,
>> MouseEvent.MOUSE_PRESSED, 0, MouseEvent.BUTTON1_DOWN_MASK,
>> 116: 5, 5, 1, false, MouseEvent.BUTTON1);
>> 117:
>
> This doesn't appear to trigger the bug -- at least on Windows. The test
> passes for me with or without your fix. You might consider moving it to the
> system tests project, under `tests/system/src/test/java/test/robot` and using
> `Robot` to effect the mouse press.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 57:
>
>> 56:
>> 57:
>> 58: @BeforeClass
>
> You can remove the extra blank line.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 65:
>
>> 64:
>> 65:
>> 66: try {
>
> You can remove the extra blank line.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 79:
>
>> 78:
>> 79: class TestFXPanel extends JFXPanel {
>> 80: protected void processMouseEventPublic(MouseEvent e) {
>
> I think this can be a static class.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 88:
>
>> 87:
>> 88: CountDownLatch firstPressedEventLatch = new CountDownLatch(1);
>> 89:
>
> This can be final.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 91:
>
>> 90: // It's an array, so we can mutate it inside of lambda statement
>> 91: int[] pressedEventCounter = {0};
>> 92:
>
> This can be final.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 132:
>
>> 131:
>> 132:
>> 133: }
>
> You can remove the extra blank line.
>
> modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
> line 123:
>
>> 122:
>> 123: if(!firstPressedEventLatch.await(5000, TimeUnit.MILLISECONDS)) {
>> 124: throw new Exception();
>
> Add a space after the `if`.
>
> The fix seems fine. Have you tested it on all three platforms?
>
> I have several comments on the test.
>
>
>
> Disapproved by kcr (Lead).
You can run the AWT_TESTS with the following statement:
./gradlew -PFULL_TEST=true swing:clean swing:test
For some reason, it's hidden behind this flag.
Maybe that's the reason, why you
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier
wrote:
> Original PR: https://github.com/javafxports/openjdk-jfx/pull/591
>
> A side effect of JDK-8200224 is, that the first click on a JFXPanel is always
> a double click.
> The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in
> JFXPanel ignored if another swing component has focus).
> This fix introduced synthesized press-events, which is an unsafe fix for the
> problem.
>
> The pull request introduces a new fix for the problem.
> Instead of simulating new input-events, we set JavaFX Scene/Window to focused.
> We do so, by using setFocused.
> When the original Swing-Focus event is called slightly later, it won't have
> any side-effects,
> because calling setFocused just sets the value of a boolean property.
>
> I've now also added a unit-test for the problem.
>
>
>
> Commits:
> - 314e423c: JDK-8200224
> - 725e5fef: JDK-8200224
>
> Changes: https://git.openjdk.java.net/jfx/pull/25/files
> Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00
> Issue: https://bugs.openjdk.java.net/browse/JDK-8200224
> Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod
> Patch: https://git.openjdk.java.net/jfx/pull/25.diff
> Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25
modules/javafx.swing/src/main/java/javafx/embed/swing/JFXPanel.java line 457:
> 456:
> 457: sendMouseEventToFX(e);
> 458: super.processMouseEvent(e);
typo: save --> safe
modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
line 2:
> 1: /*
> 2: * Copyright (c) 2014, 2016 Oracle and/or its affiliates. All rights
> reserved.
> 3: * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
replace `2014, 2016` with `2019,`
modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
line 28:
> 27:
> 28: import javafx.application.Application;
> 29: import javafx.application.Platform;
Remove unused import (several of the below imports are similarly unused). Also,
since this is a new test, please sort the imports.
modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
line 48:
> 47:
> 48: import static junit.framework.Assert.assertEquals;
> 49: import static org.junit.Assert.assertNotNull;
This method should be imported from `org.junit` package.
modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
line 116:
> 115: MouseEvent e = new MouseEvent(fxPnl,
> MouseEvent.MOUSE_PRESSED, 0, MouseEvent.BUTTON1_DOWN_MASK,
> 116: 5, 5, 1, false, MouseEvent.BUTTON1);
> 117:
This doesn't appear to trigger the bug -- at least on Windows. The test passes
for me with or without your fix. You might consider moving it to the system
tests project, under `tests/system/src/test/java/test/robot` and using `Robot`
to effect the mouse press.
modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
line 57:
> 56:
> 57:
> 58: @BeforeClass
You can remove the extra blank line.
modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
line 65:
> 64:
> 65:
> 66: try {
You can remove the extra blank line.
modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
line 79:
> 78:
> 79: class TestFXPanel extends JFXPanel {
> 80: protected void processMouseEventPublic(MouseEvent e) {
I think this can be a static class.
modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
line 88:
> 87:
> 88: CountDownLatch firstPressedEventLatch = new CountDownLatch(1);
> 89:
This can be final.
modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
line 91:
> 90: // It's an array, so we can mutate it inside of lambda statement
> 91: int[] pressedEventCounter = {0};
> 92:
This can be final.
modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
line 132:
> 131:
> 132:
> 133: }
You can remove the extra blank line.
modules/javafx.swing/src/test/java/test/javafx/embed/swing/JFXPanelTest.java
line 123:
> 122:
> 123: if(!firstPressedEventLatch.await(5000, TimeUnit.MILLISECONDS)) {
> 124: throw new Exception();
Add a space after the `if`.
The fix seems fine. Have you tested it on all three platforms?
I have several comments on the test.
Disapproved by kcr (Lead).
PR: https://git.openjdk.java.net/jfx/pull/25
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Tue, 29 Oct 2019 15:59:58 GMT, Kevin Rushforth wrote: > On Tue, 29 Oct 2019 15:48:46 GMT, Florian Kirmaier > wrote: > >> On Tue, 29 Oct 2019 14:47:19 GMT, Kevin Rushforth wrote: >> >>> On Tue, 29 Oct 2019 14:45:46 GMT, Kevin Rushforth wrote: >>> On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier wrote: > Original PR: https://github.com/javafxports/openjdk-jfx/pull/591 > > A side effect of JDK-8200224 is, that the first click on a JFXPanel is > always a double click. > The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in > JFXPanel ignored if another swing component has focus). > This fix introduced synthesized press-events, which is an unsafe fix for > the problem. > > The pull request introduces a new fix for the problem. > Instead of simulating new input-events, we set JavaFX Scene/Window to > focused. > We do so, by using setFocused. > When the original Swing-Focus event is called slightly later, it won't > have any side-effects, > because calling setFocused just sets the value of a boolean property. > > I've now also added a unit-test for the problem. > > > > Commits: > - 314e423c: JDK-8200224 > - 725e5fef: JDK-8200224 > > Changes: https://git.openjdk.java.net/jfx/pull/25/files > Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00 > Issue: https://bugs.openjdk.java.net/browse/JDK-8200224 > Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod > Patch: https://git.openjdk.java.net/jfx/pull/25.diff > Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25 @FlorianKirmaier you still need to file a JBS issue to associate your GitHub username with your OpenJDK user ID as noted [above](#issuecomment-546883472), and per the instructions in [this message](https://mail.openjdk.java.net/pipermail/openjfx-dev/2019-September/023558.html) sent to openjfx-dev. >>> >>> @prsadhuk please review this. I will also review it (this will need two >>> reviewers). >> >> @kevinrushforth >> If created the ticket a moment ago. >> https://bugs.openjdk.java.net/browse/JDK-8233121 >> Is this okay, or is any information missing? > >> https://bugs.openjdk.java.net/browse/JDK-8233121 > > It was created in the JDK Project rather than the SKARA project (odd...the > link should have filled in the right project and component). I fixed it. Great, thanks. Some more notes on the importance of this Change. A project which uses a lot of JFXPanels for small components was basically unusable because the majority of clicks were registered as a double click. This made the software basically unusable with JavaFX11. PR: https://git.openjdk.java.net/jfx/pull/25
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Tue, 29 Oct 2019 15:48:46 GMT, Florian Kirmaier wrote: > On Tue, 29 Oct 2019 14:47:19 GMT, Kevin Rushforth wrote: > >> On Tue, 29 Oct 2019 14:45:46 GMT, Kevin Rushforth wrote: >> >>> On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier >>> wrote: >>> Original PR: https://github.com/javafxports/openjdk-jfx/pull/591 A side effect of JDK-8200224 is, that the first click on a JFXPanel is always a double click. The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in JFXPanel ignored if another swing component has focus). This fix introduced synthesized press-events, which is an unsafe fix for the problem. The pull request introduces a new fix for the problem. Instead of simulating new input-events, we set JavaFX Scene/Window to focused. We do so, by using setFocused. When the original Swing-Focus event is called slightly later, it won't have any side-effects, because calling setFocused just sets the value of a boolean property. I've now also added a unit-test for the problem. Commits: - 314e423c: JDK-8200224 - 725e5fef: JDK-8200224 Changes: https://git.openjdk.java.net/jfx/pull/25/files Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00 Issue: https://bugs.openjdk.java.net/browse/JDK-8200224 Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod Patch: https://git.openjdk.java.net/jfx/pull/25.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25 >>> >>> @FlorianKirmaier you still need to file a JBS issue to associate your >>> GitHub username with your OpenJDK user ID as noted >>> [above](#issuecomment-546883472), and per the instructions in [this >>> message](https://mail.openjdk.java.net/pipermail/openjfx-dev/2019-September/023558.html) >>> sent to openjfx-dev. >> >> @prsadhuk please review this. I will also review it (this will need two >> reviewers). > > @kevinrushforth > If created the ticket a moment ago. > https://bugs.openjdk.java.net/browse/JDK-8233121 > Is this okay, or is any information missing? > https://bugs.openjdk.java.net/browse/JDK-8233121 It was created in the JDK Project rather than the SKARA project (odd...the link should have filled in the right project and component). I fixed it. PR: https://git.openjdk.java.net/jfx/pull/25
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Tue, 29 Oct 2019 14:47:19 GMT, Kevin Rushforth wrote: > On Tue, 29 Oct 2019 14:45:46 GMT, Kevin Rushforth wrote: > >> On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier >> wrote: >> >>> Original PR: https://github.com/javafxports/openjdk-jfx/pull/591 >>> >>> A side effect of JDK-8200224 is, that the first click on a JFXPanel is >>> always a double click. >>> The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in >>> JFXPanel ignored if another swing component has focus). >>> This fix introduced synthesized press-events, which is an unsafe fix for >>> the problem. >>> >>> The pull request introduces a new fix for the problem. >>> Instead of simulating new input-events, we set JavaFX Scene/Window to >>> focused. >>> We do so, by using setFocused. >>> When the original Swing-Focus event is called slightly later, it won't have >>> any side-effects, >>> because calling setFocused just sets the value of a boolean property. >>> >>> I've now also added a unit-test for the problem. >>> >>> >>> >>> Commits: >>> - 314e423c: JDK-8200224 >>> - 725e5fef: JDK-8200224 >>> >>> Changes: https://git.openjdk.java.net/jfx/pull/25/files >>> Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00 >>> Issue: https://bugs.openjdk.java.net/browse/JDK-8200224 >>> Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod >>> Patch: https://git.openjdk.java.net/jfx/pull/25.diff >>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25 >> >> @FlorianKirmaier you still need to file a JBS issue to associate your GitHub >> username with your OpenJDK user ID as noted >> [above](#issuecomment-546883472), and per the instructions in [this >> message](https://mail.openjdk.java.net/pipermail/openjfx-dev/2019-September/023558.html) >> sent to openjfx-dev. > > @prsadhuk please review this. I will also review it (this will need two > reviewers). @kevinrushforth If created the ticket a moment ago. https://bugs.openjdk.java.net/browse/JDK-8233121 Is this okay, or is any information missing? PR: https://git.openjdk.java.net/jfx/pull/25
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Tue, 29 Oct 2019 14:45:46 GMT, Kevin Rushforth wrote: > On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier > wrote: > >> Original PR: https://github.com/javafxports/openjdk-jfx/pull/591 >> >> A side effect of JDK-8200224 is, that the first click on a JFXPanel is >> always a double click. >> The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in >> JFXPanel ignored if another swing component has focus). >> This fix introduced synthesized press-events, which is an unsafe fix for the >> problem. >> >> The pull request introduces a new fix for the problem. >> Instead of simulating new input-events, we set JavaFX Scene/Window to >> focused. >> We do so, by using setFocused. >> When the original Swing-Focus event is called slightly later, it won't have >> any side-effects, >> because calling setFocused just sets the value of a boolean property. >> >> I've now also added a unit-test for the problem. >> >> >> >> Commits: >> - 314e423c: JDK-8200224 >> - 725e5fef: JDK-8200224 >> >> Changes: https://git.openjdk.java.net/jfx/pull/25/files >> Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00 >> Issue: https://bugs.openjdk.java.net/browse/JDK-8200224 >> Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod >> Patch: https://git.openjdk.java.net/jfx/pull/25.diff >> Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25 > > @FlorianKirmaier you still need to file a JBS issue to associate your GitHub > username with your OpenJDK user ID as noted [above](#issuecomment-546883472), > and per the instructions in [this > message](https://mail.openjdk.java.net/pipermail/openjfx-dev/2019-September/023558.html) > sent to openjfx-dev. @prsadhuk please review this. I will also review it (this will need two reviewers). PR: https://git.openjdk.java.net/jfx/pull/25
Re: RFR: 8200224: Multiple press event when JFXPanel gains focus
On Tue, 29 Oct 2019 11:27:35 GMT, Florian Kirmaier wrote: > Original PR: https://github.com/javafxports/openjdk-jfx/pull/591 > > A side effect of JDK-8200224 is, that the first click on a JFXPanel is always > a double click. > The cause for JDK-8200224 is the fix for JDK-8087914 (Clicking on Menu in > JFXPanel ignored if another swing component has focus). > This fix introduced synthesized press-events, which is an unsafe fix for the > problem. > > The pull request introduces a new fix for the problem. > Instead of simulating new input-events, we set JavaFX Scene/Window to focused. > We do so, by using setFocused. > When the original Swing-Focus event is called slightly later, it won't have > any side-effects, > because calling setFocused just sets the value of a boolean property. > > I've now also added a unit-test for the problem. > > > > Commits: > - 314e423c: JDK-8200224 > - 725e5fef: JDK-8200224 > > Changes: https://git.openjdk.java.net/jfx/pull/25/files > Webrev: https://webrevs.openjdk.java.net/jfx/25/webrev.00 > Issue: https://bugs.openjdk.java.net/browse/JDK-8200224 > Stats: 140 lines in 2 files changed: 133 ins; 2 del; 5 mod > Patch: https://git.openjdk.java.net/jfx/pull/25.diff > Fetch: git fetch https://git.openjdk.java.net/jfx pull/25/head:pull/25 @FlorianKirmaier you still need to file a JBS issue to associate your GitHub username with your OpenJDK user ID as noted [above](#issuecomment-546883472), and per the instructions in [this message](https://mail.openjdk.java.net/pipermail/openjfx-dev/2019-September/023558.html) sent to openjfx-dev. PR: https://git.openjdk.java.net/jfx/pull/25
