Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Cocoa conformance with key equivalent handling #1278

Merged
merged 3 commits into from Jul 18, 2011

Conversation

Projects
None yet
6 participants
Contributor

aparajita commented May 23, 2011

I discovered the following behavior that is not in conformance with Cocoa:

  • Backspace, tab, escape, and space are considered key equivalents.
  • Function keys (F1 - FNN) are not considered key equivalents.
  • CPTextField executes performKeyEquivalent twice if a field is first responder.

Build this Cocoa project and debug it: http://d.pr/Xc3h

Take a look at the debug log while typing some keys. You will see that backspace, tab, escape and space do not trigger performKeyEquivalent, and that performKeyEquivalent is only executed once for the first responder.

Then run the performKeyEquivalentsTest app in this commit and look at the console log while trying keys in the text fields. If you turn the old code on, you will see how performKeyEquivalent is executed twice for the first responder.

For reference on key handling code, I consulted this:

http://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/EventOverview/EventArchitecture/EventArchitecture.html%23//apple_ref/doc/uid/10000060i-CH3-SW11

Aparajita Fishman Fixed set of keys that can be considered a key equivalent, removed re…
…dundant call to performKeyEquivalent, test app
65b6ff6
Contributor

MCF commented May 23, 2011

As an initial comment I'd just point out that we discussed Cocoa conformance for the space key in this pull request:

https://github.com/280north/cappuccino/pull/1014

Noted in that issue was a snippet of code discussing setKeyEquivalentModifierMask in the documentation for NSMenuItem that shows the space key being used as a key equivalent.

Can you point me at the location in the documentation from Apple that outlines what keys can and cannot be used for key equivalents? I had a quick look at the reference you noted, but nothing jumped out at me.

Contributor

klaaspieter commented May 23, 2011

I just tested this in a Cocoa application. Apple allows space as a key equivalent for controls and menu items. Technically space is a keyboard interface control, but because key equivalents are handled before keyboard interface controls setting it as a key equivalent of a button will override this behavior. This is bad because it overrides expected behavior if full keyboard access is turned on. We don't have full keyboard access so currently it's not a problem.

TL;DR our current space key behavior is correct.

Contributor

aparajita commented May 23, 2011

Yes, you are correct about the space key.

Can you point me at the location in the documentation from Apple that outlines what keys can and cannot be used for key equivalents? I had a quick look at the reference you noted, but nothing jumped out at me.

I don't know of any, I was going by empirical evidence. But I was only looking at text fields. Looking at buttons in Cocoa, it is clear that space, backspace and escape are valid key equivalents.

However, there must be extra code that filters out those three such that they are not considered key equivalents for text fields, thus preventing performKeyEquivalent from being called. This was the original behavior I was addressing.

I just updated my test Cocoa app to use both buttons with key equivalents and a text field: http://d.pr/Cfd4

Run it and take a look at the log. The results are very interesting. It seems that the top level event dispatcher does special handling when the first responder is a text field, because performKeyEquivalent is not sent at all for backspace, space, and escape.

Owner

aljungberg commented May 23, 2011

Not only space but also delete, forward delete and escape are valid key equivalents. I think I was the one who implemented support for that back last year.

Like aparajita says, delete (backspace) and space are not handled as key equivalents when a text field is the first responder. Surprisingly, forward delete is. Strangely in my test case 'escape' is not intercepted by an active text field, but in aparajita's test app it is. I didn't have time to dig into the difference, but in aparajita's text field subclass things are behaving very oddly indeed. If I hit the down arrow it types a unicode character, and backspace doesn't delete anything even that the code clearly calls [super keyDown:theEvent];.

Contributor

aparajita commented May 23, 2011

aparajita's text field subclass things are behaving very oddly indeed. If I hit the down arrow it types a unicode character, and backspace doesn't delete anything even that the code clearly calls

The first responder in Cocoa is an NSTextView, not the NSTextField. So it's NSTextView -keyDown: that is being called, not NSTextField -keyDown:, and I am just sending insertText: because it's a category and I don't care about what is inserted, I just want to see when keyDown: is called and when performKeyEquivalent: is called.

Contributor

MCF commented May 23, 2011

I don't own a Mac so I cannot participate in the empirical testing.

But in general I like the current list of allowed key equivalents. Klass' concern about full keyboard access is reasonable, but we aren't implementing that yet anyway. Is there any reason why we can't leave things as they are for the time being - despite Cocoa doing slightly different things?

Contributor

aparajita commented May 23, 2011

Like aparajita says, delete (backspace) and space are not handled as key equivalents when a text field is the first responder. Surprisingly, forward delete is.

