On 5/22/2014 9:59 PM, Pete Brunet wrote:
I'd like one more reviewer of this fix.
Also I removed the @Deprecated and will deal with this in a following
JBS issue.
http://cr.openjdk.java.net/~ptbrunet/JDK-8009883/webrev.04/
The fix looks good for me.
Thanks,
Alexandr.
Pete
On 5/21/14 10:08 AM, Pete Brunet wrote:
On 5/21/14 7:04 AM, Alexander Scherbatiy wrote:
On 5/21/2014 2:56 PM, Alexander Scherbatiy wrote:
On 5/20/2014 1:10 AM, Pete Brunet wrote:
On 5/16/14 11:04 AM, Pete Brunet wrote:
On 5/16/14 10:45 AM, Alexander Scherbatiy wrote:
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?
This looks like it will work and I would like that solution a lot
better. I'll give it a try and if that works out I'll provide a new
webrev. Thanks Alexandr.
Hi Alexandr and AWT/Swing teams,
Please review the new webrev at
http://cr.openjdk.java.net/~ptbrunet/JDK-8009883/webrev.01/
The fix looks good for me.
Sorry. I just realize that the AccessibleFocusHandler class has
protected access and can't be removed because of the compatibility
reasons.
http://docs.oracle.com/javase/7/docs/api/javax/swing/JComponent.AccessibleJComponent.AccessibleFocusHandler.html
All others looks good for me. Just leave the
AccessibleFocusHandler class from AccessibleJComponent as is.
Thanks very much for catching that Alexandr! The new webrev is here:
http://cr.openjdk.java.net/~ptbrunet/JDK-8009883/webrev.03/
Note that I added a @Deprecated tag (and javadoc tag). Hopefully that's
the correct thing to do.
I'd like to list one more reviewer on the patch so I'd appreciate one
more set of eyes on this.
Pete
Thanks,
Alexandr.
Thanks,
Alexandr.
Pete
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
