Hi Semyon & Avik, Thanks to both of you for the review.
Please review the modified the code. http://cr.openjdk.java.net/~arapte/manajit/8149456/webrev.02/ <http://cr.openjdk.java.net/~arapte/manajit/8149456/webrev.02/> Regards, Manajit > On 14-Mar-2016, at 11:38 am, Semyon Sadetsky <semyon.sadet...@oracle.com> > wrote: > > Hi Manajit, > > The fix itself looks good. > Several minor remarks: > - In the test setting GridBagLayout to the frame seems meaningless. > - The year in the files header should be 2016. > - For the future, add "<AWT dev> [9]" in the beginning of review subject. > > --Semyon > > On 3/9/2016 4:43 PM, Manajit Halder 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/%7Earapte/manajit/8149456/webrev.01/> >> >> Thanks, >> Manajit >> >> >>> On 09-Mar-2016, at 3:18 pm, Avik Niyogi < >>> <mailto:avik.niy...@oracle.com>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 >>>> <https://bugs.openjdk.java.net/browse/JDK-8149456> >>>> >>>> Webrev: >>>> >>>> <http://cr.openjdk.java.net/%7Earapte/manajit/8149456/webrev.00/>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 >>>> >>> >> >
