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

Bad key map for < #45

Closed
m1k1o opened this issue Apr 6, 2021 · 17 comments
Closed

Bad key map for < #45

m1k1o opened this issue Apr 6, 2021 · 17 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@m1k1o
Copy link
Owner

m1k1o commented Apr 6, 2021

When writing < on US keyboard (shift + ,) it writes > instead. In Guacamole (their keyboard is being used) it works without issues.

@m1k1o m1k1o added bug Something isn't working help wanted Extra attention is needed labels Apr 6, 2021
@mbattista
Copy link
Contributor

I checked in the server/internal/xorg/xorg.go and it got the following key on press shift + < (US Keyboard):
pressed key: 65505
pressing key 60
Output: >

Same with a german Keyboard :
pressing key 60
Output: <

It seems that shift + , is wrongly mapped in this case. It also seems that this is the only combination which seems to be broken.

@m1k1o
Copy link
Owner Author

m1k1o commented Apr 7, 2021

Key 60 (0x003c) is even named by /usr/include/X11/keysymdef.h file as LESS-THAN SIGN.

image

Meaning that Shift + LESS-THAN SIGN produces GREATER-THAN SIGN.
But interestingly Shift + GREATER-THAN SIGN does not produce LESS-THAN SIGN.

I'm really curious, if that is some bug originates from xorg or keyboard layout itself. Or if this was somehow intended by original creators.

I'm using Slovakian keyboard layout, and no problems there either.

@m1k1o
Copy link
Owner Author

m1k1o commented Apr 7, 2021

Or maybe just bad default variant of US keyboard is being used. It looks like the right bottom keyboard, that indeed has a key that produces with shift + < = > character.

image
Source: http://www.farah.cl/Keyboardery/A-Visual-Comparison-of-Different-National-Layouts/

@mbattista
Copy link
Contributor

I suspect the shift key to be the problem. Since on a US Keyboard (I have a US and a german one) sends the correct ASCII number (60) for the less-than sign. The same ASCII key without shift on the same layout (german keyboard on US layout) produces the correct sign.

@m1k1o
Copy link
Owner Author

m1k1o commented Apr 7, 2021

Interestingly, with UK keyboard it is working properly.

Or when switching to mac variant (found in /usr/share/X11/xkb/rules/base.lst source) using setxkbmap -variant mac.

image

@mbattista
Copy link
Contributor

setxkbmap us intl works like intended

@mbattista
Copy link
Contributor

but... I have no fix in mind....
https://github.com/m1k1o/neko/blob/dev/server/internal/xorg/xorg.go#L222 could check us and append an intl?

Trying to replace the us layout with the us intl layout while building the docker?

@m1k1o
Copy link
Owner Author

m1k1o commented Apr 7, 2021

Yeah, that looks as the only way to fix it for now. It is some pretty dirty workaround, but effective.

m1k1o added a commit that referenced this issue Apr 7, 2021
@mbattista
Copy link
Contributor

Sorry, I did not test properly. us intl breaks the ' key :(

@m1k1o
Copy link
Owner Author

m1k1o commented Apr 7, 2021

It would be beneficial to have some test, that can check all of the keys, and ideally in multiple languages.

So right now, setxkbmap us mac seems to be wroking fine. I did not find any bad key (yet).

m1k1o added a commit that referenced this issue Apr 7, 2021
@mbattista
Copy link
Contributor

setxkbmap us altgr-intl also seems to work. But I also did not find any bad key with mac so take mac?

@m1k1o
Copy link
Owner Author

m1k1o commented Apr 7, 2021

From the comment in /usr/share/X11/xkb/symbols/us file, it looks like on intl variant if you want to write ' you have to press additional space. And that is presumably altgr-intl avoiding, that might be our choice.

// I do NOT like dead-keys - the International keyboard as defined by Microsoft
// does not fit my needs. Why use two keystrokes for all simple characters (eg '
// and <space> generates a single ') just to have an <C3><A9> (eacute) in two strokes
// as well? I type ' more often than <C3><A9> (eacute).
//
// This file works just like a regular keyboard, BUT has all dead-keys
// accessible at level3 (through AltGr). An <C3><AB> (ediaeresis) is now: AltGr+"
// followed by an e. In other words, this keyboard is not international as long
// as you leave the right Alt key alone.
//
// The original MS International keyboard was intended for Latin1 (iso8859-1).
// With the introduction of iso8859-15, the (important) ligature oe (and OE)
// became available. I added them next to ae. Because I write ediaeresis more
// often than registered, I moved registered to be next to copyright and added
// ediaeresis and idiaeresis. - Adriaan

