Steve Northover (Inactive) added a comment - 2014-06-10 11:22 Tested-with: MouseTest_RT37436.java (the attachment), SampleApp.java (in the comment of this JIRA) Unit Tests: none

Steve Northover (Inactive) added a comment - 2014-06-10 11:20 Changeset: d10817fc3844 Author: snorthov Date: 2014-06-10 14:06 -0400 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/d10817fc3844

Anthony Petrov (Inactive) added a comment - 2014-06-10 10:30 The fix looks fine now. +1

Felipe Heidrich (Inactive) added a comment - 2014-06-10 10:09 I'm okay with the last patch, personally i would let the "case MouseEvent.MOUSE_DRAGGED:' fall thru.

Steve Northover (Inactive) added a comment - 2014-06-10 10:05 - edited diff --git a/modules/swing/src/main/java/javafx/embed/swing/JFXPanel.java b/modules/swing/src/main/java/javafx/embed/swing/JFXPanel.java --- a/modules/swing/src/main/java/javafx/embed/swing/JFXPanel.java +++ b/modules/swing/src/main/java/javafx/embed/swing/JFXPanel.java @@ -319,6 +319,15 @@          if (scenePeer == null || !isFxEnabled()) {              return;          } + + // FX only supports 3 buttons so don't send the event for other buttons + switch (e.getID()) { + case MouseEvent.MOUSE_DRAGGED: + case MouseEvent.MOUSE_PRESSED: + case MouseEvent.MOUSE_RELEASED: + if (e.getButton() > 3) return; + break; + }            int extModifiers = e.getModifiersEx();          // Fix for RT-15457 : we should report no mouse button upon mouse release, so diff --git a/modules/swt/src/main/java/javafx/embed/swt/FXCanvas.java b/modules/swt/src/main/java/javafx/embed/swt/FXCanvas.java --- a/modules/swt/src/main/java/javafx/embed/swt/FXCanvas.java +++ b/modules/swt/src/main/java/javafx/embed/swt/FXCanvas.java @@ -398,17 +398,26 @@              }              @Override              public void mouseDown(MouseEvent me) { + // FX only supports 3 buttons so don't send the event for other buttons + if (me.button > 3) return;                  FXCanvas.this.sendMouseEventToFX(me, AbstractEvents.MOUSEEVENT_PRESSED);              }              @Override              public void mouseUp(MouseEvent me) { + // FX only supports 3 buttons so don't send the event for other buttons + if (me.button > 3) return;                  FXCanvas.this.sendMouseEventToFX(me, AbstractEvents.MOUSEEVENT_RELEASED);              }          });            addMouseMoveListener(me -> {              if ((me.stateMask & SWT.BUTTON_MASK) != 0) { - FXCanvas.this.sendMouseEventToFX(me, AbstractEvents.MOUSEEVENT_DRAGGED); + // FX only supports 3 buttons so don't send the event for other buttons + if ((me.stateMask & (SWT.BUTTON1 | SWT.BUTTON2 | SWT.BUTTON3)) != 0) { + FXCanvas.this.sendMouseEventToFX(me, AbstractEvents.MOUSEEVENT_DRAGGED); + } else { + FXCanvas.this.sendMouseEventToFX(me, AbstractEvents.MOUSEEVENT_MOVED); + }              } else {                  FXCanvas.this.sendMouseEventToFX(me, AbstractEvents.MOUSEEVENT_MOVED);              }

Steve Northover (Inactive) added a comment - 2014-06-10 10:04 The extra code is obviously bogus. I was working quickly and trying to do something else and missed this stupidity. Sorry about that and thanks! I would not have pushed the code with the extra crap in it. I'm not sure about just running away when button is greater than three. It is likely ok to say that any event with a button count greater than three should be thrown away, however the code is explicit. I will attach the new code that should allow mouse move for both, however, if you run the test code. Mouse dragged for 4 and 5 never comes in from AWT/Swing. (I will attach an AWT/Swing only example).

Felipe Heidrich (Inactive) added a comment - 2014-06-10 09:54 FXCanvas.java is fine. As for JFXPanel.java, since you moved the filtering code inside of sendMouseEventToFX(), why do you still need the copies of the code in processMouseEvent() and processMouseMotionEvent() ? For JFXPanel.java, what about just "if (e.getButton() > 3) return;" right at the start of sendMouseEventToFX() ? Would that work ? The one problem with the fix above (and with the fix you proposed as well) is the case of MOUSE_DRAGGED of buttons 4 and 5. For swt.FXCanvas you chose to translate those to mouse moved events, in swing.JFXPanel you chose to ignore them. Is that right ?

Steve Northover (Inactive) added a comment - 2014-06-10 08:13 Much better code. I was unaware the ENTER / EXIT came through the previous place. In fact, they don't seem to be delivered for me in the test code that is attached to this JIRA (before or after the patch).

