[osx] Adapt mirrored-shortcut handling to 10.9 differences.

Hebrew keyboard shortcuts for 10.9 mirror brackets and parens
differently than 10.8.  Convert to a more general mapping table which
doesn't make assumptions about the codes for the mirrored elements.

BUG=319182

Review URL: https://codereview.chromium.org/78783006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@238197 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/global_keyboard_shortcuts_mac.mm

@@ -173,52 +173,63 @@ unichar KeyCharacterForEvent(NSEvent* event) {
   NSString* eventString = [event charactersIgnoringModifiers];
   NSString* characters = [event characters];
 
-  // Character pairs that undergo BiDi mirrored.
-  // There are actually many more such pairs, but these are the ones that
-  // are likely to show up in keyboard shortcuts.
-  const struct {
-    unichar a;
-    unichar b;
-  } kMirroredBiDiChars[] = {
-    {'{', '}'},
-    {'[', ']'},
-    {'(', ')'},
-  };
-
   if ([eventString length] != 1)
     return 0;
 
   if ([characters length] != 1)
     return [eventString characterAtIndex:0];
 
+  // Some characters are BiDi mirrored.  The mirroring is different
+  // for different OS versions.  Instead of having a mirror table, map
+  // raw/processed pairs to desired outputs.
+  const struct {
+    unichar rawChar;
+    unichar unmodChar;
+    unichar targetChar;
+  } kCharMapping[] = {
+    // OSX 10.8 mirrors certain chars.
+    {'{', '}', '{'},
+    {'}', '{', '}'},
+    {'(', ')', '('},
+    {')', '(', ')'},
+
+    // OSX 10.9 has the unshifted raw char.
+    {'[', '}', '{'},
+    {']', '{', '}'},
+    {'9', ')', '('},
+    {'0', '(', ')'},
+
+    // These are the same either way.
+    {'[', ']', '['},
+    {']', '[', ']'},
+  };
+
   unichar noModifiersChar = [eventString characterAtIndex:0];
   unichar rawChar = [characters characterAtIndex:0];
-  // When both |characters| and |charactersIgnoringModifiers| are ascii,
-  // return the first character of |characters|, if...
+
+  // Only apply transformation table for ascii.
   if (isascii(noModifiersChar) && isascii(rawChar)) {
-    // |characters| is an alphabet (mainly for dvorak-qwerty layout), or
+    // Alphabetic characters aren't mirrored, go with the raw character.
+    // [A previous partial comment said something about Dvorak?]
     if (isalpha(rawChar))
       return rawChar;
 
     // http://crbug.com/42517
+    // http://crbug.com/315379
     // In RTL keyboard layouts, Cocoa mirrors characters in the string
     // returned by [event charactersIgnoringModifiers].  In this case, return
     // the raw (unmirrored) char.
-    // FIXME: If there is a need to add any more characters to the
-    // kMirroredBiDiChars table, then it's probably better to use ICU's
-    // u_charMirror() function to perform this test.
-    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kMirroredBiDiChars); ++i) {
-      const unichar& a = kMirroredBiDiChars[i].a;
-      const unichar& b = kMirroredBiDiChars[i].b;
-      if ((rawChar == a && noModifiersChar == b) ||
-          (rawChar == b && noModifiersChar == a))
-          return rawChar;
+    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kCharMapping); ++i) {
+      if (rawChar == kCharMapping[i].rawChar &&
+          noModifiersChar == kCharMapping[i].unmodChar) {
+        return kCharMapping[i].targetChar;
+      }
     }
 
     // opt/alt modifier is set (e.g. on german layout we want '{' for opt-8).
     if ([event modifierFlags] & NSAlternateKeyMask)
-      return [characters characterAtIndex:0];
+      return rawChar;
   }
 
-  return [eventString characterAtIndex:0];
+  return noModifiersChar;
 }

chrome/browser/global_keyboard_shortcuts_mac_unittest.mm

@@ -182,9 +182,12 @@ TEST(GlobalKeyboardShortcuts, KeyCharacterForEvent) {
   // cmd-'z' / cmd-shift-';' on dvorak-qwerty
   EXPECT_EQ('z', KeyCharacterForEvent(
       KeyEvent(true,  true,  false, false, @"z", @":")));
-  // cmd-shift-'[' in an RTL context.
+  // cmd-shift-'[' in an RTL context pre 10.9.
   EXPECT_EQ('{', KeyCharacterForEvent(
       KeyEvent(true,  true,  false, false, @"{", @"}")));
+  // cmd-shift-'[' in an RTL context on 10.9.
+  EXPECT_EQ('{', KeyCharacterForEvent(
+      KeyEvent(true,  true,  false, false, @"[", @"}")));
   // Test if getting dead-key events return 0 and do not hang.
   EXPECT_EQ(0,   KeyCharacterForEvent(
       KeyEvent(false, false, false, false, @"",  @"")));