The fix looks good to me.

Thanks,
Alexandr.

On 14/04/16 10:44, Ajit Ghaisas wrote:
Thanks Alex for the good suggestion to have a private method to avoid code 
repetition.

I have modified the code accordingly.  Please review the updated webrev.
http://cr.openjdk.java.net/~aghaisas/8049069/webrev.02/

Regards,
Ajit

-----Original Message-----
From: Alexander Scherbatiy
Sent: Thursday, April 14, 2016 1:00 AM
To: Ajit Ghaisas; Sergey Bylokhov; [email protected]
Subject: Re: <Swing Dev> [9] JDK-8049069 : JButton incorrect behaviour on 
button release


There is the same code which is repeated in three methods. Is it possible to 
add a private method which takes button, button mask, and event as parameters 
and returns true if the mouse event specifies the given button? It would be 
possible to reuse it in public methods.

Thanks,
Alexandr.

On 13/04/16 17:08, Ajit Ghaisas wrote:
Hi,

      Thanks for the review.
      Your questioning lead me to think about correcting the SwingUtilities 
isXXXMouseButton() methods.
It is possible to identify under which mouse state these methods are invoked using the MouseEvent parameter.
      Please review the updated webrev.
      http://cr.openjdk.java.net/~aghaisas/8049069/webrev.01/

      With this fix, I have verified tests mentioned under : JDK-7146377, 
JDK-7088744 and JDK-8049069. All of them passed.

Regards,
Ajit

-----Original Message-----
From: Alexander Scherbatiy
Sent: Tuesday, March 29, 2016 6:56 PM
To: Ajit Ghaisas; Sergey Bylokhov; [email protected]; Rajeev
Chamyal
Subject: Re: <Swing Dev> [9] JDK-8049069 : JButton incorrect behaviour
on button release

On 28/03/16 15:43, Ajit Ghaisas wrote:
Hi,
In JDK-6u115, this bug is not reproducible.
       Fix for JDK-7088744 did  not break the JButton behavior.
       This bug is regression due to changes done to SwingUtilities as a fix of 
JDK-7146377.

       After reading comments on JBS for JDK-7146377, I realized that the 
modification was done to support left mouse drag.
       Ideally, it should have been done using a new public method such as 
isLeftMouseButtonDown() and using it in test/client code.
       In this case, we will need to add isMiddleMouseButtonDown() and 
isRightMouseButtonDown() methods as well.
       Modifying existing APIs as well as introducing new public APIs
raises a risk of forced test/client code modifications. (This is the
reason why I have chosen option 2 as mentioned below)
     The SwingUtilities.isLeftMouseButton() method used not only in JButton 
listener but in other places too.
     Could you check some component like JSpinner with proper L&F that it does 
not have the same issue described in 8049069.

     It is possible just to add one internal method
isLeftMouseButtonDown() to the sun.swing.SwingUtilities2 class first.
     Will it be enough to  fix these two issues 8049069 and 7146377?

     Thanks,
     Alexandr.
       I have fixed the JDK-8049069 in BasicButtonListener.java as there is no 
need to use swingUtilities methods in mousePressed() and mouseReleased() 
methods.

Regards,
Ajit

-----Original Message-----
From: Sergey Bylokhov
Sent: Friday, March 25, 2016 3:33 AM
To: Ajit Ghaisas; [email protected]; Alexander Scherbatiy;
Rajeev Chamyal
Subject: Re: <Swing Dev> [9] JDK-8049069 : JButton incorrect
behaviour on button release

Hi, Ajit.
Is this bug a regression of JDK-7088744 or JDK-8049069, or it works this way in 
jdk6 as well?
I guess the code in jdk6 was something like this:
        public static boolean isLeftMouseButton(MouseEvent anEvent) {
             return ((anEvent.getModifiers() & InputEvent.BUTTON1_MASK) != 0);
        }

On 24.03.16 12:55, Ajit Ghaisas wrote:
Hi,

Bug : https://bugs.openjdk.java.net/browse/JDK-8049069

Issue :  A JButton which is pressed using left mouse button gets released if 
right mouse button is pressed and released.

Root cause :
---------------
In file BasicButtonListener.java, mousePressed() and mouseReleased() methods 
check whether the event is from left mouse button.
This check is done using ----  if
(SwingUtilities.isLeftMouseButton(e)
) This method returns true if left mouse button is down OR event is from left 
mouse button.
SwingUtilities.isLeftMouseButton() returns true if it is called for right mouse 
event while holding left button down. This causes mouseReleased() method to 
release pressed JButton.


Alternatives considered :
-----------------------------
1. Modifying SwingUtilities.isLeftMouseButton() method to only test
for whether the event is from left mouse button only 2. Modifying 
mousePressed() and mouseReleased() methods to check for whether the event is 
from left mouse button using argument passed to them.

Option 1 will break the code wherever SwingUtilities.isLeftMouseButton() is 
used to test for left mouse button down.
File history revealed that the mouse button down condition is added to fix 
another bug.

Hence, option 2 is chosen as it is safe and intuitive.
I have also added a test. It passes on Windows, Linux and Mac.
Test mentioned in comment on the bug also passed.

Please review the webrev :

http://cr.openjdk.java.net/~aghaisas/8049069/webrev.00/

Regards,
Ajit

--
Best regards, Sergey.

Reply via email to