Hi All,

Please review webrev.01 updated with Avik’s review comments. 

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> 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
>> 
> 

Reply via email to