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