On Oct 16, 2013, at 9:37 PM, Anthony Petrov <[email protected]> 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.
