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

[Semyon Sadetsky]

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 
>> <mailto: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/ 
>>> <http://cr.openjdk.java.net/%7Earapte/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/d42f7f6b/attachment-0001.html>