7189299 DefaultButtonModel instance keeps stale listeners of JButton in case of multiple SwingUtilities.updateComponentTreeUI() calls (original) (raw)

Pavel Porvatov pavel.porvatov at oracle.com
Wed Aug 15 16:19:31 UTC 2012

Hi Frank,

See comments below...

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", "<FORM_ _ACTION="examplescript.cgi"><INPUT type="submit"_ _value="Submit">"); 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.

  1. Could you name the test from lower case? There is no strict rules for that, but most tests starts from "bug..."
  2. You should use the SunToolkit.realSync method to be sure that frame became visible. Take a look in other test, e.g. test/javax/swing/JPopupMenu/7156657
  3. You can reduce code and put try/finally block into single SwingUtilities.invokeAndWait
  4. About the fix: you are removing ALL listeners from public model. That can lead to regressions and this behavior is not expected from users.

    The regression mark was a mistake. If possible, could you clear regression status? Ok, I removed the regression keyword

    http://cr.openjdk.java.net/~dingxmin/7189299/webrev.01/ Please review the new one. Thanks Best regards, Frank

More information about the swing-dev mailing list