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

[Prasanta Sadhukhan]

Since CSR is approved, and if there is no objection to this, I will 
commit this fix today.

Regards
Prasanta
On 6/29/2017 10:37 AM, Prasanta Sadhukhan wrote:
> 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
>>>
>>
>