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

[Luke Usherwood]

Hi,

Sorry for the late reply - I've been travelling. As the bug reporter, 
would you mind if I gave a few passing thoughts?

On 6/20/2017 11:16 PM, Semyon Sadetsky wrote:
 > Hi Prasanta,
 >
 >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().

On one hand, it seems the best default methods are the ones able to 
produce correct behaviours w.r.t. the other interface methods when 
retrofitted, to avoid any 'surprises'. For example if some other code 
decides to switch from DefaultButtonModel to accept any ButtonModel - 
that would happily compile but it might be easy to miss the changed 
semantics: that calling getGroup may not necessarily give back a value 
passed in to setGroup any more, and hence there's scope for a subtle 
runtime crash bug to be introduced.

Also, would the change that pulls up a new default method into 
ButtonModel be permitted if this fix were back-ported to JDK-9?

On the other hand I suppose there's no other choice if you want to patch 
this oversight in the interface design (which looks like it was wanted 
since Java 1.3!) ... so this is the trade-off you've taken. The 
likelihood of an actual crash in practice is low, and more so because 
most calling code would test for a null value anyway. So I guess on 
balance this may be reasonable.

If you do proceed with default method solution...

1)  should the javadoc for state that all implementers are expected to 
override the default implementation of getGroup(), and callers should 
always test for a null value (even when that might not be expected) to 
account for legacy implementations where the method might not be 
implemented?

2) will you change similar code in the JDK - such as found in 
JToggleButton.getGroupSelection() - to avoid the check-and-cast to 
DefaultButtonModel. This would seem required to ensure that all 
ButtonModels implementing getGroup() are treated consistently to other 
DefaultButtonModel instances.

Kind regards,
Luke