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
