<Swing Dev> 7189299 DefaultButtonModel instance keeps stale listeners of JButton in case of multiple SwingUtilities.updateComponentTreeUI() calls

Thu Nov 1 06:35:54 UTC 2012

Hi Alexandr,
   Thanks for your review.  The problem that the JButton does not 
release itself as you mentioned is, after taking closer look at relevant 
code again, that I removed instance AbstractButton.Handler as change 
listener where repainting the button press animation is defined.  So 
this time I get around this issue and here comes v6 patch @
http://cr.openjdk.java.net/~dingxmin/7189299/webrev.06/

   Below are answers to what you have raised in previous email.
1. if ( if (!listener.equals(this)) { that 'this' means the 
JButtonWrapper instead of an action listener.
   'this' refers to JButtonWrapper.this because the closest class 
definition is JButtonWrapper.  I also verified it during my debugging.

2. Why are two FormViews created in this sample?
SwingUtilities.updateComponentTreeUI() triggers html View hierarchy to 
update by instantiating new UI instances involved in the html form.  It 
could have been avoided to have 2 View instances but we are restricted 
to current design that LF change causes creation of View instance which 
further leads to creation of new UI instances.  Creation of new UI 
instances upon LF change can be referenced in BasicTextUI.modelChanged().

3. Why are there 2 listeners added to the button model in this sample?
The issue is caused by combination of the 2 factors.  First, a shared 
ButtonModel is created per html Element.  Second, JButton.setModel() 
automatically registers its private Handler in the new model but no way 
to deregister it.  The first is a design choice but the second is a 
design flaw.  Consequence is that the shared ButtonModel instance bears 
more and more JButton.Handler.

I think v6 patch is promising as a final fix.  If you have any further 
concern or comment, please don't hesitate to let me know. Thanks again 
for your careful review.

Best regards,
Frank

On 10/30/2012 9:38 PM, Alexander Scherbatiy wrote:
> 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
>>