partial alphanumeric_keys
xkb_symbols "altgr-intl" {

   include "us(intl)"
   name[Group1]= "English (intl., with AltGr dead keys)";

// five dead keys moved into level3:

   key <TLDE> { [    grave, asciitilde,  dead_grave,   dead_tilde      ] };
   key <AC11> { [apostrophe,quotedbl,    dead_acute,   dead_diaeresis  ] };

// diversions from the MS Intl keyboard:

   key <AE01> { [        1, exclam,      onesuperior,  exclamdown      ] };
   key <AD04> { [        r, R,           ediaeresis,   Ediaeresis      ] };
   key <AC07> { [        j, J,           idiaeresis,   Idiaeresis      ] };
   key <AB02> { [        x, X,           oe,           OE              ] };
   key <AB04> { [        v, V,           registered,   registered      ] };

// onequarter etc (not in iso8859-15) moved to get three unshifted deadkeys:

   key <AE06> { [        6, asciicircum, dead_circumflex, onequarter    ] };
   key <AE07> { [        7, ampersand,   dead_horn,       onehalf       ] };
   key <AE08> { [        8, asterisk,    dead_ogonek,     threequarters ] };

   include "level3(ralt_switch)"
};

I'll test both of them and see if I find reason to switch to altgr-intl from mac, if both of them work good. Maybe just because of naming convention, since we are running it on debian, getting rid of term mac would be a good reason.

@m1k1o
Copy link
Owner Author

m1k1o commented Apr 9, 2021

I investigated and found following:

To clarify some terminology used here (as I understood it after hours of researching, so note that):

  • key modifier -> Key, that is modifying following keys either while pressed (Shift, Alt) or when active (CapsLock).
  • KeyCode -> Physical key on your keyboard [8, 255].
  • KeySym -> Key, that is generated by your keyboard after applied key modifiers. It can be uppercase A, lowercase a etc...
  • keyboard layout -> Table, with KeyCodes as rows and key modifiers as columns. It is mapping, what KeySym is generated, when pressing KeyCode, depending on active key modifiers.

One KeyCode can have multiple KeySyms. What we get from a browser is a KeySym. When we want to send it to the server, we need to find corresponding KeyCode for it, in order to simulate physical key being pressed on a keyboard. This is done using XKeysymToKeycode and is dependent on current keyboard layout.

This works almost all the time... except when there is multiple KeyCodes for a single KeySym. That would not be a huge problem, unless the KeySym is generated using different key modifier.

Pressing left shift generates 0xffe1 KeySym. When pressing , key (while having shift sill pressed) browser generates KeySym 0x003c that is < character. We are sending both to the server, as they are typed. So, server receives 0xffe1 and 0x003c. It needs to find KeyCodes for them. Using XKeysymToKeycode function yields 62 and 94. When we send them to xorg server, < is written. Why?

We need to investigate KeyCodes, that were sent to xorg. Searching for 0xffe1 (shift) in keymap leads to single key code.

root@bb3669a838d2:/# xmodmap -pk | grep 0xffe1
     50         0xffe1 (Shift_L)        0x0000 (NoSymbol)       0xffe1 (Shift_L)

When we look for the 0x003c (<) key, we see two KeyCodes. Our previous result was KeyCode 94, and since we have already applied shift modifier, the second column will be read. That yields 0x003e (>), what will be written.

root@bb3669a838d2:/# xmodmap -pk | grep 0x003c
     59         0x002c (comma)  0x003c (less)   0x002c (comma)  0x003c (less)
     94         0x003c (less)   0x003e (greater)        0x003c (less)   0x003e (greater)        0x007c (bar)    0x00a6 (brokenbar)      0x007c (bar)

Since XKeysymToKeycode is returning only single KeyCode, it choose "the other one". There is no way of knowing, which KeyCode was meant by user, when pressing that KeySym. Although, we have that information:

  • it needs to be KeyCode, that creates following KeySym with currently applied modifiers.

That would mean, we need to keep track of pressed modifiers and search according to them.

Another solution could be, using "empty" keymap and on each keypress allocating new KeyCode getting entirely rid of layout. Although, I am not sure how that would be working and if it would be even possible.

Or simply disabling / remapping that single key somehow.

Other vendors (novnc, tigervnc...) have faced exactly this problem, it looks like they solved it by having custom mapping rather than relaying on XKeysymToKeycode. What we are using now, that is different keyboard variant, that has this key differently allocated.

@m1k1o
Copy link
Owner Author

m1k1o commented Apr 9, 2021

Same problem here TigerVNC/tigervnc#491 with their solution.

@mbattista
Copy link
Contributor

mbattista commented Apr 10, 2021

Wow, good job diving in this deep.

I worked the solution from tigervnc into the code. (#46)
It works (right key is displayed) but crashes with a bad value. I will try to debug it later.

m1k1o added a commit to mbattista/neko that referenced this issue Apr 11, 2021
@m1k1o
Copy link
Owner Author

m1k1o commented Apr 11, 2021

After some intensive tests (or you can call it smashing in the keyboard) sometimes it ends with panic.
image

@m1k1o
Copy link
Owner Author

m1k1o commented Apr 12, 2021

Fixed by #46.

@m1k1o m1k1o closed this as completed Apr 12, 2021
m1k1o pushed a commit that referenced this issue Jun 23, 2024
Bumps [@babel/traverse](https://github.com/babel/babel/tree/HEAD/packages/babel-traverse) from 7.22.8 to 7.23.2.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.23.2/packages/babel-traverse)

---
updated-dependencies:
- dependency-name: "@babel/traverse"
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants