Add a function that can check if a menu item would be fired by a keypress.

Add a function that can check if a menu item would be fired by a keypress.

BUG=Needed for a CL that I'm writing for 15090.
TEST=Unittest included.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30030

[Nico, 2009-10-22 08:40:28]

Hi keyboard crowd,

can you think of any weird case that I missed in the test cases?

If you're wondering: This is needed to implement
BrowserWindowCocoa::GetCommandId(), which needs to return a command id given a
keyboard NSEvent. So the plan is to walk the menu there, find a menu item that
matches the event, and return its tag it its action is commandDispatch (
http://codereview.chromium.org/273032 ).

[James Su, 2009-10-22 12:18:45]

[event charactersIgnoringModifiers] may not be able to support different
keyboard layouts. Please help confirm.

James Su

On 2009/10/22 08:40:28, Nico wrote:
> Hi keyboard crowd,
> 
> can you think of any weird case that I missed in the test cases?
> 
> If you're wondering: This is needed to implement
> BrowserWindowCocoa::GetCommandId(), which needs to return a command id given a
> keyboard NSEvent. So the plan is to walk the menu there, find a menu item that
> matches the event, and return its tag it its action is commandDispatch (
> http://codereview.chromium.org/273032 ).

[James Su, 2009-10-22 12:30:17]

Just found [NSMenu menuHasKeyEquivalent:forEvent:target:action:]. According to
its document, the default implementation is just for looking up the item
matching a key equivalent. But seems that it can't return the item's tag, which
is used by us as the command id.

I'm just wondering if we could change our menu code to map each menu item to a
different action so that we can just use this method.

Regards
James Su

On 2009/10/22 12:18:45, James Su wrote:
> [event charactersIgnoringModifiers] may not be able to support different
> keyboard layouts. Please help confirm.
> 
> James Su
> 
> On 2009/10/22 08:40:28, Nico wrote:
> > Hi keyboard crowd,
> > 
> > can you think of any weird case that I missed in the test cases?
> > 
> > If you're wondering: This is needed to implement
> > BrowserWindowCocoa::GetCommandId(), which needs to return a command id given
a
> > keyboard NSEvent. So the plan is to walk the menu there, find a menu item
that
> > matches the event, and return its tag it its action is commandDispatch (
> > http://codereview.chromium.org/273032 ).

[Nico, 2009-10-22 15:11:09]

Do you mean physical keyboard layouts? I tested with a German software
layout on my US keyboard, and things were sane as far as I could tell.
I also tested with the Kotoeri IM, which worked too (because this is
done for a keyDown method, which receives the raw keypresses). What
keyboard layout did you have in mind? (Jeremy: Are there international
keyboards for testing this somewhere?)

All I found about menuHasKeyEquivalent:forEvent:target:action: was
that it's part of a protocol, which means it's something we could
implement in our menu delegate, but it's nothing the menu implements.
Is that wrong?

On Thu, Oct 22, 2009 at 5:30 AM,  <suzhe@chromium.org> wrote:
> Just found [NSMenu menuHasKeyEquivalent:forEvent:target:action:]. According
> to
> its document, the default implementation is just for looking up the item
> matching a key equivalent. But seems that it can't return the item's tag,
> which
> is used by us as the command id.
>
> I'm just wondering if we could change our menu code to map each menu item to
> a
> different action so that we can just use this method.
>
> Regards
> James Su
>
> On 2009/10/22 12:18:45, James Su wrote:
>>
>> [event charactersIgnoringModifiers] may not be able to support different
>> keyboard layouts. Please help confirm.
>
>> James Su
>
>> On 2009/10/22 08:40:28, Nico wrote:
>> > Hi keyboard crowd,
>> >
>> > can you think of any weird case that I missed in the test cases?
>> >
>> > If you're wondering: This is needed to implement
>> > BrowserWindowCocoa::GetCommandId(), which needs to return a command id
>> > given
>
> a
>>
>> > keyboard NSEvent. So the plan is to walk the menu there, find a menu
>> > item
>
> that
>>
>> > matches the event, and return its tag it its action is commandDispatch (
>> > http://codereview.chromium.org/273032 ).
>
>
>
> http://codereview.chromium.org/319001
>

[James Su, 2009-10-22 15:26:35]

On 2009/10/22 15:11:09, Nico wrote:
> Do you mean physical keyboard layouts? I tested with a German software
> layout on my US keyboard, and things were sane as far as I could tell.
> I also tested with the Kotoeri IM, which worked too (because this is
> done for a keyDown method, which receives the raw keypresses). What
> keyboard layout did you have in mind? (Jeremy: Are there international
> keyboards for testing this somewhere?)
I tested with a Russian software layout, which seems doesn't work. You may have
a try.

> 
> All I found about menuHasKeyEquivalent:forEvent:target:action: was
> that it's part of a protocol, which means it's something we could
> implement in our menu delegate, but it's nothing the menu implements.
> Is that wrong?
From NSMenu's documentation for this method:
"If the delegate does not define this method, the menu is populated to find out
if any items have a matching key equivalent."
"Available in Mac OS X v10.3 and later."

> 
> On Thu, Oct 22, 2009 at 5:30 AM,  <mailto:suzhe@chromium.org> wrote:
> > Just found [NSMenu menuHasKeyEquivalent:forEvent:target:action:]. According
> > to
> > its document, the default implementation is just for looking up the item
> > matching a key equivalent. But seems that it can't return the item's tag,
> > which
> > is used by us as the command id.
> >
> > I'm just wondering if we could change our menu code to map each menu item to
> > a
> > different action so that we can just use this method.
> >
> > Regards
> > James Su
> >
> > On 2009/10/22 12:18:45, James Su wrote:
> >>
> >> [event charactersIgnoringModifiers] may not be able to support different
> >> keyboard layouts. Please help confirm.
> >
> >> James Su
> >
> >> On 2009/10/22 08:40:28, Nico wrote:
> >> > Hi keyboard crowd,
> >> >
> >> > can you think of any weird case that I missed in the test cases?
> >> >
> >> > If you're wondering: This is needed to implement
> >> > BrowserWindowCocoa::GetCommandId(), which needs to return a command id
> >> > given
> >
> > a
> >>
> >> > keyboard NSEvent. So the plan is to walk the menu there, find a menu
> >> > item
> >
> > that
> >>
> >> > matches the event, and return its tag it its action is commandDispatch (
> >> > http://codereview.chromium.org/273032 ).
> >
> >
> >
> > http://codereview.chromium.org/319001
> >

[jeremy, 2009-10-22 16:43:18]

http://codereview.chromium.org/319001/diff/3001/2002
File chrome/browser/cocoa/nsmenuitem_additions.h (right):

http://codereview.chromium.org/319001/diff/3001/2002#newcode10
Line 10: @interface NSMenuItem(ChromeAdditions)
Could you add a header comment here?

http://codereview.chromium.org/319001/diff/3001/2003
File chrome/browser/cocoa/nsmenuitem_additions.mm (right):

http://codereview.chromium.org/319001/diff/3001/2003#newcode14
Line 14: // this method can ignore |userKeyEquivalent| and check |keyEquivalent|
only.
Could you provide some context here, I don't entirely understand this comment.

http://codereview.chromium.org/319001/diff/3001/2003#newcode27
Line 27: if ([event keyCode] == 0x35 &&
Could you make 0x35 a constant?

http://codereview.chromium.org/319001/diff/3001/2003#newcode53
Line 53: }
Could you combine both these if statements so we don't have code duplication in
the body?

http://codereview.chromium.org/319001/diff/3001/2004
File chrome/browser/cocoa/nsmenuitem_additions_unittest.mm (right):

http://codereview.chromium.org/319001/diff/3001/2004#newcode5
Line 5: NSEvent* KeyEvent(const NSUInteger modifierFlags, NSString* chars,
NSString* charsNoMods, const NSUInteger keyCode) {
line wrap

http://codereview.chromium.org/319001/diff/3001/2004#newcode142
Line 142: item = MenuItem(@"\x08", 0x20000);
Could you make all of these constant to make this easier to read?

[Nico, 2009-10-22 20:57:52]

Jeremy suggested that it might be better to just hardcode a list of keyboard
shortcuts around somewhere, a duplicate of the data in the xib file. However,
this would have the disadvantage of not supporting custom keyboard shortcuts
that someone configures in sysprefs. It would seem fair if we would honor it if
a user wants to bind "next tab" to cmd-right and not send that key to the
renderer either. At least I think that would be my mental model if I were an
unsuspecting user.

Having said that, it looks like supporting all keyboard layouts correctly is, as
they say, a bag of hurt. Shortcut Recorder has some code to transform a keycode
into its characters, e.g. here
http://code.google.com/p/shortcutrecorder/source/browse/trunk/Source/SRCommon...
, but since menu items store text and not key codes, this approach doesn't
really help us. So maybe supporting user shortcuts is asking for too much.

Having said _that_, I came up with an ungodly hack (uploaded) that seems to make
things work for cyrillic keyboards. So maybe this CL could fly after all. What
do people think?

http://codereview.chromium.org/319001/diff/3001/2002
File chrome/browser/cocoa/nsmenuitem_additions.h (right):

http://codereview.chromium.org/319001/diff/3001/2002#newcode10
Line 10: @interface NSMenuItem(ChromeAdditions)
On 2009/10/22 16:43:20, jeremy wrote:
> Could you add a header comment here?

Done.

http://codereview.chromium.org/319001/diff/3001/2003
File chrome/browser/cocoa/nsmenuitem_additions.mm (right):

http://codereview.chromium.org/319001/diff/3001/2003#newcode14
Line 14: // this method can ignore |userKeyEquivalent| and check |keyEquivalent|
only.
On 2009/10/22 16:43:20, jeremy wrote:
> Could you provide some context here, I don't entirely understand this comment.

Done.

http://codereview.chromium.org/319001/diff/3001/2003#newcode27
Line 27: if ([event keyCode] == 0x35 &&
On 2009/10/22 16:43:20, jeremy wrote:
> Could you make 0x35 a constant?

Done, sort of.

http://codereview.chromium.org/319001/diff/3001/2003#newcode53
Line 53: }
On 2009/10/22 16:43:20, jeremy wrote:
> Could you combine both these if statements so we don't have code duplication
in
> the body?

I think duplicating 2 lines is better than having more complicated conditionals
in this case.

http://codereview.chromium.org/319001/diff/3001/2004
File chrome/browser/cocoa/nsmenuitem_additions_unittest.mm (right):

http://codereview.chromium.org/319001/diff/3001/2004#newcode5
Line 5: NSEvent* KeyEvent(const NSUInteger modifierFlags, NSString* chars,
NSString* charsNoMods, const NSUInteger keyCode) {
On 2009/10/22 16:43:20, jeremy wrote:
> line wrap

Done.

http://codereview.chromium.org/319001/diff/3001/2004#newcode142
Line 142: item = MenuItem(@"\x08", 0x20000);
On 2009/10/22 16:43:20, jeremy wrote:
> Could you make all of these constant to make this easier to read?

I'd rather not. As mentioned above, I have a small program that basically prints
keycodes, modifiers, etc every time I press a key, and that checks if a menu
item fires. This is just a copy of a subset of that program's output. With the
numbers  being literally embedded in here, it's easier to compare the output of
that program with this test, and there's a smaller chance of me making a mistake
when constantifying numbers.

[jeremy, 2009-10-22 21:03:09]

My fear is that there are other keyboard layouts out there that would
require similar evil hacks that could change between arbitrary point
releases of the system software.

Have you looked into how hard it would be to directly load the relevant
plist to support custom keyboard mappings?

On Thu, Oct 22, 2009 at 10:57 PM, <thakis@chromium.org> wrote:

> Jeremy suggested that it might be better to just hardcode a list of
> keyboard
> shortcuts around somewhere, a duplicate of the data in the xib file.
> However,
> this would have the disadvantage of not supporting custom keyboard
> shortcuts
> that someone configures in sysprefs. It would seem fair if we would honor
> it if
> a user wants to bind "next tab" to cmd-right and not send that key to the
> renderer either. At least I think that would be my mental model if I were
> an
> unsuspecting user.
>
> Having said that, it looks like supporting all keyboard layouts correctly
> is, as
> they say, a bag of hurt. Shortcut Recorder has some code to transform a
> keycode
> into its characters, e.g. here
>
>
http://code.google.com/p/shortcutrecorder/source/browse/trunk/Source/SRCommon...
> , but since menu items store text and not key codes, this approach doesn't
> really help us. So maybe supporting user shortcuts is asking for too much.
>
> Having said _that_, I came up with an ungodly hack (uploaded) that seems to
> make
> things work for cyrillic keyboards. So maybe this CL could fly after all.
> What
> do people think?
>
>
>
> http://codereview.chromium.org/319001/diff/3001/2002
> File chrome/browser/cocoa/nsmenuitem_additions.h (right):
>
> http://codereview.chromium.org/319001/diff/3001/2002#newcode10
> Line 10: @interface NSMenuItem(ChromeAdditions)
> On 2009/10/22 16:43:20, jeremy wrote:
>
>> Could you add a header comment here?
>>
>
> Done.
>
>
> http://codereview.chromium.org/319001/diff/3001/2003
> File chrome/browser/cocoa/nsmenuitem_additions.mm (right):
>
> http://codereview.chromium.org/319001/diff/3001/2003#newcode14
> Line 14: // this method can ignore |userKeyEquivalent| and check
> |keyEquivalent| only.
> On 2009/10/22 16:43:20, jeremy wrote:
>
>> Could you provide some context here, I don't entirely understand this
>>
> comment.
>
> Done.
>
>
> http://codereview.chromium.org/319001/diff/3001/2003#newcode27
> Line 27: if ([event keyCode] == 0x35 &&
> On 2009/10/22 16:43:20, jeremy wrote:
>
>> Could you make 0x35 a constant?
>>
>
> Done, sort of.
>
>
> http://codereview.chromium.org/319001/diff/3001/2003#newcode53
> Line 53: }
> On 2009/10/22 16:43:20, jeremy wrote:
>
>> Could you combine both these if statements so we don't have code
>>
> duplication in
>
>> the body?
>>
>
> I think duplicating 2 lines is better than having more complicated
> conditionals in this case.
>
>
> http://codereview.chromium.org/319001/diff/3001/2004
> File chrome/browser/cocoa/nsmenuitem_additions_unittest.mm (right):
>
> http://codereview.chromium.org/319001/diff/3001/2004#newcode5
> Line 5: NSEvent* KeyEvent(const NSUInteger modifierFlags, NSString*
> chars, NSString* charsNoMods, const NSUInteger keyCode) {
> On 2009/10/22 16:43:20, jeremy wrote:
>
>> line wrap
>>
>
> Done.
>
>
> http://codereview.chromium.org/319001/diff/3001/2004#newcode142
> Line 142: item = MenuItem(@"\x08", 0x20000);
> On 2009/10/22 16:43:20, jeremy wrote:
>
>> Could you make all of these constant to make this easier to read?
>>
>
> I'd rather not. As mentioned above, I have a small program that
> basically prints keycodes, modifiers, etc every time I press a key, and
> that checks if a menu item fires. This is just a copy of a subset of
> that program's output. With the numbers  being literally embedded in
> here, it's easier to compare the output of that program with this test,
> and there's a smaller chance of me making a mistake when constantifying
> numbers.
>
>
> http://codereview.chromium.org/319001
>

[Nico, 2009-10-22 21:11:23]

I venture to claim that this should work for all layouts. Cocoa stores
literal text like @"a" for menu items and from playing with IB, it
doesn't seem possible to put characters which are not in basic ascii
into the menu. Hence, one of the two texts of the event should always
contain ascii text, else cocoa couldn't trigger the menu item with
that key. That does at least sound plausible to me. (But I'm aware
that this are famous last words, and that OS X is often weirder than
expected).

I looked around a bit for plist files, but couldn't find any. Which
plists are you talking about? (Also, to me, that sounds less stable
than what this CL contains. But I'll definitely look at it when you
point me to the file locations)

On Thu, Oct 22, 2009 at 2:02 PM, Jeremy Moskovich <jeremy@chromium.org> wro=
te:
> My fear is that there are other keyboard layouts out there that would
> require similar evil hacks that could change between arbitrary point
> releases of the system software.
> Have you looked into how hard it would be to directly load the relevant
> plist to support custom keyboard mappings?
> On Thu, Oct 22, 2009 at 10:57 PM, <thakis@chromium.org> wrote:
>>
>> Jeremy suggested that it might be better to just hardcode a list of
>> keyboard
>> shortcuts around somewhere, a duplicate of the data in the xib file.
>> However,
>> this would have the disadvantage of not supporting custom keyboard
>> shortcuts
>> that someone configures in sysprefs. It would seem fair if we would hono=
r
>> it if
>> a user wants to bind "next tab" to cmd-right and not send that key to th=
e
>> renderer either. At least I think that would be my mental model if I wer=
e
>> an
>> unsuspecting user.
>>
>> Having said that, it looks like supporting all keyboard layouts correctl=
y
>> is, as
>> they say, a bag of hurt. Shortcut Recorder has some code to transform a
>> keycode
>> into its characters, e.g. here
>>
>> http://code.google.com/p/shortcutrecorder/source/browse/trunk/Source/SRC=
ommon.m#163
>> , but since menu items store text and not key codes, this approach doesn=
't
>> really help us. So maybe supporting user shortcuts is asking for too muc=
h.
>>
>> Having said _that_, I came up with an ungodly hack (uploaded) that seems
>> to make
>> things work for cyrillic keyboards. So maybe this CL could fly after all=
.
>> What
>> do people think?
>>
>>
>> http://codereview.chromium.org/319001/diff/3001/2002
>> File chrome/browser/cocoa/nsmenuitem_additions.h (right):
>>
>> http://codereview.chromium.org/319001/diff/3001/2002#newcode10
>> Line 10: @interface NSMenuItem(ChromeAdditions)
>> On 2009/10/22 16:43:20, jeremy wrote:
>>>
>>> Could you add a header comment here?
>>
>> Done.
>>
>> http://codereview.chromium.org/319001/diff/3001/2003
>> File chrome/browser/cocoa/nsmenuitem_additions.mm (right):
>>
>> http://codereview.chromium.org/319001/diff/3001/2003#newcode14
>> Line 14: // this method can ignore |userKeyEquivalent| and check
>> |keyEquivalent| only.
>> On 2009/10/22 16:43:20, jeremy wrote:
>>>
>>> Could you provide some context here, I don't entirely understand this
>>
>> comment.
>>
>> Done.
>>
>> http://codereview.chromium.org/319001/diff/3001/2003#newcode27
>> Line 27: if ([event keyCode] =3D=3D 0x35 &&
>> On 2009/10/22 16:43:20, jeremy wrote:
>>>
>>> Could you make 0x35 a constant?
>>
>> Done, sort of.
>>
>> http://codereview.chromium.org/319001/diff/3001/2003#newcode53
>> Line 53: }
>> On 2009/10/22 16:43:20, jeremy wrote:
>>>
>>> Could you combine both these if statements so we don't have code
>>
>> duplication in
>>>
>>> the body?
>>
>> I think duplicating 2 lines is better than having more complicated
>> conditionals in this case.
>>
>> http://codereview.chromium.org/319001/diff/3001/2004
>> File chrome/browser/cocoa/nsmenuitem_additions_unittest.mm (right):
>>
>> http://codereview.chromium.org/319001/diff/3001/2004#newcode5
>> Line 5: NSEvent* KeyEvent(const NSUInteger modifierFlags, NSString*
>> chars, NSString* charsNoMods, const NSUInteger keyCode) {
>> On 2009/10/22 16:43:20, jeremy wrote:
>>>
>>> line wrap
>>
>> Done.
>>
>> http://codereview.chromium.org/319001/diff/3001/2004#newcode142
>> Line 142: item =3D MenuItem(@"\x08", 0x20000);
>> On 2009/10/22 16:43:20, jeremy wrote:
>>>
>>> Could you make all of these constant to make this easier to read?
>>
>> I'd rather not. As mentioned above, I have a small program that
>> basically prints keycodes, modifiers, etc every time I press a key, and
>> that checks if a menu item fires. This is just a copy of a subset of
>> that program's output. With the numbers =A0being literally embedded in
>> here, it's easier to compare the output of that program with this test,
>> and there's a smaller chance of me making a mistake when constantifying
>> numbers.
>>
>> http://codereview.chromium.org/319001
>
>

[Nico, 2009-10-24 02:34:44]

Ping. I still think this is one of the smaller evils.

(Jeremy: If you replied, it didn't get through again)

[Nico, 2009-10-24 05:16:20]

I take that ping back, I want to look into something first. I will ping this
again when I mean it :-)

[Nico, 2009-10-25 16:59:27]

Another approach to do this is to temporarily put the menu item into a menu, set
its action to null, see what |menu performKeyEquivalent| returns, then re-set
the original action and return. Unfortunately, this is at least 80 times slower
than the approach in this patch (microbenchmark at http://pastebin.ca/1642773 ).

I thought I could at least use this method to write a test that iterates over
all keyboard layouts and keycodes, converts the current keycode to a character
on the current layout, and sends that character to both the fast and the slow
method, to make sure the fast method returns the same everywhere, but it looks
like the "real" menu item activation code looks at more than the documented
fields in an NSEvent:


NSEvent* CopyKeyEvent(NSEvent* e) {
  return [NSEvent keyEventWithType:[e type]
                          location:[e locationInWindow]
                     modifierFlags:[e modifierFlags]
                         timestamp:[e timestamp]
                      windowNumber:[e windowNumber]
                           context:[e context]
                        characters:[e characters]
       charactersIgnoringModifiers:[e charactersIgnoringModifiers]
                         isARepeat:[e isARepeat]
                           keyCode:[e keyCode]];
}

  // Create a dummy app with a custom view whose pKE method
  // contains this (make sure there's a menu item that has cmd-a
  // as shortcut):
  NSLog(@"blink: %d", [[NSApp mainMenu] performKeyEquivalent:KeyEvent([event
modifierFlags], [event characters], [event charactersIgnoringModifiers], [event
keyCode])]);
  NSLog(@"blink: %d", [[NSApp mainMenu] performKeyEquivalent:event]);

When hitting cmd-a with a russian layout with such an app, the first line logs a
"0" while the second logs "1", even though the event sent in the first line is
an exact copy of the event sent in the second line.

Well, I'll continue looking.

[Nico, 2009-10-25 22:39:40]

I've added another test. Due to the deficiencies in Cocoa (see above), it's not
as convincing as I planned, but it's at least somewhat convincing. I think this
is now in reasonable shape to be checked in.

If you think it's necessary, I can add explicit tests for the keys we will
actually care about (cmd-w, shift-cmd-w, cmd-t, and the like) for the
interesting layouts (french/belgian, italian, turkish).

I think this approach is the way to go. It's by now reasonably well-tested (hey,
it even seems to support dvorak!), it's fast, the keys we care about are normal
letters and not some weird edge-cases (even though those should work too), and
we're not going to use this for something critical.

(Thanks for the skepticism though, it made me find a small bug, made me add more
tests, _and_ made me aware of a potential problem in the
global_keyboard_shortcuts_mac code)

[James Su, 2009-10-26 03:10:12]

This CL looks good to me mostly. You may go ahead with this approach for now.
Though I still think we should find out a better solution for this issue. I'll
think about it when fixing http://crbug.com/24479.

James Su

http://codereview.chromium.org/319001/diff/9004/11008
File chrome/browser/cocoa/nsmenuitem_additions.mm (right):

http://codereview.chromium.org/319001/diff/9004/11008#newcode82
Line 82: eventString = [event characters];
Is it possible to make above two if statements easier to understand? For
example, is it ok to just compare [self keyEquivalent] to both [event
characters] and [event charactersIgnoringModifiers] and returns YES if any of
them matches?

[Nico, 2009-10-26 03:22:14]

Thanks, then I'll submit this for now :-)

http://codereview.chromium.org/319001/diff/9004/11008
File chrome/browser/cocoa/nsmenuitem_additions.mm (right):

http://codereview.chromium.org/319001/diff/9004/11008#newcode82
Line 82: eventString = [event characters];
On 2009/10/26 03:10:13, James Su wrote:
> Is it possible to make above two if statements easier to understand? For
> example, is it ok to just compare [self keyEquivalent] to both [event
> characters] and [event charactersIgnoringModifiers] and returns YES if any of
> them matches?

No, on the dvorak-qwertycmd layout, cmd-z will send "z" in "characters" and ";"
in "charactersIgnoringModifiers". For a menu item with accelerator "cmd-;", this
event should not fire.

[James Su, 2009-10-26 03:23:41]

I see. LGTM.

On 2009/10/26 03:22:14, Nico wrote:
> Thanks, then I'll submit this for now :-)
> 
> http://codereview.chromium.org/319001/diff/9004/11008
> File chrome/browser/cocoa/nsmenuitem_additions.mm (right):
> 
> http://codereview.chromium.org/319001/diff/9004/11008#newcode82
> Line 82: eventString = [event characters];
> On 2009/10/26 03:10:13, James Su wrote:
> > Is it possible to make above two if statements easier to understand? For
> > example, is it ok to just compare [self keyEquivalent] to both [event
> > characters] and [event charactersIgnoringModifiers] and returns YES if any
of
> > them matches?
> 
> No, on the dvorak-qwertycmd layout, cmd-z will send "z" in "characters" and
";"
> in "charactersIgnoringModifiers". For a menu item with accelerator "cmd-;",
this
> event should not fire.