<Swing Dev> <Swing dev>[13] Review request for JDK-8216971: [macosx swing] For JCheckBoxMenuItem actionPerformed() is called twice, when apple.laf.useScreenMenuBar=true and modifier is InputEvent.META_DOWN_MASK
Prasanta Sadhukhan
prasanta.sadhukhan at oracle.com
Tue Mar 19 05:27:07 UTC 2019
Thanks for making the test really standalone as discussed but you can
also remove
30 * @library /test/lib
and
46 import jdk.test.lib.Platform;
since you do not need them now. Rest looks ok.
Regards
Prasanta
On 18-Mar-19 6:10 PM, Manajit Halder wrote:
> Thanks Prasanta, I have corrected the indentation issue. Also removed
> use of @build jdk.test.lib.Platform and replaced Platform.isOSX() with
> System.getProperty.
> Please review the updated webrev.
> http://cr.openjdk.java.net/~mhalder/8216971/webrev.03/
> <http://cr.openjdk.java.net/%7Emhalder/8216971/webrev.03/>
>
> Regards,
> Manajit
>
>> On 18-Mar-2019, at 4:25 PM, Prasanta Sadhukhan
>> <prasanta.sadhukhan at oracle.com
>> <mailto:prasanta.sadhukhan at oracle.com>> wrote:
>>
>> Still
>>
>> 74 } finally {
>> in testcase not fixed. Please fix it before pushing...No need to send
>> webrev.
>>
>> Regards
>> Prasanta
>> On 18-Mar-19 4:12 PM, Manajit Halder wrote:
>>> 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/%7Emhalder/8216971/webrev.02/>
>>>
>>> Regards,
>>> Manajit
>>>
>>>> On 18-Mar-2019, at 11:21 AM, Prasanta Sadhukhan
>>>> <prasanta.sadhukhan at oracle.com
>>>> <mailto:prasanta.sadhukhan at 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.sadhukhan at oracle.com
>>>>>> <mailto:prasanta.sadhukhan at 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.halder at oracle.com <mailto:manajit.halder at 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.addepalli at oracle.com
>>>>>>>>> <mailto:krishna.addepalli at 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.halder at oracle.com
>>>>>>>>>> <mailto:manajit.halder at 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.addepalli at oracle.com
>>>>>>>>>>> <mailto:krishna.addepalli at 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.halder at oracle.com
>>>>>>>>>>>> <mailto:manajit.halder at oracle.com>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi All,
>>>>>>>>>>>>
>>>>>>>>>>>> Please review the fix for JDK13.
>>>>>>>>>>>>
>>>>>>>>>>>> Bug:
>>>>>>>>>>>> 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
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20190319/88a6254c/attachment-0001.html>
More information about the swing-dev
mailing list