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

Tue Aug 14 03:06:08 UTC 2012

On 8/6/2012 10:30 PM, Pavel Porvatov wrote:
> Hi Frank,
>> Hi guys,
>>   There is a bug in FormView class which can be best illustrated by 
>> the following test case.
>>
>>   1. First create a JEditorPane that has a "submit" input type in the 
>> "FORM" tag,
>>   JEditorPane html = new JEditorPane("text/html", "<html><body><FORM 
>> ACTION=\"examplescript.cgi\"><INPUT type=\"submit\" 
>> value=\"Submit\"></FORM></BODY></HTML>");
>>   2. And call SwingUtilities.updateComponentTreeUI(html).
>>   3. Now a NPE is thrown when the "submit" button is clicked, which 
>> is apparently a bug.
>>
>>   I filed a sun bug 7189299 to track the bug.  A complete runnable 
>> Java test throwing NPE is available in that sun bug. The root cause 
>> is that SwingUtilities.updateComponentTreeUI() triggers FormView 
>> instance to call its member method createInputComponent(AttributeSet 
>> attr, Object model) that instantializes JButton instance.  Then, 
>> immediately after the new JButton instance is created, a 
>> DefaultButtonModel instance that is kept in AttributeSet is used to 
>> replace innate button model by calling 
>> button.setModel((ButtonModel)model).  Tracing into setModel method, 
>> several listeners linking to the new JButton instance are registered 
>> on the shared button model. However, there are no un-registration 
>> calls to remove any previously registered listeners pertaining to 
>> stale JButton instance.
>>
>>   The bug applies to "submit", "reset", "image", "checkbox", "radio" 
>> html types because they call AbstractButton.setModel eventually. But 
>> seems easy to manifest the bug in observable way with only "submit" 
>> type, for example, the NPE above.  I wrote another java test in 
>> webrev that asserts number of listeners, which can be applied to all 
>> affected html types.
>>
>>   Please take a look at its patch and test @
>>   http://cr.openjdk.java.net/~youdwei/ojdk-138/webrev.00/
>>
>>   Your comment and effort are highly appreciated.
> I've accepted the bug. I see that you marked bug as a regression. Did 
> you investigated in which release the problem appeared and which fix 
> introduced the regression?
>
> I didn't reviewed you fix, but I have two comments:
> 1. What is the reason to name the test FormImageTestV3? Why FormImage? 
> What is th V3?
> 2. Could you please put webrevs into folders with the bugID, but not 
> into ojdk-138?
>
> Regards, Pavel
>
Hi Pavel,
   Thanks for your comments.  I have revised the test and uploaded the 
webrev review package to the following folder.  Now the test name and 
folder name have been corrected according to your comments.  The 
regression mark was a mistake.  If possible, could you clear regression 
status?

http://cr.openjdk.java.net/~dingxmin/7189299/webrev.01/

   Please review the new one.  Thanks

Best regards,
Frank