Hi Manajit, The fix looks good to me. With Regards, Avik Niyogi
> On 09-Mar-2016, at 7:13 pm, Manajit Halder <manajit.hal...@oracle.com> wrote: > > Hi All, > > Please review webrev.01 updated with Avik’s review comments. > > http://cr.openjdk.java.net/~arapte/manajit/8149456/webrev.01/ > <http://cr.openjdk.java.net/~arapte/manajit/8149456/webrev.01/> > > Thanks, > Manajit > > >> On 09-Mar-2016, at 3:18 pm, Avik Niyogi <avik.niy...@oracle.com >> <mailto:avik.niy...@oracle.com>> wrote: >> >> Hi Manajit, >> >> Following are my inputs: >> CRobot.m : Line 359 : retval not needed >> alphabetical objC header list convention not followed. Line 36 should be >> placed at line 32 >> CRobotKeyCode.m : Line 171: retval not needed. use else block for cases >> undefined. >> Use non-Swing implementations for test case. >> >> With Regards, >> Avik Niyogi >>> On 09-Mar-2016, at 2:52 pm, Manajit Halder <manajit.hal...@oracle.com >>> <mailto:manajit.hal...@oracle.com>> wrote: >>> >>> Hi All, >>> >>> Kindly review the fix for JDK9. >>> >>> Bug: >>> https://bugs.openjdk.java.net/browse/JDK-8149456 >>> <https://bugs.openjdk.java.net/browse/JDK-8149456> >>> >>> Webrev: >>> http://cr.openjdk.java.net/~arapte/manajit/8149456/webrev.00/ >>> <http://cr.openjdk.java.net/~arapte/manajit/8149456/webrev.00/> >>> >>> Issue: >>> [macosx] robot.keyPress do not generate key events (keyPressed and >>> keyReleased) for function keys F13 to F19. >>> >>> Cause: >>> robot.keyPress(keyCode) passes java key code for the particular key >>> pressed to native side (MacOS X). On the MacOS corresponding key code >>> matched and returned to java side. But as per the current implementation >>> keys with java key code below 222 returns a valid key code form MacOS X and >>> for all java key codes above 222 MacOS don’t have any valid key code to >>> return and it returns a default value 127 (undefined) for all of them >>> including function keys F13 to F20. The java key codes for keys F13 to F19 >>> is in range 61440 to 61447. >>> >>> Fix: >>> Create a dictionary of java keys and OS X keys with: >>> key: java key >>> value: OS X key >>> >>> Use the macros defined as macros in header java_awt_event_KeyEvent.h for >>> java keys >>> Define integer constants for OS X keys as shown below: >>> >>> const static int OSX_kVK_ANSI_A = 0x00; >>> const static int OSX_kVK_ANSI_S = 0x01; >>> const static int OSX_kVK_ANSI_D = 0x02; >>> const static int OSX_kVK_ANSI_F = 0x03; >>> >>> Problem in modifying/extending current fix: >>> Why the current fix can’t be extended/modified to support keys F13 to >>> F20. >>> >>> 1) The current fix creates an array with array index as java key and array >>> value as corresponding OS X key. >>> static const unsigned char javaToMacKeyCode[] = { >>> 127, // 0 0 VK_UNDEFINED No_Equivalent >>> 127, // 1 0x1 Not_Used >>> 127, // 2 0x2 Not_Used >>> >>> Here 0, 1 and 2 are the java keys and 127, 127 and 127 are the >>> corresponding key values in OS X for the same key. >>> The array can’t be modified to add support for F13 to F20 because their >>> corresponding values are 61440 to 61447. >>> It won’t be a good solution to extended the array with index 61440 to 61447. >>> >>> 2) The current fix creates an array using a pearl script. The pearl script >>> uses hardcoded values of java keys and OS X keys. >>> Also the Pearl script is not checked in to the java codebase. >>> >>> 3) The current fix not modifiable and not readable. >>> >>> Time comparisons between with and without fix in milliseconds: >>> The time comparison is done by taking the time diff in milliseconds >>> before and after calling robot keypress for all the keys using the test >>> code written for the fix. >>> Time diff taken for 5 test runs: >>> >>> With fix: 1658, 1698, 1705, 1453 and 1516 >>> Without fix: 1523, 1579, 1690, 1648 and 1504 >>> >>> Note: >>> The following java keys are not matched in the current fix because of >>> the following reasons: >>> 1) They are already matched with their corresponding keys on >>> Mac OS. >>> 2) There are no defined keys on Mac OS with the same >>> functionality for PRINTSCREEN, SCROLL LOCK and PAUSE. >>> 3) The following java keys are not defined in keyTable in >>> AWTEvent.m file. >>> >>> java_awt_event_KeyEvent_VK_PRINTSCREEN >>> java_awt_event_KeyEvent_VK_SCROLL_LOCK >>> java_awt_event_KeyEvent_VK_PAUSE >>> >>> In the previous fix these keys were matched to OSX_F14, OSX_F13 and >>> OSX_F15 respectively. >>> >>> >>> Regards, >>> Manajit >>> >> >
