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

Problem with pipe- and at-character in windows with chrome #33

Closed
chani opened this issue Oct 12, 2015 · 27 comments
Closed

Problem with pipe- and at-character in windows with chrome #33

chani opened this issue Oct 12, 2015 · 27 comments
Labels
type/bug Something is misbehaving

Comments

@chani
Copy link
Contributor

chani commented Oct 12, 2015

Hello,

in Windows (does not matter if virtualized or native) we are having problems with the | and @. While the latter is simply not printed it gives the following output:

altKey: true
bubbles: true
cancelBubble: false
cancelable: true
charCode: 0
ctrlKey: true
currentTarget: null
defaultPrevented: false
detail: 0
eventPhase: 0
keyCode: 81
keyIdentifier: "U+0051"
keyLocation: 0
location: 0
metaKey: false
path: Array[12]
repeat: false
returnValue: true
shiftKey: false
srcElement: textarea.xterm-helper-textarea
target: textarea.xterm-helper-textarea
timeStamp: 1444663462186
type: "keydown"
view: Window
which: 81
proto: KeyboardEvent

For the | key I get no debug output at all for key/ev. In Firefox and in Chrome running Linux it does work fine, though. Any ideas on this?

Cheers

@chani
Copy link
Contributor Author

chani commented Oct 12, 2015

Windows 7 Prof, Chrome Version 45.0.2454.101

@parisk
Copy link
Contributor

parisk commented Oct 14, 2015

Hi @chani, thanks for reporting! I couldn't reproduce your issue though. I typed both characters in the demo page (http://sourcelair.github.io/xterm.js/demo/) and at SourceLair (https://www.sourcelair.com) and both of them were printed normally, using Chrome 46 on Windows.

Could you give me some more context or some more particular steps on reproducing this?

Screenshots

screenshot 2015-10-14 20 08 52
screenshot 2015-10-14 20 09 32

@chani
Copy link
Contributor Author

chani commented Oct 15, 2015

Hello,

just verified, it does not work in your demo page neither with that combination (windows, chrome). We've got a german keyboard layout, the pipe char is made through ALT-GR+< and the at-char is made through @Alt-gr+q

(ALT-GR is the alt key on the right of the keyboard, not sure if it is named equally on keyboards outside of germany). Does that information help?

@TDaglis
Copy link
Contributor

TDaglis commented Nov 16, 2015

It seems that the issue here is the German keyboard layout.
The issue is that there is no way to know the keyboard layout beforehand. And in the keydown events you described, the Alt-gr is "translated" to Ctrl + Alt.
With the current version (0.31), Ctrl + Alt leads here where supposedly xterm will handle key = String.fromCharCode(ev.keyCode - 64); which results to just Ctrl key in the case of AltGr + q.
With my keyboard layout, @ (shift + 2) seems to be the result of just the keystroke and not the result of input handling. In the latest commits (though not in the latest version yet) the flow that led to Alt-gr being handled by the Ctrl clause has changed so it might work for you.
Try bower install xterm.js#c1701561474cd2aa7389b66f7acb2a1e00c84e24 and see if this fixes anything.
If not, we'll have to find a way to deal with other keyboard layouts properly

@chani
Copy link
Contributor Author

chani commented Nov 16, 2015

Hello,

thanks for the response, I am going to try that. Just as a quick idea, some way to override the mappings with custom-maps (i.e. people can maintain a de_DE.map which can be injected to xterm.js and would override the mappings in a way that it does fit for the used layout) would be nice. That way one could as well implement a "switcher" between different layouts.

@chani
Copy link
Contributor Author

chani commented Nov 17, 2015

Hello,

just verified. Still not working in chrome in windows.

@chani
Copy link
Contributor Author

chani commented Jan 6, 2016

Hello,

this problem still persists; However, could someone guide me / give me an example, what I could do to map (additionally) alt+7 to print a pipe character in the terminal window? Something like:

evt = (e==null ? event:e);
if ((evt.altKey && evt.keyCode == 55)) {
.. make a | into the terminal window.. :-)
}

