Re: [14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Pankaj Bansal]

Hello Tejpal,

I have few minor comments. You can fix them before pushing this fix.

1. Import individual packages instead of import javax.swing.*.
2. You don't need to create editor1 variable.
3. There should be space after comma editor1,editor2.
4. Rename variables JTextField_Focus_Status, LF according to rules.

Regards,
Pankaj



-Original Message-
From: Sergey Bylokhov 
Sent: Monday, January 13, 2020 12:17 PM
To: Tejpal Rebari
Cc: swing-dev@openjdk.java.net
Subject: Re:  [14] RFR JDK-8223788 - [macos] JSpinner buttons in 
JColorChooser dialog may capture focus using TAB Key

Looks fine.

On 1/12/20 10:32 pm, Tejpal Rebari wrote:
> Hi Sergey,
> 
>> On 13-Jan-2020, at 1:15 AM, Sergey Bylokhov > <mailto:sergey.bylok...@oracle.com>> wrote:
>>
>> Hi, Tejpal
>>
>> Small note that "editor2.getTextField().isFocusOwner()" should be called on 
>> EDT, it is a Swing component.
> 
> I have modified the test.
> webrev : http://cr.openjdk.java.net/~trebari/swing/8223788/webrev3/
> 
> regards
> Tejpal
> 
>>
>> On 1/10/20 1:32 am, Tejpal Rebari wrote:
>>> Hi Sergey,
>>> I had removed the colorchooser and created a frame and added two spinner to 
>>> it.
>>> Then noticed that the issue is reproducible only when the 
>>> ContainerOrderFocusTraversalPolicyis set.
>>> WhenContainerOrderFocusTraversalPolicy is set, in JSpinner there are 
>>> four components which captures focus 1)JSpinner 
>>> 2)JSpinner$NumberEditor 3)JFormattedTextField 
>>> 4)AquaSpinnerUI$SpinPainter In JColorChooser, SlidingSpinner is used 
>>> which sets focusable property to false for1)JSpinner and 
>>> 2)JSpinner$NumberEditor So in the test I 
>>> usedContainerOrderFocusTraversalPolicy and set the focusable property false 
>>> for 1)JSpinner and 2)JSpinner$NumberEditor and then check that the 
>>> spinpainter is capturing focus or not.
>>> Here is the updated 
>>> webrev:http://cr.openjdk.java.net/~trebari/swing/8223788/webrev2/
>>> Regards
>>> Tejpal
>>
>>
>> --
>> Best regards, Sergey.
> 


-- 
Best regards, Sergey.

Re: [14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Sergey Bylokhov]

Looks fine.

On 1/12/20 10:32 pm, Tejpal Rebari wrote:

Hi Sergey,


On 13-Jan-2020, at 1:15 AM, Sergey Bylokhov mailto:sergey.bylok...@oracle.com>> wrote:

Hi, Tejpal

Small note that "editor2.getTextField().isFocusOwner()" should be called on 
EDT, it is a Swing component.


I have modified the test.
webrev : http://cr.openjdk.java.net/~trebari/swing/8223788/webrev3/

regards
Tejpal



On 1/10/20 1:32 am, Tejpal Rebari wrote:

Hi Sergey,
I had removed the colorchooser and created a frame and added two spinner to it.
Then noticed that the issue is reproducible only when the 
ContainerOrderFocusTraversalPolicyis set.
WhenContainerOrderFocusTraversalPolicy is set, in JSpinner there are four 
components which captures focus
1)JSpinner
2)JSpinner$NumberEditor
3)JFormattedTextField
4)AquaSpinnerUI$SpinPainter
In JColorChooser, SlidingSpinner is used which sets focusable property to false 
for1)JSpinner and 2)JSpinner$NumberEditor
So in the test I usedContainerOrderFocusTraversalPolicy and set the focusable 
property false for 1)JSpinner and 2)JSpinner$NumberEditor and then check that 
the spinpainter is capturing focus or not.
Here is the updated 
webrev:http://cr.openjdk.java.net/~trebari/swing/8223788/webrev2/
Regards
Tejpal



--
Best regards, Sergey.





--
Best regards, Sergey.

Re: [14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Tejpal Rebari]

Hi Sergey,

> On 13-Jan-2020, at 1:15 AM, Sergey Bylokhov  
> wrote:
> 
> Hi, Tejpal
> 
> Small note that "editor2.getTextField().isFocusOwner()" should be called on 
> EDT, it is a Swing component.

I have modified the test.
webrev : http://cr.openjdk.java.net/~trebari/swing/8223788/webrev3/ 


regards
Tejpal

