Re: [10] RFR: 8182577: Crash when Tab key moves focus to a JCheckbox with a custom ButtonModel

[Prasanta Sadhukhan]

Modified JToogleButton.getGroupSelection() to remove check-and-cast to 
DefautlButtonModel since we have added getGroup() to ButtonModel interface.

Also, slight modification of @implSpec as per CSR suggestion and to be 
more explicit.

http://cr.openjdk.java.net/~psadhukhan/8182577/webrev.05/

Regards
Prasanta
On 6/27/2017 9:30 PM, Semyon Sadetsky wrote:
+1

Please, fix formatting (some modified lines are longer then 80)

--Semyon


On 06/26/2017 10:11 PM, Prasanta Sadhukhan wrote:
It seems we need to add @implSpec for default method as suggested by 
CSR group. Modified webrev to add @implSpec in the default method.

http://cr.openjdk.java.net/~psadhukhan/8182577/webrev.04/

Regards
Prasanta
On 6/24/2017 3:13 AM, Sergey Bylokhov wrote:
ok, +1

----- prasanta.sadhuk...@oracle.com wrote:

I could not find any default implementation having @implNote

http://hg.openjdk.java.net/jdk10/client/jdk/file/4cdd5d954479/src/java.desktop/share/classes/javax/swing/Action.java#l390 

http://hg.openjdk.java.net/jdk10/client/jdk/file/4cdd5d954479/src/java.desktop/share/classes/javax/swing/plaf/synth/SynthIcon.java#l69 


@implNote is present here but it is not for default methods
http://hg.openjdk.java.net/jdk10/client/jdk/file/4cdd5d954479/src/java.desktop/share/classes/java/awt/Desktop.java#l771 

http://hg.openjdk.java.net/jdk10/client/jdk/file/4cdd5d954479/src/java.desktop/share/classes/java/awt/Taskbar.java#l40 

http://hg.openjdk.java.net/jdk10/client/jdk/file/4cdd5d954479/src/java.desktop/share/classes/javax/swing/text/html/StyleSheet.java#l120 


Regards
Prasanta
On 6/22/2017 8:38 PM, Sergey Bylokhov wrote:
I do not remember should we describe default implementation via
@implNote or something like that?
The change to the public API for the ButtonModel interface looks OK
to me now, but I am not familiar enough with the rest to review it.
Thanks.

-- Kevin


Prasanta Sadhukhan wrote:
Modified webrev to give default implementation of the new method

http://cr.openjdk.java.net/~psadhukhan/8182577/webrev.03/

Regards
Prasanta
On 6/21/2017 8:49 PM, Kevin Rushforth wrote:
Two quick comments:

1) Is ButtonModel an interface that applications would ever
implement? If so, then it is an incompatible change, since there is no
default implementation of the new method.
2) You should add the '@since 10' javadoc tag to the new method.

-- Kevin


Semyon Sadetsky wrote:
Looks good. You will need to have CCC approval before push.

--Semyon


On 06/21/2017 07:55 AM, Prasanta Sadhukhan wrote:
ok. Modified webrev:

http://cr.openjdk.java.net/~psadhukhan/8182577/webrev.02/

Regards
Prasanta
On 6/21/2017 7:45 PM, Semyon Sadetsky wrote:
Hi Prasanta,

Ii is not necessary to cast to DefaultButtonModel after you
add getGroup() to ButtonModel:
241 ButtonModel model =
((JToggleButton)aComponent).getModel();
--Semyon

On 06/20/2017 10:36 PM, Prasanta Sadhukhan wrote:
Hi Semyon,

Yes, it seems the problem will be there in that case.
Modified to have getGroup() in the interface
http://cr.openjdk.java.net/~psadhukhan/8182577/webrev.01/

Regards
Prasanta
On 6/20/2017 11:16 PM, Semyon Sadetsky wrote:
Hi Prasanta,

With the DefaultButtonModel we can get the same exception if
a custom implementation of the ButtonModel is used.
So, it is better check whether  the model is a
DefaultButtonModel and skip if grouping is not supported. Or, perhaps,
it is reasonable to pull up the getGroup() into the ButtonModel
interface which already has setGroup().
--Semyon


On 06/20/2017 10:21 AM, Prasanta Sadhukhan wrote:
Hi All,

Please review a fix for an issue where a crash is reported
when focus is moved with custom ButtonModel.
Issue was in LayoutFocusTraversalPolicy, the ButtonModel
was wrongly typecasted to JToggleButton when the button model is
DefaultButtonModel, resulting in ClassCastException.
Proposed fix is to cast to super class DefaultButtonModel
and then check for JToggleButton member.
Bug: https://bugs.openjdk.java.net/browse/JDK-8182577
webrev:
http://cr.openjdk.java.net/~psadhukhan/8182577/webrev.00/
Regards
Prasanta