<Swing Dev> [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.sadhukhan at 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
>>
>