<AWT Dev> <AWT dev>[9] Review request for JDK-8149456: [macosx] robot.keyPress do not generate key events (keyPressed and keyReleased) for function keys F13 to F19.

[Semyon Sadetsky]

+1

On 3/14/2016 12:10 PM, Avik Niyogi wrote:
> Hi Manajit,
>
> The fix looks good to me.
> With Regards,
> Avik Niyogi
>> On 14-Mar-2016, at 2:42 pm, Manajit Halder <manajit.halder at oracle.com 
>> <mailto:manajit.halder at oracle.com>> wrote:
>>
>> 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/%7Earapte/manajit/8149456/webrev.02/>
>>
>> Regards,
>> Manajit
>>
>>
>>> On 14-Mar-2016, at 11:38 am, Semyon Sadetsky 
>>> <semyon.sadetsky at oracle.com <mailto:semyon.sadetsky at 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 <avik.niyogi at 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.halder at oracle.com <mailto:manajit.halder at oracle.com>> wrote:
>>>>>>
>>>>>> Hi All,
>>>>>>
>>>>>> Kindly review the fix for JDK9.
>>>>>>
>>>>>> *Bug*:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8149456
>>>>>>
>>>>>> *Webrev*:
>>>>>> 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
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20160314/3d4ffc9f/attachment-0001.html>