that would be a nice workaround for our chrome users..

@parisk
Copy link
Contributor

parisk commented Jan 7, 2016

Hi @chani, thanks for getting back on this.

All the key handling functionality happens in Terminal.prototype.write. So this is where you could add your functionality.

Feel free to implement this in your fork and submit a PR here. We would love to review it and merge it into the upstream if it fixes the functionality 😄.

@exsilium
Copy link
Contributor

exsilium commented Jun 7, 2016

Seeing as now the torch has been officially given over to xterm.js by @chjj - the previously referenced fix has worked for me without issues. If this would be the ok approach to address this (via kbLayout option), I can prepare a pull req with any additional country layouts that are needed.

However, there is already another pull in review to address the different platform modifier keys so I'd like to see if that is pulled in or not. #60

@parisk
Copy link
Contributor

parisk commented Jun 7, 2016

Hi @exsilium. #60 seems like a well-prepared PR, but breaks some functionality though for mainstream usage.

I believe the best thing is to bring @runarberg into this discussion and see how we could modify #60 in order to achieve the desired result, without breaking functionality.

@exsilium
Copy link
Contributor

exsilium commented Jun 7, 2016

Hi! Thanks @parisk for the update. Yeah, the main flow should work reliably without question and that's prerequisite for any kind or 3rd level char fixes. Therefor I'd decouple the problems:

  1. Inconsistency of reported metaKeys depending on platform (this is the main scenario where everyone is impacted)
  2. 3rd level characters that are due to country specific keyboard layouts. This I'd say is a minority usecase that is very specific depending on which keyboard is being used. However, I'd avoid linking kb to any kind of language setting. These are completely two different things :) and very often unrelated.

Cheers!

@runarberg
Copy link
Contributor

The biggest problem with #60 is that on Mac OS, the alt key ⌥ Opt is also the third level shift. I saw there was some logic that mapped Mac’s meta key ⌘ Cmd to Alt on other platforms, but it was broken a few lines later. #60 fixes that (and causes inherent inconsistency in the mean time).

This leaves us with 3 options:

  1. Leave Fix cross platform input problems #60 as is. Hijack Mac OS user’s command key ⌘ Cmd, that may cause operations such as keyboard copy-pasting to fail.
  2. Remove said logic, and leave ⌥ Opt as Alt. That will forbid Mac OS users from entering third level shift.
  3. Remove said logic, and leave ⌥ Opt as a third level shift. That will prevent handy shortcuts such as Alt + D (delete word forward), but leaves the equivalent Esc D working as intended.

If we are going along with #60, I recommend patching it to comply with option nr. 3. As that is the only one that doesn’t remove any functionality for Mac OS users, Alt-clicking will still work through the Esc key, and ⌘ Cmd will work as the users expect. This is also the default settings of popular native Mac OS X terminal emulators such as Terminal and iTerm2.

@exsilium
Copy link
Contributor

exsilium commented Jun 7, 2016

Hi @runarberg.

Well said, I agree with your preference that number 3 would be the best choice because of the already wide use of ESC as meta in iTerm and Terminal. Some emacs diehards might disagree though ;) But I think it's a good compromise.

@parisk
Copy link
Contributor

parisk commented Jun 10, 2016

Could you please try checking out the latest commit on master and let us know if merging #60 fixed this issue?

/cc @chani @exsilium @runarberg

@exsilium
Copy link
Contributor

Seems to work nicely! Thanks! (Tested on OSX and Safari)

@chani
Copy link
Contributor Author

chani commented Jun 12, 2016

Tested with the demo-page and the new xterm.js from master; @ and | are still not working in the combination windows+chrome+german-keyboard. Wondering if I could maybe provide a VM for testing. However I had my wife testing that (since I do not have any windows machine and my colleagues are having weekend) will get back to you as soon as I have further results. In Linux+firefox/chrome it is still working fine. Not working versions:

chrome version 51.0.2704.84 m
windows 8

@parisk
Copy link
Contributor

parisk commented Jun 13, 2016

I think we might need a Pull Request for that, since not all of us have a keyboard with German layout.

