Many Thanks, Ambarish Rapte
-----Original Message----- From: Sergey Bylokhov Sent: Thursday, September 10, 2015 11:55 PM To: Ambarish Rapte; Philip Race; awt-dev@openjdk.java.net Subject: Re: Request for review fo bug JDK-8039467 [TEST_BUG] Test java/awt/Choice/UnfocusableToplevel/UnfocusableToplevel.java lefts keystrokes in a keyboard buffer on Windows The fix looks fine to me. Please, before the push, split the added comment to align 80 chars. On 10.09.15 17:43, Ambarish Rapte wrote: > Hi, > Thanks for the review comments, Sergey. > > Please review the updated webrev, > > http://cr.openjdk.java.net/~psadhukhan/ambarish/8039467/webrev.01/ > > > Many Thanks, > Ambarish Rapte > > > -----Original Message----- > From: Sergey Bylokhov > Sent: Thursday, September 10, 2015 4:04 PM > To: Ambarish Rapte; Philip Race; awt-dev@openjdk.java.net > Subject: Re: Request for review fo bug JDK-8039467 [TEST_BUG] Test > java/awt/Choice/UnfocusableToplevel/UnfocusableToplevel.java lefts > keystrokes in a keyboard buffer on Windows > > Hi, Ambarish. > The idea of the fix looks fine. > Note that the call to > 72 tempFrameToHoldFocus.requestFocus(); > is not synchronous, so please add these lines after, to be sure that > the temporary window is in focus > > Util.waitForIdle(robot); > Util.clickOnComp(w, robot); > Util.waitForIdle(robot); > > After the first line we will be sure that the temporary window is visible. > After The 2 and 3 lines we will be sure that the window is focused. > > > Also please update the comment to something like this: > > 68 // Note that the Window w is non focusable, key press > events will not be consumed by w, but by any /previously focused/ > window, and this can disturbs the environment. > 69 // So creating tempFrameToHoldFocus frame, to consume key > press events > 70 Frame tempFrameToHoldFocus = new Frame(); > > Or similar text, is not necessary to describe the bug id and bug > summary, this information can be obtained via hg history, but the > general comment is useful. > > > Also you can update the date in the header of the file, but this is > not a strictly necessary: > * Copyright (c) 2007, 2015, Oracle and/or its affiliates. All > rights reserved. > > The first date is a date of creation of the file, the second date is a > date of the latest update. > > On 10.09.15 9:23, Ambarish Rapte wrote: >> Hi, >> >> Please review the following fix for jdk9. >> >> Bug: >> https://bugs.openjdk.java.net/browse/JDK-8039467 >> >> Webrev: >> http://cr.openjdk.java.net/~psadhukhan/ambarish/8039467/webrev.00/ >> >> Issue: >> >> As the test window is non focusable, the automated keyboard events >> are not consumed by the window. >> >> But these keyboard events are getting consumed by any /previously >> focused/ window, & this disturbs the environment. >> >> Fix: >> >> 1.Create /temporary frame/ from test code & set focus on it. >> >> 2.Keep same test code flow for testing unfocusable window. >> >> 3.The automated keyboard events will be consumed by the /temporary >> frame/. This will assure that other window / environment is not affected. >> >> Many Thanks, >> Ambarish Rapte >> > > -- Best regards, Sergey.
