Re: [Rev 04] RFR: 8200224: Multiple press event when JFXPanel gains focus
On Wed, 27 Nov 2019 15:13:33 GMT, Kevin Rushforth wrote: > On Wed, 27 Nov 2019 05:31:27 GMT, Prasanta Sadhukhan > wrote: > >> On Tue, 26 Nov 2019 13:06:36 GMT, Florian Kirmaier >> wrote: >> >>> The pull request has been updated with additional changes. >>> >>> >>> >>> Added commits: >>> - 24385eb8: JDK-8200224 >>> - e0829ad3: JDK-8200224 >>> - c190384f: JDK-8200224 >>> - 17b458b1: JDK-8200224 >>> >>> Changes: >>> - all: https://git.openjdk.java.net/jfx/pull/25/files >>> - new: https://git.openjdk.java.net/jfx/pull/25/files/44774dfb..24385eb8 >>> >>> Webrevs: >>> - full: https://webrevs.openjdk.java.net/jfx/25/webrev.04 >>> - incr: https://webrevs.openjdk.java.net/jfx/25/webrev.03-04 >>> >>> Issue: https://bugs.openjdk.java.net/browse/JDK-8200224 >>> Stats: 141 lines in 2 files changed: 123 ins; 11 del; 7 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 449: >> >>> 448: requestFocus(); >>> 449: // This fixes JDK-8087914 without causing JDK-8200224 >>> 450: // It is safe, because in JavaFX only the method >>> "setFocused(true)" is called, >> >> We actually do not clutter source code with bugids, it will be good if it >> can be removed with proper comments maybe something like "extra simulated >> mouse pressed event is removed by making the JavaFX scene focused" > > In general, we no longer reference bug IDs in source code, so I agree with > the recommendation to reword the comment. > > @FlorianKirmaier - can you reword as suggested? Once you are done, you can > integrate it (using the `/integrate` command), and I will sponsor it. There > will likely be a delay due to the US Thanksgiving holiday. Done PR: https://git.openjdk.java.net/jfx/pull/25
Re: [Rev 04] RFR: 8200224: Multiple press event when JFXPanel gains focus
On Wed, 27 Nov 2019 05:31:27 GMT, Prasanta Sadhukhan wrote: > On Tue, 26 Nov 2019 13:06:36 GMT, Florian Kirmaier > wrote: > >> The pull request has been updated with additional changes. >> >> >> >> Added commits: >> - 24385eb8: JDK-8200224 >> - e0829ad3: JDK-8200224 >> - c190384f: JDK-8200224 >> - 17b458b1: JDK-8200224 >> >> Changes: >> - all: https://git.openjdk.java.net/jfx/pull/25/files >> - new: https://git.openjdk.java.net/jfx/pull/25/files/44774dfb..24385eb8 >> >> Webrevs: >> - full: https://webrevs.openjdk.java.net/jfx/25/webrev.04 >> - incr: https://webrevs.openjdk.java.net/jfx/25/webrev.03-04 >> >> Issue: https://bugs.openjdk.java.net/browse/JDK-8200224 >> Stats: 141 lines in 2 files changed: 123 ins; 11 del; 7 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 449: > >> 448: requestFocus(); >> 449: // This fixes JDK-8087914 without causing JDK-8200224 >> 450: // It is safe, because in JavaFX only the method >> "setFocused(true)" is called, > > We actually do not clutter source code with bugids, it will be good if it can > be removed with proper comments maybe something like "extra simulated mouse > pressed event is removed by making the JavaFX scene focused" In general, we no longer reference bug IDs in source code, so I agree with the recommendation to reword the comment. @FlorianKirmaier - can you reword as suggested? Once you are done, you can integrate it (using the `/integrate` command), and I will sponsor it. There will likely be a delay due to the US Thanksgiving holiday. PR: https://git.openjdk.java.net/jfx/pull/25
Re: [Rev 04] RFR: 8200224: Multiple press event when JFXPanel gains focus
On Tue, 26 Nov 2019 13:06:36 GMT, Florian Kirmaier wrote: > The pull request has been updated with additional changes. > > > > Added commits: > - 24385eb8: JDK-8200224 > - e0829ad3: JDK-8200224 > - c190384f: JDK-8200224 > - 17b458b1: JDK-8200224 > > Changes: > - all: https://git.openjdk.java.net/jfx/pull/25/files > - new: https://git.openjdk.java.net/jfx/pull/25/files/44774dfb..24385eb8 > > Webrevs: > - full: https://webrevs.openjdk.java.net/jfx/25/webrev.04 > - incr: https://webrevs.openjdk.java.net/jfx/25/webrev.03-04 > > Issue: https://bugs.openjdk.java.net/browse/JDK-8200224 > Stats: 141 lines in 2 files changed: 123 ins; 11 del; 7 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 449: > 448: requestFocus(); > 449: // This fixes JDK-8087914 without causing JDK-8200224 > 450: // It is safe, because in JavaFX only the method > "setFocused(true)" is called, We actually do not clutter source code with bugids, it will be good if it can be removed with proper comments maybe something like "extra simulated mouse pressed event is removed by making the JavaFX scene focused" PR: https://git.openjdk.java.net/jfx/pull/25
Re: [Rev 04] RFR: 8200224: Multiple press event when JFXPanel gains focus
The pull request has been updated with additional changes. Added commits: - 24385eb8: JDK-8200224 - e0829ad3: JDK-8200224 - c190384f: JDK-8200224 - 17b458b1: JDK-8200224 Changes: - all: https://git.openjdk.java.net/jfx/pull/25/files - new: https://git.openjdk.java.net/jfx/pull/25/files/44774dfb..24385eb8 Webrevs: - full: https://webrevs.openjdk.java.net/jfx/25/webrev.04 - incr: https://webrevs.openjdk.java.net/jfx/25/webrev.03-04 Issue: https://bugs.openjdk.java.net/browse/JDK-8200224 Stats: 141 lines in 2 files changed: 123 ins; 11 del; 7 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 PR: https://git.openjdk.java.net/jfx/pull/25
