Comment 1 Peter Åstrand 2003-11-10 13:58:05 CET

When using numlock sync, there are some things that needs to be taken care of.
For example:

* How to handle multiple clients, connected at the same time

* How to handle the case where on client reconnects to a session. 

I think the most simply approach would be this:

* Whenever a client connects, the server numlock state is syncronized with the
client. There is only problem with this approach: Assume that one client is
connected to the server. Numlock is off (both on the client and in Xvnc). Assume
that one additional client connects to Xvnc. This new client has numlock on.
This means that the Xvnc numlock state will be turned on. But, without some kind
of "callback" to the first client, the first client will have numlock off, even
though Xvnc now has numlock on. 

Maybe we can disregard this last problem, though. 

Comment 2 Peter Åstrand 2004-01-20 10:51:56 CET

I've modified rdesktop, so that it supports both "modes"; either keep the WTS in
sync with the Xserver numlock state, or toggle when needed. This last mode is
default. This makes the latest CVS of rdesktop work with ThinLinc without
problems. This solution probably is sufficient for now. 

Comment 3 Peter Åstrand 2005-06-07 14:56:34 CEST

Sulamerika has problems with this, because they are using the old Reflection 1
software. 

The rdesktop implementation of "-N" (numlock sync) was a bit buggy, since it
only synced numlock on FocusIn. I've patched it, so that it syncs on every
Num_Lock release. 

The problem with the Xvnc state still exists. My idea for Sulamerika was to
"permanently" enable NumLock in Xvnc, but this is somewhat hard to do: To change
and inspect the numlock state, it must be listed as a modifier. So, if we are
not adding Num_Lock as an modifier, rdesktop can't check the state. But: If we
*are* adding it as a modifier, the state will toggle upon every Num_Lock keysym.
The Windows client doesn't send Num_Lock events, but the Linux client does. 

I've implemented a temporary solution in Xvnc: Num_Lock is added as a modifier,
but all VNC Num_Lock events are ignored. This makes it possible to change the
Xvnc numlock state with "numlockx", and rdesktop can inspect it. 

