<Swing Dev> Request for review, JDK-8009883, REGRESSION: test/closed/javax/swing/AbstractButton/4246045/bug4246045.java fails

[Pete Brunet]

On 5/16/14 6:45 AM, Alexander Scherbatiy wrote:
>
>   Hi Peter,
>
>   Is there any difference between AccessibleAWTFocusHandler and
> AccessibleFocusHandler classes?
Hi Alexandr, The former is the focus handler used by the inner class
accessibility support for an AWT Component and the latter is the focus
handler used by the inner class accessibility support for a JComponent.

The code is the same.

The former code is this:

        protected class AccessibleAWTFocusHandler implements FocusListener {
            public void focusGained(FocusEvent event) {
                if (accessibleContext != null) {
                    accessibleContext.firePropertyChange(
                                                        
AccessibleContext.ACCESSIBLE_STATE_PROPERTY,
                                                         null,
AccessibleState.FOCUSED);
                }
            }
            public void focusLost(FocusEvent event) {
                if (accessibleContext != null) {
                    accessibleContext.firePropertyChange(
                                                        
AccessibleContext.ACCESSIBLE_STATE_PROPERTY,
                                                        
AccessibleState.FOCUSED, null);
                }
            }
        }

and the latter code is this

        protected class AccessibleFocusHandler implements FocusListener {
           public void focusGained(FocusEvent event) {
               if (accessibleContext != null) {
                    accessibleContext.firePropertyChange(
                        AccessibleContext.ACCESSIBLE_STATE_PROPERTY,
                        null, AccessibleState.FOCUSED);
                }
            }
            public void focusLost(FocusEvent event) {
                if (accessibleContext != null) {
                    accessibleContext.firePropertyChange(
                        AccessibleContext.ACCESSIBLE_STATE_PROPERTY,
                        AccessibleState.FOCUSED, null);
                }
            }
        }
>
>   Thanks,
>   Alexandr.
>
> On 5/7/2014 5:45 AM, Pete Brunet wrote:
>> Hi Swing and AWT teams, (and Artem since you were involved in the
>> 7179482 fix),
>>
>> I'd appreciate your review for a fix to a regression in
>> java.awt.Component and javax.swing.JComponent.
>>
>> Original bug: https://bugs.openjdk.java.net/browse/JDK-7179482
>> Orignal CCC: 7179482
>> Regression Bug: https://bugs.openjdk.java.net/browse/JDK-8009883
>> Regression Fix Webrev:
>> http://cr.openjdk.java.net/~ptbrunet/JDK-8009883/webrev.00/
>>
>> Problem Description:
>> jdk/test/closed/javax/swing/AbstractButton/4246045/bug4246045.java tests
>> to make sure the focus state changes whenever a JButton or JToggleButton
>> property change listener is called.  The problem in this case is that
>> for each change of focus the listener is called twice.  The first time
>> there is no problem because for example the component is not focused and
>> then it is, but the second time the listener is called the component is
>> already focused and the new state is also focused so the test case
>> detects that the focus state didn't change.
>>
>> Root cause:
>> There are two focus gained and two focus lost events fired when
>> JComponents gain/loose focus.  Part of the fix for JDK-7179482 was to
>> deprecate the JComponent.AccessibleJComponent.accessibleFocusHandler
>> field noting that
>> Component.AccessibleAWTComponent.accessibleAWTFocusHandler should be
>> used instead.  The problem is that this was not done when 7179482 was
>> fixed.  The end result is that two focus listeners are added when
>> JComponent.AccessibleJComponent.addPropertyChangeListener is called.
>> The first listener is added in that property change method and the
>> second one is added when the superclass method is called, i.e.
>> Component.AccessibleAWTComponent.addPropertyChangeListener.  The two
>> listeners cause two focus events with the second one not causing a state
>> change.
>>
>> Proposed fix:
>> The proposed fix is for
>> JComponent.AccessibleJComponent.addPropertyChangeListener to set its
>> listener into Component.AccessibleAWTComponent.accessibleAWTFocusHandler
>> so when the superclass method,
>> Component.AccessibleAWTComponent.addPropertyChangeListener is called the
>> listener at that level will already be set.
>>
>> A side effect of the fix is that logic was needed to be added in
>> Component.AccessibleAWTComponent.addPropertyChangeListener to deal with
>> the fact that the
>> Component.AccessibleAWTComponent.accessibleAWTFocusHandler field can be
>> set either from within the object's subclass method
>> JComponent.AccessibleJComponent.addPropertyChangeListener or via an
>> external call to Component.AccessAWTComponent.addPropertyChangeListener
>> for cases other than when there is a JComponent subclass of Component.
>> In the former case the listener is already set by the subclass so a
>> focus listener isn't added.  In the latter case the listener is not yet
>> added so a focus listener is new'd and added.
>>
>> There are similar considerations when removing a listener; this can be
>> seen in the webrev.
>>
>> There may be a better way to fix this but the proposed fix does resolve
>> the issue that the test case exposed.
>>
>> Best regards,
>> Pete
>