<Swing Dev> [10] RFR: 8182577: Crash when Tab key moves focus to a JCheckbox with a custom ButtonModel

[Prasanta Sadhukhan]

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