@chani do you believe that by taking a look at #60 you could prepare a similar PR for the German keyboard layout?

@parisk parisk added the type/bug Something is misbehaving label Jun 13, 2016
@runarberg
Copy link
Contributor

@chani @parisk There shouldn't be any remaining issues with german keyboards in particular. Once you allow ISO level 3 shifts, it should be available for all ISO keyboards. Now there might still be issues on weather we're still preventing third-level inputs on some systems and some browsers. I never tested on Chrome on Windows for example.

http://stackoverflow.com/questions/10657346/detect-alt-gr-alt-graph-modifier-on-key-press

@parisk
Copy link
Contributor

parisk commented Jun 14, 2016

It would be great if someone could take this over and also provide some tests about this functionality. Unfortunately I cannot do this on my own, since I do not have the required hardware to work efficiently.

@chani
Copy link
Contributor Author

chani commented Jun 16, 2016

@parisk trying.

@chani
Copy link
Contributor Author

chani commented Jun 16, 2016

@parisk take a look:

the output for chrome on windows, pressing the @-char

e.keyCode: 64
e.altKey: true
e.ctrlKey: true
e.keyCode || e.which: 64
e.shiftKey: false
e.metaKey: false
e.charCode: 64

the same for my linux

Linux x86_64
e.keyCode: 64
e.altKey: false
e.ctrlKey: false
e.keyCode || e.which: 64
e.shiftKey: false
e.metaKey: false
e.charCode: 64

did you notice, that windows seems to send altKey and ctrlKey true where as in linux this does not seem to happen? Might that be related? Now the |-char, linux:

e.keyCode: 124
e.altKey: false
e.ctrlKey: false
e.keyCode || e.which: 124
e.shiftKey: false
e.metaKey: false
e.charCode: 124

windows:

e.keyCode: 124
e.altKey: true
e.ctrlKey: true
e.keyCode || e.which: 124
e.shiftKey: false
e.metaKey: false
e.charCode: 124

again this discrepancy that windows sends altKey and ctrlKey true where as linux does not. I'll take a closer look this evening.

@parisk
Copy link
Contributor

parisk commented Jun 16, 2016

Great thanks a lot. A PR for this would be wonderful.

@chani
Copy link
Contributor Author

chani commented Jun 16, 2016

pretty sure the fix will take me longer @runarberg could you work something out with the information provided? If not I am going to try, though my JS is.. limited, not even sure where to start. :)

@runarberg
Copy link
Contributor

runarberg commented Jun 16, 2016

@chani We already take this into account. Linux (at least ubuntu) handles third level shift natively with a special key mapped to it (usually AltGr), so ubuntu machines work out of the box (and always have). The problem arises with Windows and especially mac, because they don't have this notion, and rely on control characters being pressed. #60 fixes this discrepancy by still emitting key event to the terminal when event.ctrlKey and event.altKey are present in the keyboard event on windows or when event.altKey is present in Mac OS.

I recently tried this on both Edge and Chrome 51 on a windows 10 machine, and was able to input these characters on an ISO keyboard using either Ctrl + Alt + Q@ or AltGr + + ⇒ ```.

Note: unlike Ubuntu, MS Windows usually maps AltGr to Ctrl + Alt.

@chani
Copy link
Contributor Author

chani commented Jun 16, 2016

@parisk @runarberg thank you both. using CTRL+ALT+Q and CTRL+ALT+< it does work as expected. However, the "usual" key-stroke which would be ALTGR+Q and ALTGR+< does not work in windows. I think this is good enough.

@chani
Copy link
Contributor Author

chani commented Jun 16, 2016

correction.. sorry. it seems strg+alt+q does NOT work in chrome in windows.

@Tyriar
Copy link
Member

Tyriar commented Jun 1, 2018

I think this is fixed in 3.2.0 https://github.com/xtermjs/xterm.js/releases/tag/3.2.0

@Tyriar Tyriar closed this as completed Jun 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is misbehaving
Projects
None yet
Development

No branches or pull requests

6 participants