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

[Alexander Scherbatiy]

On 5/16/2014 7:15 PM, Pete Brunet wrote:
> 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.

    Is it possible to remove the AccessibleFocusHandler class at all and 
initialize the accessibleAWTFocusHandler
    only in the AccessibleAWTFocusHandler.addPropertyChangeListener() 
method?

   Thanks,
   Alexandr.

>
> 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