Steve Northover (Inactive) added a comment - 2014-06-10 08:10 diff --git a/modules/swing/src/main/java/javafx/embed/swing/JFXPanel.java b/modules/swing/src/main/java/javafx/embed/swing/JFXPanel.java --- a/modules/swing/src/main/java/javafx/embed/swing/JFXPanel.java +++ b/modules/swing/src/main/java/javafx/embed/swing/JFXPanel.java @@ -319,6 +319,17 @@          if (scenePeer == null || !isFxEnabled()) {              return;          } + + // FX only supports 3 buttons so don't send the event for other buttons + switch (e.getID()) { + case MouseEvent.MOUSE_DRAGGED: + if (e.getButton() > 3) return; + break; + case MouseEvent.MOUSE_PRESSED: + case MouseEvent.MOUSE_RELEASED: + if (e.getButton() > 3) return; + break; + }            int extModifiers = e.getModifiersEx();          // Fix for RT-15457 : we should report no mouse button upon mouse release, so @@ -374,7 +385,20 @@              }          }   - sendMouseEventToFX(e); + // FX only supports 3 buttons so don't send the event for other buttons + switch (e.getID()) { + case MouseEvent.MOUSE_PRESSED: + case MouseEvent.MOUSE_RELEASED: + if (e.getButton() == MouseEvent.BUTTON1 + || e.getButton() == MouseEvent.BUTTON2 + || e.getButton() == MouseEvent.BUTTON3) { + sendMouseEventToFX(e); + } + break; + default: + sendMouseEventToFX(e); + break; + }          super.processMouseEvent(e);      }   @@ -387,7 +411,16 @@       */      @Override      protected void processMouseMotionEvent(MouseEvent e) { - sendMouseEventToFX(e); + // FX only supports 3 buttons so don't send the event for other buttons + if (e.getID() == MouseEvent.MOUSE_DRAGGED) { + if (e.getButton() == MouseEvent.BUTTON1 + || e.getButton() == MouseEvent.BUTTON2 + || e.getButton() == MouseEvent.BUTTON3) { + sendMouseEventToFX(e); + } + } else { + sendMouseEventToFX(e); + }          super.processMouseMotionEvent(e);      }   diff --git a/modules/swt/src/main/java/javafx/embed/swt/FXCanvas.java b/modules/swt/src/main/java/javafx/embed/swt/FXCanvas.java --- a/modules/swt/src/main/java/javafx/embed/swt/FXCanvas.java +++ b/modules/swt/src/main/java/javafx/embed/swt/FXCanvas.java @@ -398,17 +398,26 @@              }              @Override              public void mouseDown(MouseEvent me) { + // FX only supports 3 buttons so don't send the event for other buttons + if (me.button > 3) return;                  FXCanvas.this.sendMouseEventToFX(me, AbstractEvents.MOUSEEVENT_PRESSED);              }              @Override              public void mouseUp(MouseEvent me) { + // FX only supports 3 buttons so don't send the event for other buttons + if (me.button > 3) return;                  FXCanvas.this.sendMouseEventToFX(me, AbstractEvents.MOUSEEVENT_RELEASED);              }          });            addMouseMoveListener(me -> {              if ((me.stateMask & SWT.BUTTON_MASK) != 0) { - FXCanvas.this.sendMouseEventToFX(me, AbstractEvents.MOUSEEVENT_DRAGGED); + // FX only supports 3 buttons so don't send the event for other buttons + if ((me.stateMask & (SWT.BUTTON1 | SWT.BUTTON2 | SWT.BUTTON3)) != 0) { + FXCanvas.this.sendMouseEventToFX(me, AbstractEvents.MOUSEEVENT_DRAGGED); + } else { + FXCanvas.this.sendMouseEventToFX(me, AbstractEvents.MOUSEEVENT_MOVED); + }              } else {                  FXCanvas.this.sendMouseEventToFX(me, AbstractEvents.MOUSEEVENT_MOVED);              }

Anthony Petrov (Inactive) added a comment - 2014-06-10 07:47 1) ENTERED/EXITED events surely don't have this problem. However, this code from your patch: - sendMouseEventToFX(e); + // FX only supports 3 buttons so don't send the event for other buttons + switch (e.getID()) { + case MouseEvent.MOUSE_PRESSED: + case MouseEvent.MOUSE_RELEASED: + if (e.getButton() == MouseEvent.BUTTON1 + || e.getButton() == MouseEvent.BUTTON2 + || e.getButton() == MouseEvent.BUTTON3) { + sendMouseEventToFX(e); + } + } simply strips these events. It only passed PRESSED and RELEASED mouse events to sendMouseEventToFX(). 2) Not sure there's need to attach anything. The pattern for skipping events is all there, in the sendMouseEventToFX() method itself. E.g.:         if (e.getID() == MouseEvent.MOUSE_DRAGGED) {             if (!isCapturingMouse) {                 return;             } etc. Just add a similar code, like:         switch (ID) {             case PRESSED/RELEASED:                  if (button > 3) return;          } What problem do you see with this approach?