GNOME Bugzilla – Bug 746286
nautilus-application: don't create desktop on Wayland
Last modified: 2016-12-05 14:27:56 UTC
Se patch
Created attachment 299510 [details] [review] nautilus-application: don't create desktop on Wayland We were doing some calls to the x11 backend, which are not supported on Wayland and the application were crashing when creating the desktop on Wayland. To workaround it, simply don't create the desktop if we are on Wayland. Future work will be to split the main application and the desktop part.
Review of attachment 299510 [details] [review]: ::: src/nautilus-application.c @@ +42,3 @@ #include "nautilus-window.h" #include "nautilus-window-slot.h" +#include <gdk/gdkwayland.h> I think you need an #ifdef GDK_WINDOWING_WAYLAND around this include if you want it to work in wayland-less builds. @@ +917,3 @@ + + return; + } I think you should move this before the desktop_override check, otherwise you'll still crash if force-desktop is set ?
Created attachment 299518 [details] [review] nautilus-application: don't create desktop on Wayland We were doing some calls to the x11 backend, which are not supported on Wayland and the application were crashing when creating the desktop on Wayland. To workaround it, simply don't create the desktop if we are on Wayland. Future work will be to split the main application and the desktop part.
> @@ +917,3 @@ > + > + return; > + } > > I think you should move this before the desktop_override check, otherwise > you'll still crash if force-desktop is set ? hm no, it just returns as if x11 is not avilable does. So no harm here...no?
as discussed on irc: it may be better to check whether the default display is x11 instead, to get out of conditional linking against wayland
Created attachment 299524 [details] [review] nautilus-application: don't create desktop on Wayland We were doing some calls to the x11 backend, which are not supported on Wayland and the application were crashing when creating the desktop on Wayland. To workaround it, simply don't create the desktop if we are on Wayland. Future work will be to split the main application and the desktop part.
Review of attachment 299524 [details] [review]: ::: src/nautilus-application.c @@ +905,3 @@ +#ifdef GDK_WINDOWING_X11 + GdkDisplay *display; + gboolean visible; Are variables in the middle of the block are kosher in nautilus coding style ? @@ +912,3 @@ + if (!GDK_IS_X11_DISPLAY (display)) { + if (visible) + g_warning ("Desktop icons not supported on wayland. Desktop not created"); Maybe reword this warning to: Desktop icons only supported on X11. Desktop not created ? @@ +915,3 @@ + + return; + } Same here ? @@ +922,3 @@ +#endif + + g_warning ("Desktop icons not supported on wayland. Desktop not created"); Same here ?
Created attachment 299526 [details] [review] nautilus-application: don't create desktop on Wayland We were doing some calls to the x11 backend, which are not supported on Wayland (or another backend diferent than X11) and the application were crashing when creating the desktop on Wayland. To workaround it, simply don't create the desktop if we are on a different backend than X11. Future work will be to split the main application and the desktop part.
Yeah, I was not sure if IFDEF blocks were special for declaring variables or not. But they are not. Pushed with changes Attachment 299526 [details] pushed as c8f45f2 - nautilus-application: don't create desktop on Wayland
*** Bug 768893 has been marked as a duplicate of this bug. ***