<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

[Ambarish Rapte]

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.