Looks fine. ----- prasanta.sadhuk...@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.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
