GNOME Bugzilla – Bug 777800
backends/x11/nested: Don't interfere with host XKB state
Last modified: 2017-03-15 21:08:28 UTC
See attached patch.
Created attachment 344334 [details] [review] backends/x11/nested: Don't interfere with host XKB state Don't lock the XKB layout group when running nested, as that will interfere with the host X11 server state, as the X11 connection in the backend is the one of the host. This fixes an issue where mutter incorrectly changed the keyboard layout of Xwayland clients under Wayland. See https://bugs.freedesktop.org/show_bug.cgi?id=93237 for details.
Review of attachment 344334 [details] [review]: Heh, nested... so, I guess apply_keymap() should also be skipped in nested mode
Created attachment 344835 [details] [review] backends/x11: Split up X11 backend into Cm and Nested Split up the X11 backend into two parts, one for running as a Compositing Manager, and one for running as a nested Wayland compositor. ---- I went with this approach. Could as well just add more and more if (mode) blocks here and there, but thought a real backend split might be better.
Created attachment 344836 [details] [review] backends/x11: Only apply keymap when not running nested
Review of attachment 344835 [details] [review]: ::: src/backends/x11/meta-backend-x11.c @@ +103,3 @@ * and never under nested, as under nested all backend events * should be reported with respect to the stage window. */ + g_assert (META_IS_BACKEND_X11_CM (x11)); this could be a vfunc as well and we'd avoid having the parent class know about the child class ::: src/backends/x11/nested/meta-backend-x11-nested.c @@ +117,3 @@ + if (event->xfocus.window == xwin) + { +#ifdef HAVE_WAYLAND might as well put all but the return sentence inside the #ifdef ::: src/core/main.c @@ +427,3 @@ + else +#endif /* HAVE_WAYLAND */ + *backend_gtype = META_TYPE_BACKEND_X11_CM; if opt_nested this should still be _NESTED I think. the logic here is getting p complex
Review of attachment 344836 [details] [review]: ::: src/backends/x11/meta-backend-x11.c @@ -35,2 @@ #include <X11/extensions/sync.h> #include <X11/XKBlib.h> I believe XKBlib.h isn't needed here anymore ::: src/backends/x11/nested/meta-backend-x11-nested.c @@ +143,3 @@ + const char *options) +{ + g_warning ("Tried to set keymap when running nested.\n"); not warning worthy IMO @@ +161,3 @@ backend_class->update_screen_size = meta_backend_x11_nested_update_screen_size; backend_class->select_stage_events = meta_backend_x11_nested_select_stage_events; + backend_class->set_keymap = meta_backend_x11_nested_set_keymap; no lock_layout_group vfunc?
Created attachment 345080 [details] [review] backends/x11: Split up X11 backend into Cm and Nested Split up the X11 backend into two parts, one for running as a Compositing Manager, and one for running as a nested Wayland compositor. This commit also cleans up the compositor configuration calculation, attempting to make it more approachable.
Created attachment 345081 [details] [review] backends/x11: Only apply keymap when not running nested
Review of attachment 345081 [details] [review]: looks fine
Review of attachment 345080 [details] [review]: looks good ::: src/backends/x11/nested/meta-backend-x11-nested.c @@ +26,3 @@ +#include "backends/x11/nested/meta-cursor-renderer-x11-nested.h" + +#include "wayland/meta-wayland.h" #ifdef HAVE_WAYLAND ::: src/core/main.c @@ +402,3 @@ +/* + * Determine the compositor configuration, i.e. whether to run as a Wayland + * compositor or an X11 window manager, as well as what backend to use. nitpick: s/or an X11 window manager// - there's no way we are not an X11 window manager, but we may or may not be a wayland compositor
Attachment 345080 [details] pushed as 6d64123 - backends/x11: Split up X11 backend into Cm and Nested Attachment 345081 [details] pushed as bd2ca79 - backends/x11: Only apply keymap when not running nested