adobe / brackets

An open source code editor for the web, written in JavaScript, HTML and CSS.
http://brackets.io
MIT License
33.26k stars 7.63k forks source link

Ctrl + S shortcut not working in OS X non-native menu using Korean Layout #10329

Open patricio-marrone opened 9 years ago

patricio-marrone commented 9 years ago

Steps

  1. In OS X set the keyboard layout to Korean
  2. Launch brackets (it will be opened with native menus)
  3. Open a file, edit it
  4. Press Ctrl + S
  5. Brackets saves the file normally
  6. Launch brackets without native menus (I tested this using standard brackets by setting "global.brackets.nativeMenus = false" in src/util/Globals.js line 90. This also happens when Brackets is embedded inside Intel XDK)
  7. Open a file, edit it
  8. Press Ctrl + S

    Result

Nothing happens. The shortcut does not work.

Expected result

File should have been saved, just as when using native menus

Version

This was tested on brackets, taken from git SHA 5542986f5ffcb81e0061e5f7c727fae8df9b00ac

Posible cause

KeyBindingManager is using the keyIdentifier property of the keyboard event whenever possible. In Windows, when the S key is pressed, both the keyIdentifier property and the keyCode get changed to the corresponding S character codes; while on Mac OS X, the keyIdentifier remains pointing to the Korean character and only the keyCode and which properties of the keyboard event get changed to the 'S' code. This leads to inconsistent behavior when using some shortcuts in a group of non-latin keyboards. In this case, Ctrl+S on a Korean layout was shown as an example.

In the captured image below, it can be seen that the 'key' value was interpreted the same way as in the native menu shortcut (S), but it will be replaced in the following steps by the corresponding Korean character.

ctrl-s event capture on os x

RaymondLim commented 9 years ago

I think it should be a high priority issue since I can reproduce it in some IMEs (2-Set Korean, Zhuyin and Zhuyin - Eten). But I believe the issue is there with older releases (as fas as sprint-15). So set it to medium priority. The fix for it is pretty easy. We can prevent from assigning keyIdentifier property by checking key is not one of the A to Z characters. ie. replacing if (ident) with if (ident && /[^A-Z]/.test(key))