Package: xfdesktop4
Version: 4.20.1-1
* Intro
If the user runs the Xfce desktop remotely (example, using VNC) and the PC
doesn't have a monitor connected, and the user runs the Xfce Desktop Settings
app (Applications -> Settings -> Desktop), the app shows an error message box:
"Unable to load images from folder "(null)". Operation was cancelled".
Actually, there are TWO error message boxes, stacked on top of each other. You
can move the top one aside and see the 2nd box.
* Steps to reproduce
* Requirements
- Another PC with a VNC client installed.
* Setup (VNC server)
- Install a VNC server: sudo apt-get install x11vnc
- Manually run it in a console: x11vnc -display :0 -forever
- Connect the 2nd PC's VNC client to the test PC.
* Reproduce the bug
- Unplug all video cables from the PC.
- Open a console and run "xrandr" to confirm that no monitors are connected.
- Launch the Xfce Desktop settings app: Applications -> Settings -> Desktop
* Analysis
- The error message is displayed in xfdesktop_image_list_add_dir(), which is
an asynchronous callback function triggered by
g_file_enumerate_children_async(), used to get info about files in a directory.
What's happening is that when there's no monitor connected,
g_file_enumerate_children_async() is called 3 times. I suspect that each
subsequent request must be cancelling out the previous asynchronous request,
causing the error "Operation was cancelled".
But when there IS a monitor connected, my debugging shows that
g_file_enumerate_children_async() is called only ONCE. So there's no overlapping
async request, no errors.
- The root cause for why there are 3 async requests when monitorless is in
cb_update_background_tab(). During the app initialization, this function is
called multiple times, and there's logic to prevent it from doing the same thing
multiple times. Here's a condensed version of the code:
gchar *monitor_name =
xfdesktop_get_monitor_name_from_gtk_widget(background_settings->image_iconview,
monitor_num);
/* Most of the time we won't change monitor, screen, or workspace so try
* to bail out now if we can */
/* Check to see if we haven't changed workspace or screen */
if (background_settings->workspace == workspace_num &&
background_settings->screen == screen_num) {
/* do we support monitor names? */
if(background_settings->monitor_name != NULL || monitor_name != NULL)
{
/* Check if we haven't changed monitors (by name) */
if(g_strcmp0(background_settings->monitor_name, monitor_name) == 0) {
g_free(monitor_name);
return;
}
}
}
TRACE("screen, monitor, or workspace changed");
background_settings->monitor_name =
xfdesktop_get_monitor_name_from_gtk_widget(background_settings->image_iconview,
monitor_num);
<The code logic here eventually triggers the async request
g_file_enumerate_children_async().>
The code first gets the current monitor name. Example from my debugging:
"VGA-0". The code then checks if the monitor has changed. If it has NOT changed,
the code returns early. When there's a monitor, this works correctly, so the
only time this code proceeds ahead is the first time, when
"background_settings->monitor_name" is NULL and when the code gets the monitor name.
The problem is that when there is NO monitor,
xfdesktop_get_monitor_name_from_gtk_widget() returns a NULL monitor name. Since
there's no monitor name, there's no way to check if the monitor name has
changed. So the code proceeds ahead to the async request. And since
cb_update_background_tab() is called several times during initialization, this
code logic triggers several overlapping async requests.
* Proposed solution
- I don't have a good proposed solution, only a couple of failed solutions.
The problem is that when "background_settings->monitor_name" is NULL,
there's no way to know if this NULL means "uninitialized - We haven't checked
what monitors are connected", or if it means "We know there's no monitor connected".
- The first bad solution is to return early if both monitor names are NULL.
But the problem is that the code NEVER proceeds ahead to do the async call, so
the GUI never initializes the available background images.
if(background_settings->monitor_name == NULL && monitor_name == NULL) {
g_free(monitor_name);
return;
}
- The second solution was to change
xfdesktop_get_monitor_name_from_gtk_widget() from returning NULL to returning an
empty string. In this solution, when there's no monitor,
"background_settings->monitor_name" is set to an empty string, which clearly
indicates that there's currently no monitor.
gchar*
xfdesktop_get_monitor_name_from_gtk_widget(GtkWidget *widget, gint
monitor_num)
{
<...>
monitor = gdk_display_get_monitor(display, monitor_num);
const gchar *monitor_model = gdk_monitor_get_model(monitor);
return g_strdup(monitor_model != NULL ? monitor_model : "");
}
This seems like a better solution. But my concern is that
"background_settings->monitor_name" is intended to be NULL or have a valid
monitor name. I don't know what the side effects are of having an empty
monitor_name string.
* Additional info
- The bug does not occur in Debian 12 Bookworm, xfdesktop4, 4.18.1-1. That's
because the code has been significantly rewritten.
