<Swing Dev> 7189299 DefaultButtonModel instance keeps stale listeners of JButton in case of multiple SwingUtilities.updateComponentTreeUI() calls
Frank Ding
dingxmin at linux.vnet.ibm.com
Fri Nov 2 09:45:49 UTC 2012
Hi Alexandr,
Thanks for your quick reply. I will refine the patch and address all
your concerns ASAP.
Best regards,
Frank
On 11/1/2012 8:08 PM, Alexander Scherbatiy wrote:
> On 11/1/2012 10:35 AM, Frank Ding wrote:
>> 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.
>
> Could you give an example there this check is useful? It seems
> that the JButtonWrapper does not implement ActionListener interface
> and so the listener and the this always should not be equal.
>
>
> I dumped action listeners from the button and model before the fix
> and after it.
> Before the fix the button has the latest FormView listener and the
> model has AbstractButton$Handler added each time after a component
> input creation.
> After the fix both the button and the model have the FormView
> listener and the button does not have the AbstractButton$Handler.
> Could you preserve the listeners location and just remove old
> listeners from the model?
>
> The fix sets the model to the JButtonWrapper first and than removes
> unneccessary listeners from the model. Is it possible to rearrange
> order of these operations?
>
> I see that you check number of the listeners in the test. Could you
> add the check that the action listener is invoked only once after a
> component tree updating and
> the action does the same that it does before a component tree
> updating.
>
> Thanks,
> Alexandr.
>>
>> 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
>>>>
>>
>
More information about the swing-dev
mailing list