> 
> On 1/10/20 1:32 am, Tejpal Rebari wrote:
>> Hi Sergey,
>> I had removed the colorchooser and created a frame and added two spinner to 
>> it.
>> Then noticed that the issue is reproducible only when the 
>> ContainerOrderFocusTraversalPolicyis set.
>> WhenContainerOrderFocusTraversalPolicy is set, in JSpinner there are four 
>> components which captures focus
>> 1)JSpinner
>> 2)JSpinner$NumberEditor
>> 3)JFormattedTextField
>> 4)AquaSpinnerUI$SpinPainter
>> In JColorChooser, SlidingSpinner is used which sets focusable property to 
>> false for1)JSpinner and 2)JSpinner$NumberEditor
>> So in the test I usedContainerOrderFocusTraversalPolicy and set the 
>> focusable property false for 1)JSpinner and 2)JSpinner$NumberEditor and then 
>> check that the spinpainter is capturing focus or not.
>> Here is the updated 
>> webrev:http://cr.openjdk.java.net/~trebari/swing/8223788/webrev2/
>> Regards
>> Tejpal
> 
> 
> -- 
> Best regards, Sergey.

Re: [14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Sergey Bylokhov]

Hi, Tejpal

Small note that "editor2.getTextField().isFocusOwner()" should be called on 
EDT, it is a Swing component.


On 1/10/20 1:32 am, Tejpal Rebari wrote:

Hi Sergey,
I had removed the colorchooser and created a frame and added two spinner to it.
Then noticed that the issue is reproducible only when the 
ContainerOrderFocusTraversalPolicyis set.
WhenContainerOrderFocusTraversalPolicy is set, in JSpinner there are four 
components which captures focus
1)JSpinner
2)JSpinner$NumberEditor
3)JFormattedTextField
4)AquaSpinnerUI$SpinPainter

In JColorChooser, SlidingSpinner is used which sets focusable property to false 
for1)JSpinner and 2)JSpinner$NumberEditor

So in the test I usedContainerOrderFocusTraversalPolicy and set the focusable 
property false for 1)JSpinner and 2)JSpinner$NumberEditor and then check that 
the spinpainter is capturing focus or not.

Here is the updated 
webrev:http://cr.openjdk.java.net/~trebari/swing/8223788/webrev2/

Regards
Tejpal





--
Best regards, Sergey.

Re: [14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Tejpal Rebari]

Hi Sergey,
I had removed the colorchooser and created a frame and added two spinner to it.
Then noticed that the issue is reproducible only when the 
ContainerOrderFocusTraversalPolicy is set.
When ContainerOrderFocusTraversalPolicy is set, in JSpinner there are four 
components which captures focus 
1)JSpinner 
2)JSpinner$NumberEditor 
3)JFormattedTextField 
4)AquaSpinnerUI$SpinPainter

In JColorChooser, SlidingSpinner is used which sets focusable property to false 
for 1)JSpinner and 2)JSpinner$NumberEditor

So in the test I used ContainerOrderFocusTraversalPolicy and set the focusable 
property false for 1)JSpinner and 2)JSpinner$NumberEditor and then check that 
the spinpainter is capturing focus or not.

Here is the updated webrev : 
http://cr.openjdk.java.net/~trebari/swing/8223788/webrev2/ 


Regards
Tejpal

Re: [14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Sergey Bylokhov]

On 12/12/19 8:30 am, Tejpal Rebari wrote:

Tested on Mac,Windows and Linux , with and without the fix.


I guess the test passed on Win/Lin because it is noop on these platforms. I 
suggest to
eliminate JColorChooser and create a simple frame to emulate the bug. It would 
be useful
to check all L in the loop.


--
Best regards, Sergey.

Re: [14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Tejpal Rebari]

Hi Sergey,

> Is it possible to add an automated test for this use case?

I have added an automated test.
Here is the updated webrev :  
http://cr.openjdk.java.net/~trebari/swing/8223788/webrev1/ 


Tested on Mac,Windows and Linux , with and without the fix.

Regards
Tejpal

Re: [14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Sergey Bylokhov]

On 12/5/19 11:05 pm, Tejpal Rebari wrote:

Navigation over JSpinner editor is possible by keyboard after the fix.

Before the fix, is we are inside a JSpinner editor and press TAB key the focus 
disappears and it is captured by the JSpinner Button which is not focusable by 
mouse click.

Thank you for clarification.


I have ran SwingSet2 demo -> JColorChooser -> Background -> HSV for Testing 
JSpinner.


Is it possible to add an automated test for this use case?

--
Best regards, Sergey.

Re: [14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Tejpal Rebari]

Hi Sergey,

