JDK-8201291 : Clicking a JFXPanel having setFocusable(false) causes its processMouseEvent method to loop forever
  • Type: Bug
  • Component: javafx
  • Sub-Component: swing
  • Affected Version: 10
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: x86_64
  • Submitted: 2018-04-06
  • Updated: 2018-07-20
  • Resolved: 2018-07-20
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
openjfx11Fixed
Description
FULL PRODUCT VERSION :
java version "10" 2018-03-20
Java(TM) SE Runtime Environment 18.3 (build 10+46)
Java HotSpot(TM) 64-Bit Server VM 18.3 (build 10+46, mixed mode)

ADDITIONAL OS VERSION INFORMATION :
Any OS running Java 10, for example latest Windows 10

A DESCRIPTION OF THE PROBLEM :
Clicking a JFXPanel having setFocusable(false) causes its processMouseEvent method to loop forever.

Depending on the contents of the JFXPanel, the results may be catastrophic. With a JavaFX media player contained in a JFXPanel, the Swing app hangs immediately and after a few seconds I get out of memory errors.

Please compare:

Java 10's javafx.swing/javafx/embed/swing/JFXPanel.java
-----------------------------------------------------------
    @Override
    protected void processMouseEvent(MouseEvent e) {
        if ((e.getID() == MouseEvent.MOUSE_PRESSED) &&
            (e.getButton() == MouseEvent.BUTTON1)) {
            if (!hasFocus()) {
                requestFocus();
                // this focus request event goes to eventqueue and will be
                // asynchronously handled so MOUSE_PRESSED event will not be
                // honoured by FX immediately due to lack of focus in fx
                // component. Fire the same MOUSE_PRESSED event after
                // requestFocus() so that 2nd mouse press will be honoured
                // since now fx have focus
                AppContext context = SunToolkit.targetToAppContext(this);
                if (context != null) {
                    SunToolkit.postEvent(context, e);
                }
            }
        }

        sendMouseEventToFX(e);
        super.processMouseEvent(e);
    }
-----------------------------------------------------------

to:

Java 9's javafx.swing/javafx/embed/swing/JFXPanel.java
-----------------------------------------------------------
    @Override
    protected void processMouseEvent(MouseEvent e) {
        if ((e.getID() == MouseEvent.MOUSE_PRESSED) &&
            (e.getButton() == MouseEvent.BUTTON1)) {
            if (!hasFocus()) {
                requestFocus();
            }
        }

        sendMouseEventToFX(e);
        super.processMouseEvent(e);
    }
-----------------------------------------------------------


Obviously with Java 10, if, for any reason (for example because JFXPanel.setFocusable(false)), JFXPanel never gets the focus then processMouseEvent() loops for ever.

A simple fix is to replace:

---
if (!hasFocus()) {
---

by:

---
if (isFocusable() && !hasFocus()) {
---

Or better, do not bother invoking "requestFocus();" at all because it's the responsiblity of the client code of the JFXPanel to ensure that JavaFX gets the focus (if needed).



REGRESSION.  Last worked in version 9.0.4

ADDITIONAL REGRESSION INFORMATION: 
It last worked with:

java version "9.0.4"
Java(TM) SE Runtime Environment (build 9.0.4+11)
Java HotSpot(TM) 64-Bit Server VM (build 9.0.4+11, mixed mode)


STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
1) Compile attached code saved as NonFocusableJFXPanel.java

javac NonFocusableJFXPanel.java

2) Run it using Java 10 from a command prompt to see the messages printed by the Swing app

java -cp . NonFocusableJFXPanel

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
After clicking the button just once, should print :

1
ACTUAL -
After clicking the button just once, prints :

1
2
3
...
4464
4465
4466
...
INCREMENTS FOR EVER

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import java.awt.BorderLayout;
import javax.swing.SwingUtilities;
import javax.swing.JPanel;
import javax.swing.JFrame;
import javafx.application.Platform;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.embed.swing.JFXPanel;

/*
 * Compile attached code saved as NonFocusableJFXPanel.java:
 *
 * javac NonFocusableJFXPanel.java
 *
 * Run it from a command prompt to see the messages printed by the Swing app:
 *
 * java -cp . NonFocusableJFXPanel
 *
 * Works fine with Java 1.8.0_162, Java 9.0.4 but not with Java 10.
 */
