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.

Reply via email to