Hi Frank,
I tried your V5 patch and run the test described in the issue (see below).
I pressed the button and it does not release. This is because the
AbstractButton.Handler listener is removed because of the fix.
You could check the line
941 if (!listener.equals(this)) {
that 'this' means the JButtonWrapper instead of an action listener.
Could you also look deeper in the code and say why two FormViews are
created (and each adds it's own listener to the button model) in this
sample?
-----------------------------------------------
import java.awt.*;
import javax.swing.*;
public class FormSubmit {
public static void main(String[] args) throws Exception {
SwingUtilities.invokeAndWait(new Runnable() {
@Override
public void run() {
JEditorPane html = new JEditorPane("text/html",
"<html><body><FORM ACTION=\"examplescript.cgi\"><INPUT type=\"submit\"
value=\"Submit\"></FORM></BODY></HTML>");
JFrame frame = new JFrame();
frame.setLayout(new BorderLayout());
frame.add(html, BorderLayout.CENTER);
frame.setSize(200, 100);
frame.setDefaultCloseOperation(frame.EXIT_ON_CLOSE);
frame.setVisible(true); // Uncomment this line to see
the Exception on JDK 7
SwingUtilities.updateComponentTreeUI(html);
}
});
}
}
-----------------------------------------------
Thanks,
Alexandr.
On 10/29/2012 11:48 AM, Frank Ding wrote:
Hi all,
Seems Pavel has been busy these days. Is anybody willing to review
v5 change?
Thanks,
Frank
On 10/17/2012 1:51 PM, Frank Ding wrote:
Hi Pavel,
Thanks for your comment pointing out the problems posed by static
fields. V4 cannot be an accepted work around unless static fields
are avoided. But I think in v4 removing stale listeners is only
applied to those installed by FormView.
I have revised the approach adopted in v4 where a JButtonWrapper
class is declared for the purpose of accessing actionListener,
changeListener and itemListener defined in its base class. In
addition, overriding setModel method in JButtonWrapper allows us to
undo registration of aforementioned listeners on the newModel passed
in as setModel method parameter. In order to have FormView function
correctly in terms of triggering the actionPerformed method defined
in FormView, setModel also explicitly add FormView.this as action
listener to the newModel. Further more, setModel deregisters any
action listeners of class FormView because that is the stale action
listener registered by setModel last time. In doing all the tricks
in setModel, we successfully achieved the goal of removing stale
listener while making FormView JButton properly function.
By the way, I also updated the jtreg test case bug7189299.java a
little bit to assert number of item listeners and change listeners
because in setModel, the default changeListener and itemListener
pertaining to the JButton are removed as well.
The v5 code review is @
http://cr.openjdk.java.net/~dingxmin/7189299/webrev.05/
Your review is highly appreciated.
Thanks and best regards,
Frank
On 10/15/2012 5:16 PM, pavel porvatov wrote:
Hi Frank,
You are still removing all listeners, not only installed by FormView.
You also introduced new static field (private static JButtonWrapper
currJButton = null), but there are a lot problems with static fields:
1. Memory leaks: static fields hold reference to potentially huge
objects unlimited time
2. Conflicts between applications from different AppContext (e. g.
applets)
etc.
So the best way is to avoid static fields at all.
Regards, Pavel
Hi Pavel,
I happened to have another way of working around it though
admittedly it's ugly. Actually a good solution instead of a
workaround requires either involving large code modification or
introduction of new api.
Idea of v4 change this time is keeping track of every
JButtonWrapper instance once instantialized so that the listeners
pertaining to stale JButton instance can be de-registered upon new
JButton creation.
You can check out code review below
http://cr.openjdk.java.net/~dingxmin/7189299/webrev.04/
Your prompt comment is warmly welcomed and expected.
Best regards,
Frank
