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", "<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.
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
