Bug 4677 - ThinLinc Client sends KP_8 etc instead of KP_Up with NumLock+Shift
Summary: ThinLinc Client sends KP_8 etc instead of KP_Up with NumLock+Shift
Status: CLOSED FIXED
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: VNC (show other bugs)
Version: trunk
Hardware: PC Unknown
: P2 Normal
Target Milestone: 4.3.0
Assignee: Pierre Ossman
URL:
Keywords: astrand_tester, relnotes
Depends on:
Blocks: 3523 5228 5248
  Show dependency treegraph
 
Reported: 2013-06-03 14:54 CEST by Peter Åstrand
Modified: 2014-10-21 13:49 CEST (History)
0 users

See Also:
Acceptance Criteria:


Attachments

Description Peter Åstrand cendio 2013-06-03 14:54:52 CEST
Currently, the TL client has "windows like numlock", ie Shift does not cancel NumLock. Keysym 0xffb8 is sent regardless of if shift is depressed or not.
Comment 1 Peter Åstrand cendio 2013-06-03 15:00:16 CEST
Debug printouts when pressing KP_8 with NumLock on:

 Viewport:    Key pressed: 0xffb8 (0xffb8) '8' => 0xffb8
 Viewport:    Key released: 0xffb8 => 0xffb8

And with shift:

Mon Jun  3 14:59:48 2013
 Viewport:    Key pressed: 0xffe1 (0xffe1) '' => 0xffe1

Mon Jun  3 14:59:51 2013
 Viewport:    Key pressed: 0xffb8 (0xffb8) '8' => 0xffb8
 Viewport:    Key released: 0xffb8 => 0xffb8

Mon Jun  3 14:59:55 2013
 Viewport:    Key released: 0xffe1 => 0xffe1
Comment 2 Pierre Ossman cendio 2013-06-04 11:06:15 CEST
We have an open request from upstream to generally discuss our suggested changes to the keyboard handling. We need to look at that at the same time.
Comment 3 Pierre Ossman cendio 2013-08-30 10:50:48 CEST
Ugh. FLTK's keyboard handling is messy as hell. I don't think we can fix this without severely redesigning how FLTK handles key presses. So there are two options:

1. Ugly hack and blindly assume that Shift always negates NumLock on X11.

2. Rethink our keyboard strategy in FLTK.


I'm leaning towards 2. Our keyboard needs are a bit peculiar and dictated by how RFB is defined. It's probably cleaner for us and upstream if instead we have a mechanism to intercept low level system events and do whatever magic is necessary inside vncviewer instead. FLTK might need to help out in disabling IM though.
Comment 4 Pierre Ossman cendio 2013-08-30 11:00:52 CEST
(In reply to comment #2)
> We have an open request from upstream to generally discuss our suggested
> changes to the keyboard handling. We need to look at that at the same time.

Will handle this on bug 4786 instead as there are other FLTK issues that needs to be dealt with.
Comment 5 Pierre Ossman cendio 2014-02-06 11:10:44 CET
This bug has transformed into rewriting our keyboard changes for FLTK. My current plan is to introduce a callback routine in FLTK where we can get raw system events (MSG:s on Windows, XEvent:s on X11, etc.). All the magic will then be in vncviewer.
Comment 6 Pierre Ossman cendio 2014-02-06 11:11:25 CET
Consider bug 4984 and bug 4971 when writing the new code.
Comment 7 Pierre Ossman cendio 2014-08-21 17:38:38 CEST
Urgh. Windows is just full of horrible hacks in its keyboard code. Besides bug 5226, I've also seen these fun things:

 a) With num lock on, pressing shift-1 results in:

    (fake) shift release
    keypad end press
    keypad end release
    (fake) shift press

 b) Windows cannot track both shift keys properly. If you press both of them, you get two WM_KEYDOWN. But if you release them in the same order as you pressed them, the first WM_KEYUP will just vanish.
Comment 8 Pierre Ossman cendio 2014-08-22 11:55:18 CEST
(In reply to comment #7)
> Urgh. Windows is just full of horrible hacks in its keyboard code. Besides bug
> 5226, I've also seen these fun things:
> 
>  a) With num lock on, pressing shift-1 results in:
> 
>     (fake) shift release
>     keypad end press
>     keypad end release
>     (fake) shift press
> 

Bug 5228.

>  b) Windows cannot track both shift keys properly. If you press both of them,
> you get two WM_KEYDOWN. But if you release them in the same order as you
> pressed them, the first WM_KEYUP will just vanish.

Bug 5229.
Comment 9 Pierre Ossman cendio 2014-08-22 13:56:28 CEST
(In reply to comment #7)
>  a) With num lock on, pressing shift-1 results in:
> 
>     (fake) shift release
>     keypad end press
>     keypad end release
>     (fake) shift press

