Request for Review: 7170655 frame size change does not follow label font change (original) (raw)

Jonathan Lu luchsh at linux.vnet.ibm.com
Tue May 29 20:07:39 PDT 2012

Hi Charles,

Verified, thanks!

Regards

On 05/30/2012 11:02 AM, Charles Lee wrote:

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/71706554/ 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/71706553/ 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/viewbug.do?bugid=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/71706552/ 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

More information about the awt-dev mailing list