Which means that code has to be added either to CPWindow -keyDown: or CPEvent -_couldBeKeyEquivalent:.

Strangely in my test case 'escape' is not intercepted by an active text field

Which test case is that? I'd like to see it.

Contributor

aparajita commented May 23, 2011

in general I like the current list of allowed key equivalents

As I mentioned already, the current list is fine with two caveats:

  • We have to do special casing when a text field is first responder
  • If you look closely I added support for all function keys (F1-F35) that are missing from the current code and clearly should be included as possible key equivalents.

There are bunch of Windows function key codes listed in CPEvent that I'm not sure about, such as CPPrintScreenFunctionKey, CPScrollLockFunctionKey, etc. What should we do about those? In my patch I included those as valid key equivalents. Since those are Windows only, we can't compare with Cocoa. So it's your call.

Contributor

MCF commented May 23, 2011

My mistake, I thought you were removing some of them. I don't have a problem with adding more.

Aparajita Fishman Restored space, backspace and escape as key equivalents, but only if …
…the first responder is not a CPTextField. Updated test app to show clearly the effect of this change.
acbaa1e
Contributor

aparajita commented May 23, 2011

I implemented the special casing in CPEvent _couldBeKeyEquivalent, and updated the test app to illustrate how fubar the current behavior is if you have buttons in the same window with space, backspace or escape as a key equivalent. I can understand why space and backspace have to be special-cased -- without that, you can't do minimal editing of a text field. In Cocoa the escape key was triggers the built in spell checker.

Try using the old code and new code and typing space, escape and backspace with a text field active. Also activate the button-only window with the new code active and see that the key equivalents work as expected with no text first responder.

Owner

aljungberg commented May 28, 2011

Here is my modified version of Klaas Pieter's test app including more key equivalents and demonstrating that escape is not intercepted by a text field. To test, focus the text field and press escape. Then hit ctrl-F7 to enable keyboard navigation, if you don't already have it on, and tab to resign the text field as first responder and press escape again. With or without the text field as the first responder, the escape button depresses.

aljungberg/cappuccino-comparisons@a5ff155

Contributor

aparajita commented May 31, 2011

I know why escape was being eaten for me -- I have a custom KeyBindingsDict which maps escape to completions, which triggers the Cocoa spell checker. I just pushed a commit that enables escape as a equivalent.

Contributor

Me1000 commented Jul 17, 2011

@klaaspieter or @aljungberg Are all the issues resolved now?

@aljungberg aljungberg added a commit that referenced this pull request Jul 18, 2011

@aljungberg aljungberg Merge pull request #1278 from aparajita/cappuccino
---

 I discovered the following behavior that is not in conformance with Cocoa:

- Backspace, tab, escape, and space are considered key equivalents.
- Function keys (F1 - FNN) are not considered key equivalents.
- CPTextField executes `performKeyEquivalent` twice if a field is first responder.

Build this Cocoa project and debug it: http://d.pr/Xc3h

Take a look at the debug log while typing some keys. You will see that backspace, tab, escape and space do not trigger `performKeyEquivalent`, and that `performKeyEquivalent` is only executed once for the first responder.

Then run the performKeyEquivalentsTest app in this commit and look at the console log while trying keys in the text fields. If you turn the old code on, you will see how `performKeyEquivalent` is executed twice for the first responder.

For reference on key handling code, I consulted this:

http://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/EventOverview/EventArchitecture/EventArchitecture.html%23//apple_ref/doc/uid/10000060i-CH3-SW11
70f8e3a

@aljungberg aljungberg merged commit 99180ba into cappuccino:master Jul 18, 2011

Owner

aljungberg commented Jul 18, 2011

It seems odd to hardcode a class with special handling in the event system, but the behaviour matches Cocoa as far as I can confirm in my testing. So I've merged the change. Thanks.

Owner

aljungberg commented Nov 10, 2011

After further review it turns out that tab is a valid key equivalent as well (although you can't set it in IB). In fact most keys seem eligible despite what IB initially lead us to believe. For example, both of these work as long as a text field is not the first responder:

[leftButton setKeyEquivalent:[NSString stringWithCString:"\t"]];
[leftButton setKeyEquivalent:[NSString stringWithCString:"a"]];

Remarkably, the tab equivalent worked even with keyboard navigation turned on, superseding keyboard navigation.

@cappbot cappbot added this to the Someday milestone Sep 29, 2015

@cappbot cappbot added the #new label Sep 29, 2015

cappbot commented Sep 29, 2015

Milestone: Someday. Label: #new. What's next? A reviewer should examine this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment