Hi Semyon, Alignement is improved and it looks correct on Xcode and Netbeans. Please review the latest webrev:
http://cr.openjdk.java.net/~mhalder/8155740/webrev.05/ <http://cr.openjdk.java.net/~mhalder/8155740/webrev.05/> Thanks, Manajit > On 14-Jun-2016, at 3:17 pm, Semyon Sadetsky <semyon.sadet...@oracle.com> > wrote: > > Hi Manajit, > > Could you improve the alignment of lines 272-286? > > --Semyon > > On 6/13/2016 2:29 PM, Manajit Halder wrote: >> Hi Semyon and Sergey, >> >> Code is modified as per the comment. Please review the modified webrev. >> >> http://cr.openjdk.java.net/~mhalder/8155740/webrev.04/ >> <http://cr.openjdk.java.net/%7Emhalder/8155740/webrev.04/> >> >> webrev.02 did not work because - >> The input nsFlag (modifier value) passed to the function >> NsKeyModifiersToJavaModifiers was not correct. The calculation of modifier >> was not wrong and hence wrong modifier values was returned from the method. >> >> Test cases run: >> Remaining two tests present in >> <https://java.net/jira/browse/MACOSX_PORT>https://java.net/jira/browse/MACOSX_PORT >> <https://java.net/jira/browse/MACOSX_PORT> passed with the fix. >> java/awt/Dialog/NestedDialogs/Modal/NestedModalDialogTest.java >> java/awt/Dialog/NestedDialogs/Modeless/NestedModelessDialogTest.java >> >> Thanks, >> Manajit >> >>> On 01-Jun-2016, at 11:24 pm, Semyon Sadetsky <semyon.sadet...@oracle.com >>> <mailto:semyon.sadet...@oracle.com>> wrote: >>> >>> Hi Manaji, >>> >>> Okay, back to werev.01. >>> >>> Could you remove the comment in lines 262-268 since you assume that it is >>> not true anymore and so CGEventCreateKeyboardEvent/CGEventPost will not >>> cause any troubles. >>> Did you analyze why werev.02 fix did not pass those tests? >>> >>> --Semyon >>> >>> On 6/1/2016 4:46 PM, Manajit Halder wrote: >>>> Hi Semyon and Sergey, >>>> >>>> After running the tests shared by Sergey I found that the second webrev >>>> (webrev.01) shared by me solves the problem. >>>> >>>> http://cr.openjdk.java.net/~mhalder/8155740/webrev.01/ >>>> <http://cr.openjdk.java.net/%7Emhalder/8155740/webrev.01/> >>>> >>>> Following tests were present in >>>> <https://java.net/jira/browse/MACOSX_PORT-621>https://java.net/jira/browse/MACOSX_PORT-621 >>>> <https://java.net/jira/browse/MACOSX_PORT-621>: >>>> closed/java/awt/KeyboardFocusmanager/ConsumeNextMnemonicKeyTypedTest/ConsumeNextMnemonicKeyTypedTest >>>> java/awt/Dialog/NestedDialogs/Modal/NestedModalDialogTest.java >>>> java/awt/Dialog/NestedDialogs/Modeless/NestedModelessDialogTest.java >>>> >>>> But only first test (which is now present as part of open code) could be >>>> performed and the remaining tests were not found in the present code. >>>> The first test fails due to another issue (JDK-8156460), whose fix is in >>>> progress and will be send for after this issue. This fix (JDK-8155740) is >>>> not related to the failure of the first test case. >>>> >>>> The new location of the first test: >>>> java/awt/KeyboardFocusmanager/ConsumeNextMnemonicKeyTypedTest/ConsumeNextMnemonicKeyTypedTest >>>> >>>> Please review the webrev.01. >>>> >>>> Thanks, >>>> Manajit >>>> >>>>> On 26-May-2016, at 1:05 pm, Semyon Sadetsky <semyon.sadet...@oracle.com >>>>> <mailto:semyon.sadet...@oracle.com>> wrote: >>>>> >>>>> On 5/24/2016 2:09 PM, Manajit Halder wrote: >>>>>> Hi Semyon, >>>>>> >>>>>> It is not clear in the comment what was the problem, but it looks like >>>>>> the problem was observed just by using >>>>>> CGEventCreateKeyboardEvent/CGEventPost. In my case I don’t see any >>>>>> issues just by using the fix. It posts all the key events and all the >>>>>> key events are tested in the test file modified along with the fix. >>>>> If you found the comment is not actual anymore, why did you preserve it? >>>>> >>>>> --Semyon >>>>>> >>>>>> Apart from the unknown problem mentioned in the existing comment this >>>>>> fix has following advantages: >>>>>> Key codes for modifier key are required to distinguish between Alt and >>>>>> Alt-Gr key, which is not possible by using existing solution as it >>>>>> doesn’t post key codes for modifier keys. And also keys can’t be >>>>>> distinguished base on modifier value, which is same for both keys (Alt >>>>>> and Alt-Gr). >>>>>> As mentioned in the existing comment >>>>>> CGEventCreateKeyboardEvent/CGEventPost is a better solution. >>>>>> Online search about keyboard simulation showed that >>>>>> CGEventCreateKeyboardEvent/CGEventPost is used to simulate key stores in >>>>>> most of the cases. >>>>>> Tested the key codes, didn’t observe any problem. >>>>>> >>>>>> Regarding number keys not working with >>>>>> CGEventCreateKeyboardEvent/CGEventPost: >>>>>> Solved the problem by providing event source to >>>>>> CGEventCreateKeyboardEvent. Code is modified. >>>>>> >>>>>> Please review the modified code: >>>>>> >>>>>> <http://cr.openjdk.java.net/%7Emhalder/8155740/webrev.02/>http://cr.openjdk.java.net/~mhalder/8155740/webrev.02/ >>>>>> <http://cr.openjdk.java.net/~mhalder/8155740/webrev.02/> >>>>>> >>>>>> Thanks, >>>>>> Manajit >>>>>> >>>>>>> On 20-May-2016, at 12:02 am, Semyon Sadetsky < >>>>>>> <mailto:semyon.sadet...@oracle.com>semyon.sadet...@oracle.com >>>>>>> <mailto:semyon.sadet...@oracle.com>> wrote: >>>>>>> >>>>>>> Hi Manajit, >>>>>>> >>>>>>> Before your fix all key codes wa sent using >>>>>>> >>>>>>> AXUIElementCreateSystemWide(); >>>>>>> >>>>>>> and CGEventCreateKeyboardEvent was commented or excluded from >>>>>>> compilation: >>>>>>> >>>>>>> 274 #if 0 >>>>>>> 275 CGEventRef event = CGEventCreateKeyboardEvent(NULL, keyCode, >>>>>>> keyPressed); >>>>>>> 276 if (event != NULL) { >>>>>>> 277 CGEventPost(kCGSessionEventTap, event); >>>>>>> 278 CFRelease(event); >>>>>>> 279 } >>>>>>> 280 #endif >>>>>>> >>>>>>> I just wonder why it was done. Maybe that was some other issue fix. The >>>>>>> comment above states: >>>>>>> >>>>>>> 262 * Well, using CGEventCreateKeyboardEvent/CGEventPost would >>>>>>> have been >>>>>>> 263 * a better solution, however, it gives me all kinds of >>>>>>> trouble and I have >>>>>>> 264 * no idea how to solve them without inserting delays between >>>>>>> simulated >>>>>>> 265 * events. So, I've ended up disabling it and opted for >>>>>>> another approach >>>>>>> 266 * that uses Accessibility API instead. >>>>>>> >>>>>>> I don't understand what trouble is mentioned in the comment above. But >>>>>>> in your fix you've put the CGEventCreateKeyboardEvent back, may it >>>>>>> return this trouble back? >>>>>>> >>>>>>> Also as I understand Numpad number keys did not work as well. Could you >>>>>>> add the corresponding test case since you provide a fix this extra >>>>>>> issue? >>>>>>> >>>>>>> --Semyon >>>>>>> >>>>>>> >>>>>>> On 5/13/2016 11:50 PM, Manajit Halder wrote: >>>>>>>> Hi Semyon, >>>>>>>> >>>>>>>> The fix is changed a bit because it was observed that the modifier >>>>>>>> keys plus alphabet keys were not working together. In the modified fix >>>>>>>> only Num keys are posted by AXUIElementPostKeyboardEvent and remaining >>>>>>>> keys are posted by CGPostKeyboardEvent/CGEventPost. The fix is >>>>>>>> explained in the comment in file CRobot.m. >>>>>>>> >>>>>>>> Please review the new changes: >>>>>>>> >>>>>>>> <http://cr.openjdk.java.net/%7Emhalder/8155740/webrev.01/>http://cr.openjdk.java.net/~mhalder/8155740/webrev.01/ >>>>>>>> <http://cr.openjdk.java.net/~mhalder/8155740/webrev.01/> >>>>>>>> >>>>>>>> >>>>>>>> Answers to your questions: >>>>>>>> >>>>>>>> What is difference between AXUIElementPostKeyboardEvent and >>>>>>>> CGEventCreateKeyboardEvent? >>>>>>>> >>>>>>>> Difference as per the documentation: >>>>>>>> AXUIElementPostKeyboardEvent is similar to CGPostKeyboardEvent >>>>>>>> (which synthesizes a low-level keyboard event on the local machine), >>>>>>>> but it allows you to specify the target application as opposed to >>>>>>>> always sending the events to the active application. If the >>>>>>>> system-wide accessibility object is passed in the application >>>>>>>> parameter, the event is sent to the active application. >>>>>>>> >>>>>>>> Difference behaviour as per the implementation observed while >>>>>>>> debugging the code: >>>>>>>> >>>>>>>> AXUIElementPostKeyboardEvent: >>>>>>>> AXUIElementPostKeyboardEvent posts 0 key code for all the >>>>>>>> modifier keys with key codes 16, 17,18, 20, 157 and also for newly >>>>>>>> added modifier key VK_ALT_GRAPH. But it posts correct key code for all >>>>>>>> the remaining keys. >>>>>>>> While debugging it was that for modifier keys keyDown and keyUp >>>>>>>> events are not generated, but flagsChanged event (flagsChanged: >>>>>>>> (NSEvent *)event) is generated. But for all other keys keyDown >>>>>>>> followed by keyUp events are generated. >>>>>>>> >>>>>>>> CGEventCreateKeyboardEvent: >>>>>>>> CGEventCreateKeyboardEvent posts correct key code for all the >>>>>>>> keys except for numeric keys (numbers 0 to 9 on normal keyboard). >>>>>>>> CGEventCreateKeyboardEvent posts wrong key codes for the number keys 0 >>>>>>>> to 9. Instead of posting number key codes it posts Numpad key codes >>>>>>>> for the corresponding number key. For example Numpad0 key code is >>>>>>>> posted for number 0, Numpad1 key code is posted for number 1 and >>>>>>>> simillarly for remaining num keys. >>>>>>>> >>>>>>>> Why the latter was commented? Does it mean that valid modifier keys >>>>>>>> have not been sent by AWT robot? >>>>>>>> >>>>>>>> I didn’t get your question clearly. If you meant why in the current >>>>>>>> implementation the later part (fix using CGPostKeyboardEvent) of fix >>>>>>>> was commented. >>>>>>>> I am not very sure about it. As per the comment it is only >>>>>>>> clear that CGPostKeyboardEvent/CGEventPost would have been a better >>>>>>>> solution and I agree with that, perhaps reason could be related to the >>>>>>>> difference in behaviour observed while debugging the code as mentioned >>>>>>>> above. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Manajit >>>>>>>> >>>>>>>>> Hi Manajit, >>>>>>>>> >>>>>>>>> Just a few questions to clarify on the fix. >>>>>>>>> What is difference between AXUIElementPostKeyboardEvent and >>>>>>>>> CGEventCreateKeyboardEvent? >>>>>>>> >>>>>>>>> Why the latter was commented? Does it mean that valid modifier keys >>>>>>>>> have not been sent by AWT robot? >>>>>>>>> >>>>>>>>> --Semyon >>>>>>>>> >>>>>>>>> >>>>>>>>> On 5/12/2016 10:45 AM, Manajit Halder wrote: >>>>>>>>>> Hi All, >>>>>>>>>> >>>>>>>>>> Kindly review the fix for JDK9. >>>>>>>>>> >>>>>>>>>> Bug: >>>>>>>>>> >>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8155740>https://bugs.openjdk.java.net/browse/JDK-8155740 >>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8155740> >>>>>>>>>> >>>>>>>>>> Webrev: >>>>>>>>>> >>>>>>>>>> <http://cr.openjdk.java.net/%7Emhalder/8155740/webrev.00/>http://cr.openjdk.java.net/~mhalder/8155740/webrev.00/ >>>>>>>>>> <http://cr.openjdk.java.net/~mhalder/8155740/webrev.00/> >>>>>>>>>> >>>>>>>>>> Issue: >>>>>>>>>> [macosx] robot.keyPress and robot.keyRelease do not generate key >>>>>>>>>> event for Alt-Graph key VK_ALT_GRAPH. >>>>>>>>>> >>>>>>>>>> Cause: >>>>>>>>>> VK_ALT_GRAPH is a new key added to the Mac OS X platform and it is >>>>>>>>>> not mapped to any key on the OS X platform. >>>>>>>>>> >>>>>>>>>> Fix: >>>>>>>>>> VK_ALT_GRAPH is mapped to right option (OSX_RightOption) key on Mac >>>>>>>>>> OS X. >>>>>>>>>> >>>>>>>>>> Method Java_sun_lwawt_macosx_CRobot_keyEvent is modified for the >>>>>>>>>> following reason: >>>>>>>>>> AXUIElementPostKeyboardEvent posts 0 key code for all the modifier >>>>>>>>>> keys with key codes (16, 17,18, 20, 157) and also for newly added >>>>>>>>>> modifier key VK_ALT_GRAPH. >>>>>>>>>> But it posts correct key code for all the other keys. On the other >>>>>>>>>> hand CGEventCreateKeyboardEvent posts correct key code for all the >>>>>>>>>> modifier keys and >>>>>>>>>> hence it is used to post modifier key events and >>>>>>>>>> AXUIElementPostKeyboardEvent is used to post all the remaining key >>>>>>>>>> events. >>>>>>>>>> >>>>>>>>>> Regards, >>>>>>>>>> Manajit >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
