Hello Dmitry
The fix looks good to me as well
I should say that the final version is much better than the first one,
thank you Anthony for the thorough review!
alexp
On 8/23/2013 4:18 PM, dmitry markov wrote:
Hi Anthony,
Thank you for your comments. I changed the fix based on your
suggestions. Please find new webrev at
http://cr.openjdk.java.net/~dmarkov/8023474/webrev.01/.
Could you review it, please?
Thanks,
Dmitry
On 22/08/2013 17:28, Anthony Petrov wrote:
Hi Dmitry,
A few comments:
1. The concept of validate roots has been extended to AWT components
since JDK 7. Therefore, I suggest to use the same logic regardless of
whether the editor is an instance of JComponent or not.
2. What you're trying to implement here, is actually called
revalidateSynchronously(), as opposed to the regular revalidate()
method, which in Swing is asynchronous. I believe that rS() might be
very much useful in many cases. Therefore, I suggest to add a
package-private java.awt.Component.revalidateSynchronously(), and
call it via the AWTAccessor from BasicTreeUI.java. Note that the
current Component.revalidate() should simply call the new rS()
directly, and the rS() may reuse the current implementation of the
Component.revalidate().
3.
2230 // The implementation of the method is copied from
SwingUtilities
Copying an implementation is almost always wrong. It might be better
to access a method from another package via e.g. an accessor, or
reflection, or otherwise find a way to share this code rather than
copy it. However, I think you won't need this code anyway if we
implement the suggestion #2 above.
4. Is there a similar problem with JTable custom editors? What else
Swing components allow for editors?
As far as I know, JTable does not have this problem.
--
best regards,
Anthony
On 08/22/13 15:08, dmitry markov wrote:
Hello,
Could you review the fix, please?
bug: http://bugs.sun.com/view_bug.do?bug_id=8023474
webrev: http://cr.openjdk.java.net/~dmarkov/8023474/webrev.00/
The method BasicTreeUI.startEditing() should find the first valid root
for the editingComponent and call validateUnconditionally() for it
instead of editingComponent.revalidate() invocation.
Thanks,
Dmitry
