-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Fix cross platform input problems #60
Conversation
I've been having some serious cross platform inconsistencies using this package. Most serious is not being able to input fifth-level keys with my Icelandic keyboard. Meaning I'm unable to enter keys like the pipe I've tested this with Icelandic keyboard on firefox 46.0.1 and chrome 50.0.2661.102 (64-bit) on Mac OS X 10.11.5 as well as on MS Edge on Windows 10. Note that this brakes copy-pasting using |
Hi @runarberg, thanks a lot for your contribution. Let me make a few comments please:
This is a great addition. It actually sounds a lot more sane utilizing the user agent's platform attribute. As far as hitting different combinations of Ctrl + any key that trigger default browser behavior, this should not be handled by xterm.js, but by the application that uses it. Since xterm.js is just a front-end terminal emulator, it does not imply its use case. For example:
Last, copy and paste should not be broken for any platforms, as xterm.js is only a front-end terminal emulator. While Ctrl + C might make sense not to copy in Windows, but send a keyboard interrupt to a back-end Linux terminal, this case should not be handled by xterm.js itself, but by the parent application. Also would it be able to apply particular rules based on the language of the user agent (use Do you believe we could fix these comments, in order to merge this PR? Feel free to open also an issue, in order to discuss anything you believe we need to. |
// TODO: This also brakes copy-pasting into the terminal using | ||
// ``ctrl + C'' and ``ctrl + V''. Find a different way of | ||
// allowing users to copy-paste intuitively. | ||
this.cancel(ev, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@parisk If I remove this part, would you consider merging the rest?
@parisk I don't know if the
In this PR I opted for detecting the system, rather than the preferred language, because generally the popular OS platforms are consistent in how a fifth-level key is reached:
|
I amended the commit. I stopped preventing the browsers default behavior, stopped referring to the “fifth-level” (after some investigation, it is actually called a “third level shift”), and simplified the commit message. |
Thanks for the info and commits @runarberg. What you are writing sound quite rational. We just have to make sure that they keep the desired user experience for most of potential users. I will give a deeper look into this today and also make a local rebase with #78 to see if everything works 🆗 . |
I pulled this branch, checked it out, rebased with master and still I cannot copy and paste using my keyboard on OS X. The existing experience should not break for the majority of users. @runarberg can you please list all behavior that is expected to break after your latest patches and possible workarounds for each of them? |
@parisk This is because we are using ⌘ Cmd on Mac OS as Alt on other platforms. If you prevent default behavior on the terminal key event, this key won't emit default behavior. For example ⌘ Cmd + C will act as Alt + C on other systems, and emit “Capitalize word from cursor” to the terminal instead of “Copy” to the browser window. This is the only expected behavior that brakes that I can think of, unless some windows users have browser default behavior to Ctrl + Alt + key. |
I amended the commit, stopped using Mac OS ⌘cmd as Alt on other systems (now Mac OS users have to use Esc like before), and rebased onto upstream/master. Keyboard copy-pasting works again on Macs. Ps. I discovered an unintended side-effect where chrome users on ubuntu can use ⇧shift + Ctrl + V to paste. ⇧shift + Ctrl + C wont work for copying, nor does this work on firefox, nor edge on windows. |
Thanks a lot for making progress on this. I move forward with pulling your branch and testing right now. |
Okay the only issue that I found out is that Ctrl + Shift + V. Won't work for pasting in Firefox. Was that just me or happens to you as well? Also I think that we can manage with making Ctrl + Shift + C work, as it invokes the developer tools by default. |
@parisk I don't think Ctrl + Shift + key should work as a replacement for Ctrl + key in firefox on ubuntu, it didn't doesn't do so upstream either. It works as a replacement in chrome (and Terminator) for some reason. We could bind Ctrl + Shift + {V, C} to |
I submitted #107 to fix keyboard copy-pasting on non-macs. |
b9350b8
to
3d00632
Compare
Ammended: Fixed a bug in the last patch, and rebased onto upstream. |
Hi @runarberg, thanks again for your patches. It seems that Alt + ←/→ does not work any more to move back and forward with words (tested on Chrome on Mac). Is there any workaround for this? |
Also could you please provide us some documentation/reference in order to see how third level keys should work? This way we will be able to test the new functionality on top of the existing one and make sure things won't break for mainstream users. |
ISO third level keys were not working. That prevented some inputting important characters (like pipe `|` and caret `^`) on some keyboard layouts. Instead of parsing the user-agent string to find the users os, we now look into the `platform` attribute of the `navigator` object. Removed the hijacking of the command key `⌘` on Mac OS as the `Alt` key on other systems.
Amended: Fixed Alt + ←/→, added unit tests, and rebased on top of upstream. |
@parisk I don't really know much about keyboard input methods, except from experience of using ISO keyboard, and frustration when ANSI keyboard is forced upon me. But here is a wikipedia article about the ISO keyboard standard. |
👍 thanks a lot for one more time. This seems to be working great now. Merging. |
important characters (like pipe
|
and caret^
) on some keyboards.platforms.
ctrl + D
for example sent the EOT and opened thebookmark section on MS Edge.
that maps
metaKey
on Mac OS to thealtKey
on othersystems. However that logic didn't work.
Instead of parsing the user-agent string to find the users os, we now
look into the
platform
attribute of thenavigator
object.