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

[Anthony Petrov]

Hi Dmitry,

The fix looks good to me. As to the test, you should dispose() the frame 
before the if (!result){} statement, so that the test could exit when it 
passes.

--
best regards,
Anthony

On 08/23/2013 04: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
>