(It would be nice if numlock was default, so that we didn't need to use
"numlockx", but I don't know how to fix this.)

Comment 4 Peter Åstrand 2005-06-07 15:12:48 CEST

One easy way of implementing the "numlock sync" protocol over VNC is probably this:

* A Num_Lock press means "turn numlock on"

* A Num_Lock release means "turn numlock off". 

The disadvantage is that if you are using an "old" client with a server that
doesn't use this convention, then a NumLock press will turn numlock off,
regardsless of the previous state. Also, if you are using a "new" client with an
old server, things might also work a bit strange. 

A new protocol message also has the advantage that it can be used for
synchronizing Caps Lock, Scroll Lock and others, as well. 

Comment 5 Peter Åstrand 2006-04-18 17:11:18 CEST

See also bug 1919. 

Comment 6 Peter Åstrand 2013-05-21 12:59:32 CEST

With ThinLinc 4.0.0, it is not possible to change NumLock (or CapsLock) from the X side using "numlockx".

Comment 7 Peter Åstrand 2013-05-21 13:11:48 CEST

(In reply to comment #6)
> With ThinLinc 4.0.0, it is not possible to change NumLock (or CapsLock) from
> the X side using "numlockx".

Correction. It is possible, if you use Xtest and simulate a XK_Num_Lock key press, but refrains from doing a XK_Num_Lock key release. Unfortunately, the 4.0.0 code does not care if Caps Lock is set: It will generate faked shifts anyway for incoming capital letters. 4.1 should be better though.

Comment 8 Pierre Ossman 2013-07-03 09:38:03 CEST

Qemu seems to have some undocumented extension for keyboard LEDs. Might be something that can be used.

Comment 9 Pierre Ossman 2016-12-13 15:47:54 CET

TigerVNC support for QEMU's extension:

https://github.com/CendioOssman/tigervnc/tree/ledstate

Seems to work fairly well, but might need more testing, primarily with old clients.

Comment 10 Pierre Ossman 2017-09-12 16:31:15 CEST

Xorg's keyboard model with virtual devices, slaves and masters also complicate things. E.g. this bug:

https://bugs.freedesktop.org/show_bug.cgi?id=16145

However it seems to be fixed in later versions of Xorg:

https://cgit.freedesktop.org/xorg/xserver/commit/?id=45fb3a934dc0db51584aba37c2f9d73deff9191d

We need to upgrade or back port that.

Comment 11 Pierre Ossman 2018-01-23 14:00:17 CET

Support for this has now been merged in to upstream TigerVNC.

Comment 12 Pierre Ossman 2018-04-23 13:31:14 CEST

*** Bug 4548 has been marked as a duplicate of this bug. ***

Comment 13 Pierre Ossman 2018-04-23 13:34:26 CEST

*** Bug 5205 has been marked as a duplicate of this bug. ***

Comment 14 Pierre Ossman 2018-08-28 10:02:42 CEST

Some general tests of the keyboard on all platforms is probably prudent as the code got a lot of changes as part of this.

Comment 16 Pierre Ossman 2018-10-15 14:18:41 CEST

Browsers won't let us modify the local state, so we cannot implement the same thing in noVNC/Web Access. We could do client-to-server sync in some cases, but the fallback handling in the server does that as well. So we've opted to not modify Web Access and let the server handle it as it would an older native client.

Bug 7259 has been opened to track if we can improve this in the future.

Comment 17 Karl Mikaelsson 2018-10-16 11:00:18 CEST

Web Access input is broken, apart from num pad numbers, with num lock on.

Comment 18 Pierre Ossman 2018-10-16 11:09:43 CEST

Argh. The vendor drop we have of noVNC includes a half-finished version of the QEMU keyboard extension. And since the server now supports that extension that broken code now gets activated. It has been fixed upstream, but it's a large change unfortunately. For now it's probably best just to disable the broken stuff and rely on the previous keyboard code that we've been using in the past.

Comment 20 Karl Mikaelsson 2018-10-16 15:28:11 CEST

From the TigerVNC keyboard test instructions:

> - Key repeat should not send repeated release events

new client on Linux, new server: Pressing and holding a key results in sequences of KeyPress+KeyRelease for the given key on the server.

Comment 21 Pierre Ossman 2018-10-19 15:22:14 CEST

(In reply to comment #20)
> From the TigerVNC keyboard test instructions:
> 
> > - Key repeat should not send repeated release events
> 
> new client on Linux, new server: Pressing and holding a key results in
> sequences of KeyPress+KeyRelease for the given key on the server.

This feature has a massive gotcha; most applications do not display this correctly. The X server (Xvnc in our case) lies to the applications and sends fake KeyRelease events unless the application has explicitly asked it not to.

Right now we have to test this by looking at the server log when in debug mode (-Log *:stderr:100).

I'll update the test instruction with this information.

Comment 23 Pierre Ossman 2018-10-19 15:33:33 CEST

We've now tested this on Linux, Windows, macOS and with Web Access:

> * A new client connecting to an old server should not see any change in behaviour compared to an old client

Works. Seeing the old fake shift behaviour, or numpad getting alternative keys.

> * An old client (or Web Access) connecting to a new server should get the correct CapsLock and NumLock state in the session whenever A-Z is used, or keypad 0-9 (when Shift isn't pressed)

Yup, works well. Except for macOS. On that platform Shift doesn't cancel CapsLock. The effect is that the algorithm guesses wrong and disables CapsLock.

We decided to keep things as-is as we couldn't see a way to handle CapsLock+Shift correctly for both macOS and Windows/Linux clients. So for now we'll get it wrong for macOS but right for the other two. User should hopefully not see any issues as the resulting symbol is still correct.

> * The server state should be set to the client state on connect

Works.

> * The server state should be set to the client state when the client regains focus

Works.

> * Pressing NumLock/CapsLock/ScrollLock should toggle server state (assuming that lock key is configured in the server layout)

Works. We had to enable ScollLock in the session and on Linux clients though:

  xmodmap -e "add mod3 = Scroll_Lock"

> * Changes to server state should change client state when the client has focus, and be ignored when the client doesn't have focus

Works.

> * Server ScrollLock state should be unaffected when a macOS client is used (macOS doesn't have ScrollLock)

Works.


We also went through the TigerVNC keyboard tests on all three native platforms and did not see any regressions.

Comment 24 Pierre Ossman 2019-01-28 13:26:46 CET

Another issue that this should solve:

When the keyboard layout uses "." on the numpad (KP_Decimal), then you'd usually get keycode 129 in the session rather than the more common 91. This is because we are in the wrong NumLock state to use 91, and we find key 129 which is a dedicated decimal key found on some keyboards.

For many applications this doesn't matter, but those that want physical keys can get confused. E.g. FreeRDP.

Comment 26 Pierre Ossman 2019-02-27 14:24:26 CET

Something is broken here. I'm seeing NumLock getting turned off when connecting to my Ubuntu 18.04 server. Seen on several Linux clients.

Comment 27 Pierre Ossman 2019-02-27 14:44:42 CET

Argh. The problem went away after restarting the session. However I got it back after playing around with several machines in the order: Fedora, macOS, Windows, HP ThinPro.

Comment 28 Pierre Ossman 2019-02-28 10:01:41 CET

The problem is extremely random, but I managed to catch it now with a debug version of vncviewer.

I can see the client trying to do the initial sync. It sends over a fake NumLock in order to change the server state. But it doesn't seem to have any effect.

It tries a second time when it gets focus. This time it gets a response from the server. However it is still the incorrect state. But the client respects this state, since we are past the initial sync, and changes the local client state.


Maybe there is just an update that is lost somewhere? Since we end up sending two fake NumLock it might be toggling back and forth.

Comment 29 Pierre Ossman 2019-02-28 10:37:39 CET

Problem found!

The issue has to do with the fact that we are really doing two syncs on connect:

 1. When the server first announces it supports LED states

 2. When we get the first focus when the window is created

If these two happen quickly enough then there will not be time for the server to push an update of its state between the two syncs. And since we sync by toggling, not by setting, we end up toggling twice because we haven't realised the server has already been updated (the client just isn't informed yet).


There are two ways of solving this, with their own drawbacks:

 a) Avoid the first sync. It is only really needed if the server announces support when we already have focus, so we could limit it to that case. That would solve this case, but it would not solve a similar case where focus is quickly toggled back and forth without the server having time to respond. That will probably not happen because of user interaction, but it might be a side effect of e.g. a window quickly popping up and going away.

 b) Keep track of when we are already trying to sync. The problem is we don't really know if the sync is done as the server might ignore some states (e.g. ScrollLock in most cases). So we might block future, legitimate syncs of other keys. Fences could be an option, but that is complex and not supported by all servers.


I think a) might be good enough, even if it leaves a theoretical corner case.

Comment 31 Pierre Ossman 2019-02-28 11:23:00 CET

Should be fixed now.

It seemed to be more likely with a large session size, as that increased the size (and hence time) of the first update.

Comment 32 Samuel Mannehed 2019-03-05 12:01:29 CET

Seems to work flawlessly now. I have tried to be really evil by spam-toggling NumLock and CapsLock while starting a new really high-resolution session multiple times, can't see any issues.

Tested with 4.10.0beta1 with Fedora 29 client and server.