Hi,
On Thu, Mar 8, 2018 at 4:27 PM, Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote:
I don't think this is desirable logic - blindly assuming everythingOn Thu, Mar 08, 2018 at 04:19:03PM +0100, Olivier Fourdan wrote:
> When running on Xwayland, the keycode mapping property is not available,
> which causes unknown keycode mapping errors and the keyboard doesn't
> work.
>
> Use a known scancode (“XK_Page_Up”) to check against the AT scancode and
> use “xfree86” if it matches or assume “evdev” for anything else.
>
> Signed-off-by: Olivier Fourdan <ofourdan@xxxxxxxxxx>
> ---
> src/vncdisplaykeymap.c | 33 ++++-----------------------------
> 1 file changed, 4 insertions(+), 29 deletions(-)
>
> diff --git a/src/vncdisplaykeymap.c b/src/vncdisplaykeymap.c
> index 9ee501d..135412f 100644
> --- a/src/vncdisplaykeymap.c
> +++ b/src/vncdisplaykeymap.c
> @@ -162,8 +162,6 @@ const guint16 *vnc_display_keymap_gdk2xtkbd_table(GdkWindow *window,
> {
> #ifdef GDK_WINDOWING_X11
> if (GDK_IS_X11_WINDOW(window)) {
> - XkbDescPtr desc;
> - const gchar *keycodes = NULL;
> GdkDisplay *dpy = gdk_window_get_display(window);
>
> /* There is no easy way to determine what X11 server
> @@ -175,17 +173,6 @@ const guint16 *vnc_display_keymap_gdk2xtkbd_table(GdkWindow *window,
> */
>
> Display *xdisplay = gdk_x11_display_get_xdisplay(dpy);
> - desc = XkbGetMap(xdisplay,
> - XkbGBN_AllComponentsMask,
> - XkbUseCoreKbd);
> - if (desc) {
> - if (XkbGetNames(xdisplay, XkbKeycodesNameMask, desc) == Success) {
> - keycodes = gdk_x11_get_xatom_name(desc->names->keycodes);
> - if (!keycodes)
> - g_warning("could not lookup keycode name");
> - }
> - XkbFreeKeyboard(desc, XkbGBN_AllComponentsMask, True);
> - }
>
> if (check_for_xwin(dpy)) {
> VNC_DEBUG("Using xwin keycode mapping");
> @@ -195,26 +182,14 @@ const guint16 *vnc_display_keymap_gdk2xtkbd_table(GdkWindow *window,
> VNC_DEBUG("Using xquartz keycode mapping");
> *maplen = G_N_ELEMENTS(keymap_xorgxquartz2xtkbd);
> return keymap_xorgxquartz2xtkbd;
> - } else if (keycodes && STRPREFIX(keycodes, "evdev")) {
> - VNC_DEBUG("Using evdev keycode mapping");
> - *maplen = G_N_ELEMENTS(keymap_xorgevdev2xtkbd);
> - return keymap_xorgevdev2xtkbd;
> - } else if (keycodes && STRPREFIX(keycodes, "xfree86")) {
> + } else if (XKeysymToKeycode(xdisplay, XK_Page_Up) == 0x63) {
> VNC_DEBUG("Using xfree86 keycode mapping");
> *maplen = G_N_ELEMENTS(keymap_xorgkbd2xtkbd);
> return keymap_xorgkbd2xtkbd;
> } else {
> - g_warning("Unknown keycode mapping '%s'.\n"
> - "Please report to gtk-vnc-list@xxxxxxxxx\n"
> - "including the following information:\n"
> - "\n"
> - " - Operating system\n"
> - " - GDK build\n"
> - " - X11 Server\n"
> - " - xprop -root\n"
> - " - xdpyinfo\n",
> - keycodes);
> - return NULL;
> + VNC_DEBUG("Using evdev keycode mapping");
> + *maplen = G_N_ELEMENTS(keymap_xorgevdev2xtkbd);
> + return keymap_xorgevdev2xtkbd;
else is evdev is not valid - that is only true of an X server running
on a Linux host. Thus this has the effect of hiding the important
error message alerting us to use of other X servers we need to handle
and silently giving those users incorrect keymaps.
This is what was suggested in downstream bug https://bugzilla.redhat.com/show_bug.cgi?id=1479682
If I may, I would point out at https://bugzilla.redhat.com/show_bug.cgi?id=1479682#c32 where you wrote:
“Oh, that's a nice idea, we can certainly try that approach. Just distinguishing AT from evdev is sufficient in this case.”
“Oh, that's a nice idea, we can certainly try that approach. Just distinguishing AT from evdev is sufficient in this case.”
Besides, the current code is not only broken, it also leaves the user with no working keyboard at all...
Cheers,
Olivier
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel