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

[Sergey Bylokhov]

Looks fine.

----- prasanta.sadhukhan at oracle.com 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