atom / atom-keymap

Atom's selector-based keymap system
MIT License
105 stars 58 forks source link

Cannot input multibyte char via Japanese IME when pending state settled. #172

Closed t9md closed 8 years ago

t9md commented 8 years ago
$ atom --version
Atom    : 1.12.1
Electron: 1.3.6
Chrome  : 52.0.2743.82
Node    : 6.3.0

Reproduce

'atom-workspace atom-text-editor':
  'j j': 'core:move-down'

Actual situation

My real keymap is following, I set escape vim-mode-plus's insert-mode by j k keystroke. But this keymap prevent me to type by j i keystoke which was not issue in Atom v1.11.

'atom-text-editor.vim-mode-plus.insert-mode':
  'j k': 'vim-mode-plus:activate-normal-mode' # jk to escape
t9md commented 8 years ago

This is critical issue for Japanese user who are using google Japanese Input Method.

nathansobo commented 8 years ago

Sorry for the trouble. I will take a look tomorrow.

nathansobo commented 8 years ago

@t9md Okay, did some investigation. What's interesting is that for me, typing any single character on a Hirigana layout on 1.11.2 resolved to some strange accented a character. Was this working for you previously via the international keyboards package?

I think the problem is that we now recognize keystrokes correctly that were previously broken, and we're replaying keystrokes when matches fail that are actually part of IME composition. We may need to make the keymap aware of IME events and only replay when outside of IME compositions.

t9md commented 8 years ago

I haven't yet investigated in previous version 1.11. Buy what I can say is that I didn't notice this issue in 1.11. Thus, I want say worked in previous version. I'm not using keyboard layout helper package since I use US keyboard.

I wonder what drastically canged in current stable 1.12 is that when pending state is settled replaying against document.activeElement. Which was replayed against originally focused element(most case it is text-editor). That change is introduced by my PR. Sorry in that case. I'll check in short time.

nathansobo commented 8 years ago

So are you able to use multi-stroke bindings with Japanese IME input enabled? Because in 1.11 we don't seem to be recognizing the typed key correctly when IME is used.

t9md commented 8 years ago

So are you able to use multi-stroke bindings with Japanese IME input enabled?

I don't think so, in my case I configured keymap j k to map vim-mode-plus:activate-normal-mode which worked. When I enabled IME, I don't try to type j k. So what the issue is only when IME is enabled in v1.12 and I tried to type character by keystroke j i, but result become j + , I no longer can input in Atom editor v1.12. But I could type by keystroke j i in Atom v.1.11.

I'll check again in v1.11 from now(just download finished).

t9md commented 8 years ago

keybind

I checked

Both window enabled keybinding-resolver I enabled IME while I took above movie. My keystroke is j i My custom keymap is below.

'atom-text-editor.vim-mode-plus.insert-mode':
  'j k': 'vim-mode-plus:activate-normal-mode' # jk to escape

v1.11.2

