<AWT Dev> Request for Review: 7170655 frame size change does not follow label font change

[Charles Lee]

Hi Jonathan,

The patch is committed @

Changeset: 3c9adc88956d
Author:    luchsh
Date:      2012-05-30 10:58 +0800
URL:http://hg.openjdk.java.net/jdk8/awt/jdk/rev/3c9adc88956d

7170655: Frame size does not follow font size change with XToolkit
Reviewed-by: serb, art

Please verify it and thank you all for reviewing the patch.

On 05/29/2012 01:23 PM, Jonathan Lu wrote:
> Hi Artem,
>
> Thanks for review!
>
> On 05/28/2012 09:12 PM, Artem Ananiev wrote:
>> Hi, Jonathan,
>>
>> the fix looks fine.
>>
>> The test requires some corrections, though:
>>
>> 1. "@bug 7150655" should be replaced with "@bug 7170655".
> Sorry for the typo.
>>
>> 2. Most of the tests in test/java/awt/ have some "meaningful" names, 
>> e.g. LabelFontToAffectFrameSize, so every developer can easily 
>> understand what a test is about.
>>
> In the new patch, I've renamed it to "ResizeAfterSetFont".
>
>> 3. Instead of Thread.sleep() calls, I would recommend to use the 
>> test.java.awt.regtesthelpers.Util class. An example can be found here:
>>
>> test/java/awt/Window/WindowType/WindowType.java
>
> Thanks for the reference, I've updated it in the new patch, see
>
> http://cr.openjdk.java.net/~luchsh/7170655_4/
>
> Best regards!
> - Jonathan
>
>>
>> Thanks,
>>
>> Artem
>>
>> On 5/28/2012 9:17 AM, Jonathan Lu wrote:
>>> Hello Sergey,
>>>
>>> On 05/26/2012 03:32 AM, Sergey Bylokhov wrote:
>>>> Hi, Jonathan.
>>>> Fix looks good to me. Just note that one more reviewer needed.
>>>
>>> Thanks for review.
>>>
>>> Can anybody please help to take another look?
>>>
>>>>
>>>> Thanks for the fix.
>>>
>>> Best regards!
>>> - Jonathan
>>>> 24.05.2012 11:50, Jonathan Lu wrote:
>>>>> Hello Sergey,
>>>>>
>>>>> I made one automatic jtreg test case and put it to following link
>>>>> along with the patch.
>>>>> http://cr.openjdk.java.net/~luchsh/7170655_3/
>>>>>
>>>>> Could you please take a look?
>>>>>
>>>>> Thanks!
>>>>> - Jonathan
>>>>>
>>>>> On 05/23/2012 08:34 PM, Sergey Bylokhov wrote:
>>>>>> Hi, Jonathan,
>>>>>> Fix looks good. But I guess this bug was not caught by the
>>>>>> regression tests, so new test should be added(automatic test is
>>>>>> better).
>>>>>>
>>>>>> 23.05.2012 14:15, Jonathan Lu wrote:
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> On 05/22/2012 10:23 PM, Sergey Bylokhov wrote:
>>>>>>>> Hi, Jonathan,
>>>>>>>> Looks like this bug is duplicate of
>>>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7161865
>>>>>>>> And the reason of the issue is that we post UpdateEvent in
>>>>>>>> target.repaint() in XLabelPeer.setFont(), but we should paint
>>>>>>>> native part of the component in place[1] and then post 
>>>>>>>> PaintEvent[2].
>>>>>>>>
>>>>>>>> - To fix [1] we can replace target.repaint() to repaint() in
>>>>>>>> setFont(). Does it solve your issue?
>>>>>>>
>>>>>>> Yes, this resolves my problem, and here's the updated patch from
>>>>>>> your suggestion,
>>>>>>> http://cr.openjdk.java.net/~luchsh/7170655_2/
>>>>>>>
>>>>>>> Thanks a lot!
>>>>>>> - Jonathan
>>>>>>>
>>>>>>>>
>>>>>>>> 22.05.2012 11:47, Jonathan Lu wrote:
>>>>>>>>> Hi awt-dev,
>>>>>>>>>
>>>>>>>>> Here's a patch for bug 7170655, could anybody please help to take
>>>>>>>>> a look?
>>>>>>>>> http://cr.openjdk.java.net/~luchsh/7170655/
>>>>>>>>>
>>>>>>>>> The problem is that painting event from EDT during painting
>>>>>>>>> handling does not get processed immediately, so leave a lag to
>>>>>>>>> the user. My solution here is to coalesce and dispatch the new
>>>>>>>>> paint event right after it was posted. This happens only to AWT
>>>>>>>>> components.
>>>>>>>>>
>>>>>>>>> Thanks!
>>>>>>>>> -Jonathan
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>


-- 
Yours Charles