public class NonFocusableJFXPanel {
    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            public void run() {
                initAndShowGUI();
            }
        });
    }

    public static void initAndShowGUI() {
        JFrame frame = new JFrame("NonFocusableJFXPanel");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        JPanel workArea = (JPanel) frame.getContentPane();

        final JFXPanel fxPanel = new JFXPanel();

        // With Java 10, works fine with .setFocusable(true).
        fxPanel.setFocusable(false);

        workArea.add(fxPanel, BorderLayout.CENTER);

        Platform.runLater(new Runnable() {
            public void run() {
                initFX(fxPanel);
            }
        });

        frame.setSize(600, 200);
        frame.setVisible(true);
    }

    private static int clickCount = 0;

    private static void initFX(JFXPanel fxPanel) {
        Button button = new Button("Click me ONCE and look at your console.");
        button.setOnMousePressed(e -> System.err.println(++clickCount));

        Scene scene = new Scene(button);
        fxPanel.setScene(scene);
    }
}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Comment out:

fxPanel.setFocusable(false);

or replace it by:

fxPanel.setFocusable(true);


Comments
Changeset: 1184b6e038e0 Author: psadhukhan Date: 2018-07-20 18:27 +0530 URL: http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/1184b6e038e0
20-07-2018

+1
20-07-2018

lgtm
20-07-2018

Modified unit test with this changes http://cr.openjdk.java.net/~psadhukhan/fx/8201291/webrev.2/
20-07-2018

Fix looks fine. The test verifies the fix (fails without the fix and passes with the fix). Minor comments on the test: 1. In unit tests it is not usually a best practice to ignore exceptions: 113 try { 114 waitForLatch(latch, 5000); 115 } catch (Exception e) {} 116 } I recommend to fail the test if Exception is caught (e.g., by calling Assert.fail). 2. Minor nit: extra space in "testJFXPanelFocus" method before "throws Exception"
19-07-2018

Modified unit test http://cr.openjdk.java.net/~psadhukhan/fx/8201291/webrev.1/
19-07-2018

Added check for focusability for JFXPanel. Request to review http://cr.openjdk.java.net/~psadhukhan/fx/8201291/webrev.0/
19-07-2018

Few nits related to test: 64 robot = new Robot(); 65 robot.setAutoDelay(100); 66 67 SwingUtilities.invokeAndWait(this::initAndShowGUI); 68 waitForLatch(l1, 5000); 69 70 Util.sleep(1000); You can move initilization related statements to @BeforeClass, since you have @AfterClass as well. Just to keep things aligned. >> 56 final CountDownLatch l1 = new CountDownLatch(2); Could you rename the variable `l1` to something else. It took sometime to recognise it is l1 not 11 when reading `waitForLatch(l1, 5000)` :) >> 67 SwingUtilities.invokeAndWait(this::initAndShowGUI); >> 68 waitForLatch(l1, 5000); Isn't `invokeAndWait` waits until the given `Runnable` returns? I'm not a swing guy, I might be wrong. IIUC, you wanted to wait until the inner 'Platform.runLater' to completes execution. In that case you could move the CountDownLatch local to initAndShowGUI with count 1. 83 try { 84 latch.await(ms, TimeUnit.MILLISECONDS); 85 } catch (InterruptedException e) { 86 } 87 if (latch.getCount() > 0) { 88 Assert.fail("unexpected error: waiting timeout " + ms + "ms elapsed for " + latch); 89 } Better to use `Assert.assertTrue(String.format("unexpected error: waiting timeout %d ms elapsed for", ms), latch.await(ms, TimeUnit.MILLISECONDS));` 58 public static void main(String[] args) throws Exception { 59 new NonFocusableJFXPanelTest().testJFXPanelFocus(); 60 } Main method could be removed. 121 if (frame != null) { 122 SwingUtilities.invokeLater(frame::dispose); 123 } Use `assertNotNull(frame)` instead of condition statement.
19-07-2018

Issue is reproducible and its a regression introduced in JDK 10+21. Windows 10, 64-bit JDK results: ------------ 8u172-b11 : Pass 9.0.4+11 : Pass 10+20: Pass 10+21 : Fail <-- regression introduced here 10+46 : Fail 11-ea+8 : Fail -------------
09-04-2018