The above window in movie. First j is mapped to a like char(I don't know how I can say it). I can type by keystroke j i without problem.

When I typed j k, I couldn't execute vim-mode-plus:activate-normal-mode(multi keystroke keymap).

v1.12.2

The below window in movie. First j is mapped to j ^j(partial) and second i is mapped i. Yes correctly mapped to Atom's keybinding system even if IME is enabled. So when I typed j k, I can execute vim-mode-plus:activate-normal-mode(multi keystroke keymap) even in IME is enabled. So I cannot type by keystroke j i. When I disabled my custom keymap, I can type by j i.

So

As you explained, because of Atom could not recognize keystroke when IME is enabled, it worked well. But from (by the improvement of) v1.12, Atom recognise keystroke even when IME is enabled that prevent me to type by keystroke j i. I want back the behavior of previous version.

t9md commented 8 years ago

So are you able to use multi-stroke bindings with Japanese IME input enabled? Because in 1.11 we don't seem to be recognizing the typed key correctly when IME is used.

The answer after checking actual behavior. No I couldn't use multi-stroke binding when IME is enabled in v1.11.2. Yes I can use multi-stroke binding when IME is enabled in v1.12.2.

nathansobo commented 8 years ago

@t9md Can you just unset! that multi-stroke binding in the meantime? I'm having a really hard time finding a way to detect the IME composition is in progress on the i keydown event because it happens before the compositionstart event.

One approach could be to disable keystroke replay entirely on layouts that depend on IME... but it's not clear how to detect such layouts in a general way and I also have concerns that we would disable replay in situations where it might be wanted.

t9md commented 8 years ago

Yes I can remove j k multi keystroke keymap. I'll do in the meantime.

From the user-experience of only just mine, previous keybinding system is better since I can use multi-keystroke when IME disabled and I can type Japanese character(which need multi keystroke) when IME was enabled.

Regardless of by the intentional design, previous version 1.11 worked properly from user experience perspective, so I believe there is the way to recover previous behavior.

I understand you and your team's effort to support different keyboard layout and it's improvement done in v.1.12. I understand that's importance, thank you for your work.

I'll try to find solution by myself. I want fix this, don't want to be long-standing-issue.

t9md commented 8 years ago

What I want to change is not second i of keystroke j i. The diff is first j keystroke of j i.

Can you say where this difference come from(want to know the hint to investigate).

nathansobo commented 8 years ago

Yeah, I'm definitely not suggesting we should leave this broken.

Basically, we use KeyboardEvent.key to translate keyboard events to keystrokes, then we use the U.S. equivalent character if the key is non-latin . Previously, we used older APIs to determine the typed character, and those were buggy with a Japanese layout enabled. Now that we're able to cleanly resolve the first keyboard event to j, it reveals this situation where we match a partial binding and then do a replay when a subsequent keystroke invalidates the partial match.

The best solution would be to suppress replay if the IME dialog is visible, although even that could present issues for long sequences.

t9md commented 8 years ago

Want to make it clear in case you misunderstand. My keyboard is US layout on Mac Book pro. Not japanese layout. I use google Japanese IME.

If you install google Japanese IME, the consition would be same as you(if you use US layout of mac book pro). What you explained is still applied to US keyboard layout user?

DarkNami commented 8 years ago

Same issue on Windows 7 with IME (Microsoft Chinese IME & Japanese IME). When I move cursor in IME, the cursor in Atom was moved, too. v1.11.2 is normal, but v1.12.2 broken. 2016-11-15 16-53-26

nathansobo commented 8 years ago

@t9md @DarkNami Could you test out the build associated with https://github.com/atom/atom/pull/13233, available via the build status links to our third party build providers at the bottom of the PR? We've taken steps to avoid the IME being interfered with by key bindings. When we get this right, we'll probably end up only putting it on beta because it presents a risk of breaking other aspects of keyboard handling, so you'll need to go back to 1.11 or forward to 1.13-beta for a release cycle once it lands.

DarkNami commented 8 years ago

I tested it in VM (clean installed Windows 7), looks like normal.

2016-11-16 12-06-04

t9md commented 8 years ago

@DarkNami I still can see issue in the build @nathansobo mentioned(I commented on PRs comment directly) You could reproduce original issue in v1.13-beta? You need custom keymap in your keymap.cson like below and then you no longer type じ char by keystroke j i. This is the required setup before checking improvement.

'atom-workspace atom-text-editor':
  'j j': 'core:move-down'
DarkNami commented 8 years ago

1.13 Beta default:

1 13_beta_default

1.13 Beta custom keymap:

1 13beta_custom_keymap

1.14 Dev default:

1 14dev_default

1.14 Dev custom keymap:

1 14dev_custom_keymap

The 1.14 dev fixed the IME input issue. But the custom keymap will cause atom intercepts first keystroke ( j ) to IME.

1.14 Dev Default (no custom keymap): IME can get 'JI', just like v1.11.

1.14 Dev custom keymap: IME get 'I' only.