Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

~ + Tab -> Focus lost (onKeyDown_ triggered twice for Tab key) #1341

Closed
2 tasks done
chabou opened this issue Jan 4, 2017 · 2 comments
Closed
2 tasks done

~ + Tab -> Focus lost (onKeyDown_ triggered twice for Tab key) #1341

chabou opened this issue Jan 4, 2017 · 2 comments
Labels
help wanted Contributions wanted towards the issue 🐛 Type: Bug Issue pertains to something wrong within Hyper

Comments

@chabou
Copy link
Collaborator

chabou commented Jan 4, 2017

  • I am on the latest Hyper.app version
  • I have searched the issues of this repo and believe that this is not a duplicate
  • OS version and name: MacOS 10.11.6
  • Hyper.app version: Master but present in 1.0.1
  • Link of a Gist with the contents of your .hyper.js: default
  • Relevent information from devtools (CMD+SHIFT+I on Mac OS, CTRL+SHIFT+I elsewhere): See below
  • The issue is reproducible in vanilla Hyper.app: yes

Issue

The problem is : If a "composition", beginning with ~ for example, is ended with Tab and not Space, Term loose focus (hyperCaret receive a blur event).

After blur event, if I press Tab twice, focus come back. (same as #1273)

More interesting: If I have another pane, focus goes to the other pane (without highlight menu) !! After that, If a do the same on the other pane, focus disappear, press 'tab' twice and he goes back to first pane.

I tracked down this bug and it drove me nuts...

Here is what I found:

hterm.js?8d82:204 onKeyDown_ KeyboardEvent {isTrusted: true, key: "Alt", code: "AltLeft", location: 1, ctrlKey: false…}
hterm.js?8d82:204 onKeyDown_ KeyboardEvent {isTrusted: true, key: "Dead", code: "KeyN", location: 0, ctrlKey: false…}
hterm.js?8d82:204 onKeyDown_ KeyboardEvent {isTrusted: true, key: "Tab", code: "Tab", location: 0, ctrlKey: false…}
index.js?bb83:6001 No definition for keyCode: 229
hterm.js?8d82:122 onTextInput_ TextEvent {isTrusted: true, data: "~", view: Window, detail: 0, which: 0…}
core.js?dfa1:97  action @ 23:57:52.611 SESSION_USER_DATA
hterm.js?8d82:204 onKeyDown_ KeyboardEvent {isTrusted: true, key: "Tab", code: "Tab", location: 0, ctrlKey: false…}
hterm.js?8d82:206 onKeyDown emitted twice
hterm.js?8d82:370 -> BLUR

onKeyDown_ function is called twice with the same event (same e.timeStamp value). If preventDefault() is called on it, blur do not happen.

I tried to debug step by step BUT if I add a breakpoint in onTextInput_function, this bug disappear 😱

Difference I noticed:
Without breakpoint HyperCarret input eventListener is called right after SESSION_USER_DATA redux action and before SESSION_ADD_DATA action.
But with a breakpoint, the call occurs after SESSION_ADD_DATA and SESSION_PTY_DATA actions

Maybe, webview think that I have Alt pressed with Tab. And in Chrome Alt+Tab give focus to the next Link. So Caret and Tabheader maybe ?

If someone can help me, because I'm on the brink of sending a PR with this ugly e.preventDefault(); 😂

@albinekb albinekb added help wanted Contributions wanted towards the issue 🐛 Type: Bug Issue pertains to something wrong within Hyper labels Jan 5, 2017
@albinekb
Copy link
Contributor

albinekb commented Jan 5, 2017

Oh so this is what I'm seeing too! Cool that you found what's broken!

If someone can help me, because I'm on the brink of sending a PR with this ugly e.preventDefault(); 😂

That's not so ugly? 🤔 Or will it break other things?

@chabou
Copy link
Collaborator Author

chabou commented Jan 5, 2017

"ugly" fix will not break anything but it more like a protection than a real solution. Maybe this repeated event breaks something else before we stop his propagation.
It might be better to prevent this race condition than to protect against it. But I spent enough hours on it to accept this protection as a solution 😥
If someone want to further investigate... In the meantime, I'll made a PR with this protection

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Contributions wanted towards the issue 🐛 Type: Bug Issue pertains to something wrong within Hyper
Projects
None yet
Development

No branches or pull requests

2 participants