<AWT Dev> [8] Review request for JDK-8020209: [macosx] Mac OS X key event confusion for "COMMAND PLUS" (plan B)

[Sergey Bylokhov]

Hi, Leonid.
Thie fix looks good. Let's hope that in this area we will have no more bugs.

On 21.10.2013 15:26, Leonid Romanov wrote:
> Here is an updated version of the fix with the expanded comment:
>
> http://cr.openjdk.java.net/~leonidr/8020209/webrev.03/ 
> <http://cr.openjdk.java.net/%7Eleonidr/8020209/webrev.03/>
>
> On 17.10.2013, at 21:48, Anthony Petrov <anthony.petrov at oracle.com 
> <mailto:anthony.petrov at oracle.com>> wrote:
>
>> Thanks for the clarifications. They sound good to me.
>>
>> Regarding NSApp instance, your understanding is correct. Although I'm 
>> not sure if this is a good idea to tell embedders to fix their code 
>> in this case. But I'm sort of OK with the current approach for now.
>>
>> So, I'd like to see the final version of the fix with an updated 
>> comment, and then I approve it.
>>
>> --
>> best regards,
>> Anthony
>>
>> On 10/16/2013 11:19 PM, Leonid Romanov wrote:
>>>
>>> On Oct 16, 2013, at 9:37 PM, Anthony Petrov 
>>> <anthony.petrov at oracle.com <mailto:anthony.petrov at oracle.com>> wrote:
>>>
>>>> Hi Leonid,
>>>>
>>>> The problem with overriding NSApplication -sendEvent: is that you 
>>>> can't be sure AWT is running with the NSApplicationAWT instance. If 
>>>> using SWT, FX, or otherwise embedding AWT into another application, 
>>>> the NSApp will point to an instance of another application class, 
>>>> and your sendEvent: will never be called. I'd suggest to avoid 
>>>> using this method altogether if possible.
>>>
>>> Do I understand you correctly that in case of AWT embedding NSApp 
>>> could be some NSApplication subclass other than NSApplicationAWT? If 
>>> so, then this hypothetical  subclass might have the very same code 
>>> I've added to NSApplicationAWT since it's the most common approach 
>>> for solving the problem of missing NSKeyUp events. In this case, our 
>>> attempt to solve it in some other way will only make things worse: 
>>> we might end up with duplicate NSKeyUp events.
>>> So, my suggestions is to leave that code as it is and if embedders 
>>> complain about aforementioned problem, tell them to fix it on theirs 
>>> app level, as we've done it in our NSApplicationAWT.
>>>
>>>>
>>>> I see that webrev.01 also includes the changes in 
>>>> NSApplicationAWT.m. Is this really necessary for that version of 
>>>> the fix?
>>>>
>>>> I recall in your original review request you stated that the code 
>>>> currently consists of multiple workarounds, and adding another one 
>>>> could just bring more regressions or unexpected behaviors. So I'd 
>>>> actually prefer the version .01.
>>>
>>> So do I. There are circumstances beyond my control that prevent me 
>>> from landing the fix into JDK 8 GA. However, I still see .01 as the 
>>> definitive version of the fix, and planning to include it into JDK 8 
>>> Update 1/JDK 9 (I'll file a separate bug for it). Version .02 is a 
>>> not a long term solution for me.
>>>
>>>
>>>>
>>>> Anyway, here's a couple of comments regarding the new fix:
>>>>
>>>> src/macosx/native/sun/awt/AWTView.m
>>>>> 313     NSUInteger modFlags = [event modifierFlags] &
>>>>> 314         (NSCommandKeyMask | NSAlternateKeyMask | 
>>>>> NSShiftKeyMask | NSControlKeyMask);
>>>>> 315     if (modFlags == NSCommandKeyMask) {
>>>>
>>>> Do I understand correctly that OS X is fine with e.g. 
>>>> Shift+Cmd+'+', and only Cmd+'+' is causing a problem?
>>>
>>> Yep.
>>>
>>>>
>>>>> 311     // Workaround for 8020209: special case for "Cmd =" and 
>>>>> "Cmd ."
>>>>> 312     // because Cocoa calls performKeyEquivalent twice for 
>>>>> these keystrokes
>>>>
>>>> Interesting that you say that Cocoa sends multiple events and I'd 
>>>> guess one wants to filter some events out. However, at line 320 you 
>>>> call performKeyEquivalent yourself. Is this like the third call or 
>>>> something? Or the comment seems to be misleading otherwise.
>>>
>>> Ok, looks like I need to extend the comment. Cocoa calls 
>>> -performKeyEquivalent  twice only if we return NO from the first 
>>> -performKeyEquivalent call. Normally, what happens when 
>>> performKeyEquivalent returns NO is that Cocoa calls 
>>> -performKeyEquivalent for the next responder in the responders 
>>> chain, until it finds the one which would return YES. "Cmd =" and 
>>> "Cmd ." key strokes, however, are really unusual: if we and all 
>>> other responders in the chain return NO, then Cocoa will construct 
>>> another NSEvent and call -performKeyEquivalent for that event. 
>>> Returning YES from the -performKeyEquivalent prevents that. However, 
>>> this means that Cocoa won't call -performKeyEquivalent for the menu 
>>> bar, so we have to do it manually.
>>>
>>>>
>>>> --
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 10/16/2013 08:53 PM, Leonid Romanov wrote:
>>>>> Hello,
>>>>> This is plan B version of the fix for JDK-8020209: [macosx] Mac OS 
>>>>> X key event confusion for "COMMAND PLUS". The previous, proper 
>>>>> version of the fix has been reviewed here:
>>>>> http://mail.openjdk.java.net/pipermail/awt-dev/2013-September/005441.html
>>>>> Unfortunately, I can't proceed with that version because there are 
>>>>> some difficulties with submitting private JDK 8 build to Apple for 
>>>>> approval.  Since we are short on time and I want to fix this bug 
>>>>> in JDK 8, I've had to use a workaround.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8020209
>>>>> webrev: http://cr.openjdk.java.net/~leonidr/8020209/webrev.02/
>>>>>
>>>>> Thanks,
>>>>> Leonid.
>>>>>
>>>>>
>>>>>
>>>
>


-- 
Best regards, Sergey.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20131021/ac9f02c6/attachment.html