> On 06-Dec-2019, at 4:43 AM, Sergey Bylokhov  
> wrote:
> 
> Hi, Tejpal.
> 
> Are you sure that it will be possible to change the value of the
> JSpinner and

It is possible to change the value of JSpinner after the fix.
The Values can be changed by clicking on the JSpinner editor and type (legal 
values),press up/down keys or mouse click on the JSpinner Buttons.

> navigate over JSpinner and its buttons by the keyboard
> after this fix?

Navigation over JSpinner editor is possible by keyboard after the fix.

Before the fix, is we are inside a JSpinner editor and press TAB key the focus 
disappears and it is captured by the JSpinner Button which is not focusable by 
mouse click.
After the fix the focus directly goes to the second JSpinner editor.

I have ran SwingSet2 demo -> JColorChooser -> Background -> HSV for Testing 
JSpinner.

Regards
Tejpal


> On 12/4/19 10:25 pm, Tejpal Rebari wrote:
>> There is no such issue with other LookAndFeels.
>> Fix : The focusable property is set to true by default, Fix is to set the 
>> JSpinner button focusable property false in AquaLookAndFeel.
> 
> 
> 
> 
> 
> -- 
> Best regards, Sergey.

Re: [14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Sergey Bylokhov]

Hi, Tejpal.

Are you sure that it will be possible to change the value of the
JSpinner and navigate over JSpinner and its buttons by the keyboard
after this fix?

On 12/4/19 10:25 pm, Tejpal Rebari wrote:

There is no such issue with other LookAndFeels.

Fix : The focusable property is set to true by default, Fix is to set the 
JSpinner button focusable property false in AquaLookAndFeel.






--
Best regards, Sergey.

Re: [14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Pankaj Bansal]

Looks good to me

 

Regards,

Pankaj

 

From: Prasanta Sadhukhan 
Sent: Thursday, December 5, 2019 3:38 PM
To: Tejpal Rebari; swing-dev@openjdk.java.net
Subject: Re:  [14] RFR JDK-8223788 - [macos] JSpinner buttons in 
JColorChooser dialog may capture focus using TAB Key

 

+1 but you need to put noreg-demo label in JBS.

Regards

Prasanta

On 05-Dec-19 11:55 AM, wrote:

Hi All, 

Please review the following fix for jdk14.

 

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

Webrev :  http://cr.openjdk.java.net/~trebari/swing/8223788/webrev0/

 

Issue  : In AquaLookAndFeel  JSpinner buttons inside JColorChooser dialog 
captures focus.

If we click on the editor inside the JSpinner to make it gain focus and then 
press TAB key, the focus should go to next component but it disappears and it 
is captured by the JSpinner button.By again pressing TAB the focus goes to next 
component.

 

There is no such issue with other LookAndFeels.

 

Fix : The focusable property is set to true by default, Fix is to set the 
JSpinner button focusable property false in AquaLookAndFeel.

 

Testing : Tested In Mac.

 

Regards

Tejpal

Re: [14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Prasanta Sadhukhan]

+1 but you need to put noreg-demo label in JBS.

Regards

Prasanta

On 05-Dec-19 11:55 AM, wrote:

Hi All,
Please review the following fix for jdk14.

Bug: https://bugs.openjdk.java.net/browse/JDK-8223788
Webrev: http://cr.openjdk.java.net/~trebari/swing/8223788/webrev0/

Issue  : In AquaLookAndFeel  JSpinner buttons inside JColorChooser 
dialog captures focus.
If we click on the editor inside the JSpinner to make it gain focus 
and then press TAB key, the focus should go to next component but it 
disappears and it is captured by the JSpinner button.By again pressing 
TAB the focus goes to next component.


There is no such issue with other LookAndFeels.

Fix : The focusable property is set to true by default, Fix is to set 
the JSpinner button focusable property false in AquaLookAndFeel.


Testing : Tested In Mac.

Regards
Tejpal

[14] RFR JDK-8223788 - [macos] JSpinner buttons in JColorChooser dialog may capture focus using TAB Key

[Tejpal Rebari]

Hi All,
Please review the following fix for jdk14.

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

Webrev  :  http://cr.openjdk.java.net/~trebari/swing/8223788/webrev0/ 


Issue  : In AquaLookAndFeel  JSpinner buttons inside JColorChooser dialog 
captures focus.
If we click on the editor inside the JSpinner to make it gain focus and then 
press TAB key, the focus should go to next component but it disappears and it 
is captured by the JSpinner button.By again pressing TAB the focus goes to next 
component.

There is no such issue with other LookAndFeels.

Fix : The focusable property is set to true by default, Fix is to set the 
JSpinner button focusable property false in AquaLookAndFeel.

Testing : Tested In Mac.

Regards
Tejpal