<AWT Dev> Request for review fo bug JDK-8039467 [TEST_BUG] Test java/awt/Choice/UnfocusableToplevel/UnfocusableToplevel.java lefts keystrokes in a keyboard buffer on Windows

[Alexander Scherbatiy]

   The fix looks good to me.

   Thanks,
   Alexandr.

On 9/11/2015 2:54 PM, Ambarish Rapte wrote:
> Hi,
> 	Thanks for the review, Sergey.
>
> 	I require one more review for this patch, Can someone please review this.
> 	Webrev: http://cr.openjdk.java.net/~psadhukhan/ambarish/8039467/webrev.02/
>
>
> Many Thanks,
> Ambarish Rapte
>
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Thursday, September 10, 2015 11:55 PM
> To: Ambarish Rapte; Philip Race; awt-dev at 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 at 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.