<Swing Dev> [8] Review request for 8023474: First mousepress doesn't start editing in JTree

[dmitry markov]

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