Thanks Prasanta for the review. I have corrected the indentation problem. Please review the updated webrev. http://cr.openjdk.java.net/~mhalder/8216971/webrev.02/ <http://cr.openjdk.java.net/~mhalder/8216971/webrev.02/>
Regards, Manajit > On 18-Mar-2019, at 11:21 AM, Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com> wrote: > > I guess l94-111 indentation in CMenuItem.m and l69 Robot robot = new Robot(); > in testcase > is still not fixed. > Regards > Prasanta > On 15-Mar-19 4:51 PM, Manajit Halder wrote: >> Thanks Prasanta, >> >> I have corrected the indentation issue in both files. Please review the >> modified webrev. >> http://cr.openjdk.java.net/~mhalder/8216971/webrev.01/ >> <http://cr.openjdk.java.net/%7Emhalder/8216971/webrev.01/> >> >> Regards, >> Manajit >> >> >>> On 15-Mar-2019, at 11:32 AM, Prasanta Sadhukhan >>> <prasanta.sadhuk...@oracle.com <mailto:prasanta.sadhuk...@oracle.com>> >>> wrote: >>> >>> fix looks fine to me but there are indentation issue both in fix and in >>> testcase. Please rectify it. >>> Regards >>> Prasanta >>> On 12-Mar-19 6:46 PM, Krishna Addepalli wrote: >>>> Hi Manajit, >>>> >>>> Thanks for the clarification. The fix looks ok to me. >>>> >>>> Thanks, >>>> Krishna >>>> >>>>> On 12-Mar-2019, at 3:36 PM, Manajit Halder <manajit.hal...@oracle.com >>>>> <mailto:manajit.hal...@oracle.com>> wrote: >>>>> >>>>> Lines 74 to 82 explains why we need to ignore this call. This method >>>>> should be ignored if it is called as a result of user pressing a >>>>> shortcut and the window containing the menu is not minimized. >>>>> >>>>> Regards, >>>>> Manajit >>>>> >>>>> >>>>>> On 12-Mar-2019, at 3:16 PM, Krishna Addepalli >>>>>> <krishna.addepa...@oracle.com <mailto:krishna.addepa...@oracle.com>> >>>>>> wrote: >>>>>> >>>>>> Hi Manajit, >>>>>> >>>>>> Thanks for the clarification. I think you should add some more comments >>>>>> around the statement at line86, to explain in more detail, about why to >>>>>> ignore this call. >>>>>> >>>>>> Thanks, >>>>>> Krishna >>>>>> >>>>>>> On 11-Mar-2019, at 2:25 PM, Manajit Halder <manajit.hal...@oracle.com >>>>>>> <mailto:manajit.hal...@oracle.com>> wrote: >>>>>>> >>>>>>> Hi Krishna, >>>>>>> >>>>>>> Thanks for the review comment. >>>>>>> >>>>>>> The key mapping is done by method setKeyEquivalent on fMenuItem (object >>>>>>> of the NSMenuItem) in the same file. >>>>>>> >>>>>>> As discussed with you, the second key event is the problem here, and is >>>>>>> caused only when System property “apple.laf.useScreenMenuBar” is set to >>>>>>> true. The extra event is generated in the handleAction method and my >>>>>>> proposed fix is solving this issue. The difference with other look and >>>>>>> feel setting or when “apple.laf.useScreenMenuBar” is set to “false” is >>>>>>> that handleAction method is not called. I have verified and found that >>>>>>> the META_MASK and CTRL_MASK are only set when >>>>>>> “apple.laf.useScreenMenuBar” is set to true and not in case of it is >>>>>>> false. Also verified with “metal” look and feel and found the MASKS are >>>>>>> not set and handleAction method is not called and hence the extra key >>>>>>> event is not generated. >>>>>>> >>>>>>> Please let me know if you have any other query. >>>>>>> >>>>>>> Regards, >>>>>>> Manajit >>>>>>> >>>>>>> >>>>>>> >>>>>>>> On 05-Mar-2019, at 4:52 PM, Krishna Addepalli >>>>>>>> <krishna.addepa...@oracle.com <mailto:krishna.addepa...@oracle.com>> >>>>>>>> wrote: >>>>>>>> >>>>>>>> Hi Manajit, >>>>>>>> >>>>>>>> Per our discussion, The cause of the problem is : 1), Key Event being >>>>>>>> sent from the OS to the application - which the Java layer processes >>>>>>>> it correctly >>>>>>>> 2) The Mac OS calling the handleAction function directly on the >>>>>>>> NSMenutItem - although as per your description, there is no code which >>>>>>>> maps the hot key to this widget in the native layer. >>>>>>>> Ideally, since the OS is recognising the key combination, that key >>>>>>>> event should not be delivered again to the application. Or, it should >>>>>>>> be that the key event is not recognised and hence delivered to the >>>>>>>> application. >>>>>>>> >>>>>>>> Can you check why in this case, we are getting the key event as well >>>>>>>> as the handleAction from the OS? >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Krishna >>>>>>>> >>>>>>>>> On 23-Feb-2019, at 9:14 PM, Manajit Halder <manajit.hal...@oracle.com >>>>>>>>> <mailto:manajit.hal...@oracle.com>> wrote: >>>>>>>>> >>>>>>>>> Hi All, >>>>>>>>> >>>>>>>>> Please review the fix for JDK13. >>>>>>>>> >>>>>>>>> Bug: >>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8216971 >>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8216971> >>>>>>>>> >>>>>>>>> Webrev: >>>>>>>>> http://cr.openjdk.java.net/~mhalder/8216971/webrev.00/ >>>>>>>>> <http://cr.openjdk.java.net/%7Emhalder/8216971/webrev.00/> >>>>>>>>> >>>>>>>>> Fix: >>>>>>>>> actionPerformed() was called twice due to wrong handling of key down >>>>>>>>> event in method handleAction, which is corrected with this fix. >>>>>>>>> This change was added during fix of issue JDK-8152492. Apart from the >>>>>>>>> changes required to fix the problem, code related to finding out >>>>>>>>> eventKey is removed as eventKey is no more used now. >>>>>>>>> >>>>>>>>> Note: >>>>>>>>> This issue is regression of bug 8152492, which was introduced in JDK >>>>>>>>> release 9b120. >>>>>>>>> >>>>>>>>> Regards, >>>>>>>>> Manajit >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