More fun. If you release shift before End, you'll see a "1" on the release event instead of a matching End. This of course makes it easy to get out of sync.
Comment 10 Pierre Ossman cendio 2014-08-22 14:21:51 CEST
(In reply to comment #9)
> (In reply to comment #7)
> >  a) With num lock on, pressing shift-1 results in:
> > 
> >     (fake) shift release
> >     keypad end press
> >     keypad end release
> >     (fake) shift press
> 
> More fun. If you release shift before End, you'll see a "1" on the release
> event instead of a matching End. This of course makes it easy to get out of
> sync.

The solution to this is to track keys using their scan code, not their vkey. This is a bit pesky in practice though as Windows sometimes forget to inform us of the scan code, so we have to go digging for it.
Comment 11 Pierre Ossman cendio 2014-08-22 15:19:06 CEST
I have something that works well now. Available here:

https://github.com/CendioOssman/tigervnc/tree/xhandlers
Comment 13 Pierre Ossman cendio 2014-09-15 13:28:32 CEST
FLTK patches committed upstream and in cenbuild.
Comment 14 Pierre Ossman cendio 2014-09-15 13:59:43 CEST
Vendor drop of TigerVNC in r29365.
Comment 15 Pierre Ossman cendio 2014-09-16 16:21:54 CEST
AltR is sent as AltL from Windows. Need to have a look at that.
Comment 16 Pierre Ossman cendio 2014-09-17 10:32:46 CEST
(In reply to comment #15)
> AltR is sent as AltL from Windows. Need to have a look at that.

Not a bug. AltR wasn't available in the server key map so it picked AltL as a replacement.
Comment 17 Pierre Ossman cendio 2014-09-18 14:46:55 CEST
All done.

Test instructions are available as vnc/tigervnc/doc/keyboard-test.txt.
Comment 18 Peter Åstrand cendio 2014-09-29 13:05:55 CEST
Tested Linux/X11/CentOS6 and Mac OS X:

* In general, the keyboard handling works very good. Everything I've been able to test works as expected. 

* For this particular bug - the problem with KP_8 vs KP_Up - I've verified with Wireshark that shift indeed cancels NumLock on CentOS6, and that KP_Up is sent. However, on the server side, it is translated to "Up". NumLock is also off in the session. This means that in applications such as Gedit, the behaviour is not exactly like a local machine: NumLock+Shift+KP_8 will both move the cursor up as well as mark the text, since the GTK mechanism of "treat Shift as Numlock disabler instead of region marker" does not work in this case. But this is a server side problem.
Comment 19 Peter Åstrand cendio 2014-09-30 13:40:47 CEST
The Windows platform has been tested as well. Only one minor problem:

* The PrtScr/SysRq key does not work as expected: Sends XK_Print only when Alt is pressed; nothing without modifiers.
Comment 20 Pierre Ossman cendio 2014-09-30 17:10:57 CEST
(In reply to comment #19)
> The Windows platform has been tested as well. Only one minor problem:
> 
> * The PrtScr/SysRq key does not work as expected: Sends XK_Print only when Alt
> is pressed; nothing without modifiers.

Urgh. That key was very magical. Should be fixed in r29441.

Bonus points for testing pressing/releasing Alt+PrtScr in different orders as the damn thing can end up sending different scan codes on press and release.
Comment 21 Peter Åstrand cendio 2014-10-06 15:01:06 CEST
(In reply to comment #20)
> (In reply to comment #19)
> > The Windows platform has been tested as well. Only one minor problem:
> > 
> > * The PrtScr/SysRq key does not work as expected: Sends XK_Print only when Alt
> > is pressed; nothing without modifiers.
> 
> Urgh. That key was very magical. Should be fixed in r29441.
> 
> Bonus points for testing pressing/releasing Alt+PrtScr in different orders as
> the damn thing can end up sending different scan codes on press and release.

Works fine.
Comment 22 Peter Åstrand cendio 2014-10-20 14:40:20 CEST
On Windows, the first keypress is lost. Happens also for the F8 menu. Does not happen with 4.2.0.
Comment 23 Pierre Ossman cendio 2014-10-20 16:15:00 CEST
(In reply to comment #22)
> On Windows, the first keypress is lost. Happens also for the F8 menu. Does not
> happen with 4.2.0.

Only happens on full screen. Keyboard grab is not relevant though.

Turns out this is an old bug in FLTK. We have a workaround in vncviewer for it, but it is no longer effective with the new keyboard code. And it doesn't seem like another workaround can be applied. So we need to fix the original bug.
Comment 24 Pierre Ossman cendio 2014-10-20 17:32:38 CEST
Fixed in r29523.
Comment 25 Peter Åstrand cendio 2014-10-21 13:49:31 CEST
(In reply to comment #24)
> Fixed in r29523.

Works.

Note You need to log in before you can comment on or make changes to this bug.