File gnome-settings-daemon-bnc467558-bgo545115-randr-confirmation.diff of Package gnome-settings-daemon
diff --git a/ChangeLog b/ChangeLog
index 4423ee2..ec6af64 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,78 @@
+2009-01-27 Federico Mena Quintero <federico@novell.com>
+
+ http://bugzilla.gnome.org/show_bug.cgi?id=545115 - Ask for
+ confirmation, with a timeout, after changing the RANDR
+ configuration for if we leave the user with an unusable display.
+ This also handles the case where the machine may crash after
+ changing the configuration; the old/known-good configuration will
+ be restored when the user restarts his session.
+
+ Refactor:
+
+ * plugins/xrandr/gsd-xrandr-manager.c
+ (apply_stored_configuration_at_startup): Factor out the logic to
+ apply the stored configuration at startup.
+ (gsd_xrandr_manager_start): Use the function above.
+
+ During startup, restore the backup configuration if it existed, to
+ recover from the case when the machine crashes while applying an
+ intended configuration.
+
+ * plugins/xrandr/gsd-xrandr-manager.c
+ (apply_stored_configuration_at_startup): First see if we have a
+ backup configuration; if so, it means the machine or g-s-d crashed
+ while changing the RANDR parameters. If there is no backup
+ configuration, then we have a known-good configuration which we
+ can use.
+ (apply_intended_configuration): New function, used to load the
+ intended configuration (i.e. the non-backup one).
+ (restore_backup_configuration): Utility function to overwrite the
+ known-bad configuration with the known-good backup one.
+
+ Use a timeout-confirmation dialog after changing the display
+ configuration:
+
+ * plugins/xrandr/gsd-xrandr-manager.c
+ (try_to_apply_intended_configuration): New function; applies the
+ intended configuration, restores the backup configuration if that
+ fails, or asks the user to confirm if the intended configuration
+ is usable.
+ (gsd_xrandr_manager_apply_configuration): Use
+ try_to_apply_intended_configuration() in the implementation of the
+ D-Bus method to apply RANDR configurations. This way all apps
+ which use this D-Bus method will get confirmation for free.
+ (output_rotation_item_activate_cb): Use
+ try_to_apply_intended_configuration() so that the RANDR tray-icon
+ also uses the confirmation/backup logic.
+ (restore_backup_configuration): Restore the screen configuration
+ itself in addition to restoring the file on disk from the backup.
+ (user_says_things_are_ok): New utility function to handle a
+ timeout-confirmation dialog.
+
+ Fix error reporting at startup:
+
+ * plugins/xrandr/gsd-xrandr-manager.c (error_message): Handle the
+ case where the status_icon is not created yet; this happens during
+ startup or when the status_icon is disabled by the user.
+
+ Handle the case where there is no matching configuration at
+ startup; this is not an error:
+
+ * plugins/xrandr/gsd-xrandr-manager.c
+ (apply_intended_configuration): "no matching configuration" is not
+ an error when looking for a suitable configuration in
+ monitors.xml; it simply means that the user has a different set of
+ monitors than the ones that are available in that file.
+
+2009-01-24 Jens Granseuer <jensgr@gmx.net>
+
+ Patch by: Andres Freund <andres@anarazel.de>
+
+ Fix possible crash when pressing Fn-F7 (bug #568713)
+
+ * plugins/xrandr/gsd-xrandr-manager.c: (handle_fn_f7): only try to
+ dereference the error when it was actually set
+
2009-01-14 Federico Mena Quintero <federico@novell.com>
* plugins/xrandr/gsd-xrandr-manager.c (gsd_xrandr_manager_start):
diff --git a/plugins/xrandr/gsd-xrandr-manager.c b/plugins/xrandr/gsd-xrandr-manager.c
index fcd70c1..d2eba18 100644
--- a/plugins/xrandr/gsd-xrandr-manager.c
+++ b/plugins/xrandr/gsd-xrandr-manager.c
@@ -103,20 +103,183 @@ static void gsd_xrandr_manager_class_init (GsdXrandrManagerClass *klass);
static void gsd_xrandr_manager_init (GsdXrandrManager *xrandr_manager);
static void gsd_xrandr_manager_finalize (GObject *object);
+static void error_message (GsdXrandrManager *mgr, const char *primary_text, GError *error_to_display, const char *secondary_text);
+
G_DEFINE_TYPE (GsdXrandrManager, gsd_xrandr_manager, G_TYPE_OBJECT)
static gpointer manager_object = NULL;
+static void
+restore_backup_configuration_without_messages (const char *backup_filename, const char *intended_filename)
+{
+ backup_filename = gnome_rr_config_get_backup_filename ();
+ rename (backup_filename, intended_filename);
+}
+
+static void
+restore_backup_configuration (GsdXrandrManager *manager, const char *backup_filename, const char *intended_filename)
+{
+ struct GsdXrandrManagerPrivate *priv = manager->priv;
+ int saved_errno;
+ char *msg;
+
+ if (rename (backup_filename, intended_filename) == 0) {
+ GError *error;
+
+ error = NULL;
+ if (!gnome_rr_config_apply_from_filename (priv->rw_screen, intended_filename, &error)) {
+ error_message (manager, _("Could not restore the display's configuration"), error, NULL);
+
+ if (error)
+ g_error_free (error);
+ }
+
+ return;
+ }
+
+ saved_errno = errno;
+
+ msg = g_strdup_printf ("Could not rename %s to %s: %s",
+ backup_filename, intended_filename,
+ g_strerror (saved_errno));
+ error_message (manager,
+ _("Could not restore the display's configuration from a backup"),
+ NULL,
+ msg);
+ g_free (msg);
+
+ unlink (backup_filename);
+}
+
+typedef struct {
+ GsdXrandrManager *manager;
+ GtkWidget *dialog;
+
+ int countdown;
+ int response_id;
+} TimeoutDialog;
+
+static void
+print_countdown_text (TimeoutDialog *timeout)
+{
+ gtk_message_dialog_format_secondary_text (GTK_MESSAGE_DIALOG (timeout->dialog),
+ _("The display will be reset to its previous configuration in %d seconds"),
+ timeout->countdown);
+}
+
+static gboolean
+timeout_cb (gpointer data)
+{
+ TimeoutDialog *timeout = data;
+
+ timeout->countdown--;
+
+ if (timeout->countdown == 0) {
+ timeout->response_id = GTK_RESPONSE_CANCEL;
+ gtk_main_quit ();
+ } else {
+ print_countdown_text (timeout);
+ }
+
+ return TRUE;
+}
+
+static void
+timeout_response_cb (GtkDialog *dialog, int response_id, gpointer data)
+{
+ TimeoutDialog *timeout = data;
+
+ if (response_id == GTK_RESPONSE_DELETE_EVENT) {
+ /* The user closed the dialog, presumably saying, "go away, everything is fine" */
+ timeout->response_id = GTK_RESPONSE_ACCEPT;
+ } else
+ timeout->response_id = response_id;
+
+ gtk_main_quit ();
+}
+
+static gboolean
+user_says_things_are_ok (GsdXrandrManager *manager)
+{
+ TimeoutDialog timeout;
+ guint timeout_id;
+
+ timeout.manager = manager;
+
+ /* It sucks that we don't know the parent window here */
+ timeout.dialog = gtk_message_dialog_new (NULL,
+ GTK_DIALOG_MODAL,
+ GTK_MESSAGE_QUESTION,
+ GTK_BUTTONS_NONE,
+ _("Does the display look OK?"));
+ timeout.countdown = 10;
+ print_countdown_text (&timeout);
+
+ gtk_dialog_add_button (GTK_DIALOG (timeout.dialog), _("Restore the previous configuration"), GTK_RESPONSE_CANCEL);
+ gtk_dialog_add_button (GTK_DIALOG (timeout.dialog), _("Keep this configuration"), GTK_RESPONSE_ACCEPT);
+ gtk_dialog_set_default_response (GTK_DIALOG (timeout.dialog), GTK_RESPONSE_ACCEPT); /* ah, the optimism */
+
+ g_signal_connect (timeout.dialog, "response",
+ G_CALLBACK (timeout_response_cb),
+ &timeout);
+
+ gtk_widget_show_all (timeout.dialog);
+ timeout_id = g_timeout_add (1000,
+ timeout_cb,
+ &timeout);
+ gtk_main ();
+
+ gtk_widget_destroy (timeout.dialog);
+ g_source_remove (timeout_id);
+
+ if (timeout.response_id == GTK_RESPONSE_ACCEPT)
+ return TRUE;
+ else
+ return FALSE;
+}
+
+static gboolean
+try_to_apply_intended_configuration (GsdXrandrManager *manager, GError **error)
+{
+ struct GsdXrandrManagerPrivate *priv = manager->priv;
+ char *backup_filename;
+ char *intended_filename;
+ gboolean result;
+
+ /* Try to apply the intended configuration */
+
+ backup_filename = gnome_rr_config_get_backup_filename ();
+ intended_filename = gnome_rr_config_get_intended_filename ();
+
+ result = gnome_rr_config_apply_from_filename (priv->rw_screen, intended_filename, error);
+ if (!result) {
+ error_message (manager, _("The selected configuration for displays could not be applied"), error ? *error : NULL, NULL);
+ restore_backup_configuration_without_messages (backup_filename, intended_filename);
+ goto out;
+ }
+
+ /* Confirm with the user */
+
+ if (user_says_things_are_ok (manager))
+ unlink (backup_filename);
+ else
+ restore_backup_configuration (manager, backup_filename, intended_filename);
+
+out:
+ g_free (backup_filename);
+ g_free (intended_filename);
+
+ return result;
+}
/* DBus method; see gsd-xrandr-manager.xml for the interface definition */
static gboolean
gsd_xrandr_manager_apply_configuration (GsdXrandrManager *manager,
GError **error)
{
- GnomeRRScreen *screen = manager->priv->rw_screen;
-
- return gnome_rr_config_apply_stored (screen, error);
+ return try_to_apply_intended_configuration (manager, error);
}
+
/* We include this after the definition of gsd_xrandr_manager_apply_configuration() so the prototype will already exist */
#include "gsd-xrandr-manager-glue.h"
@@ -485,10 +648,17 @@ error_message (GsdXrandrManager *mgr, const char *primary_text, GError *error_to
g_assert (error_to_display == NULL || secondary_text == NULL);
- notification = notify_notification_new_with_status_icon (primary_text,
- error_to_display ? error_to_display->message : secondary_text,
- GSD_XRANDR_ICON_NAME,
- priv->status_icon);
+ if (priv->status_icon)
+ notification = notify_notification_new_with_status_icon (primary_text,
+ error_to_display ? error_to_display->message : secondary_text,
+ GSD_XRANDR_ICON_NAME,
+ priv->status_icon);
+ else
+ notification = notify_notification_new (primary_text,
+ error_to_display ? error_to_display->message : secondary_text,
+ GSD_XRANDR_ICON_NAME,
+ NULL);
+
notify_notification_show (notification, NULL); /* NULL-GError */
#else
GtkWidget *dialog;
@@ -526,7 +696,7 @@ handle_fn_f7 (GsdXrandrManager *mgr)
g_debug ("Handling fn-f7");
error = NULL;
- if (!gnome_rr_screen_refresh (screen, &error)) {
+ if (!gnome_rr_screen_refresh (screen, &error) && error) {
char *str;
str = g_strdup_printf (_("Could not refresh the screen information: %s"), error->message);
@@ -896,13 +1066,7 @@ output_rotation_item_activate_cb (GtkCheckMenuItem *item, gpointer data)
return;
}
- if (!gnome_rr_config_apply_stored (priv->rw_screen, &error)) {
- error_message (manager, _("The selected rotation could not be applied"), error, NULL);
- if (error)
- g_error_free (error);
-
- return;
- }
+ try_to_apply_intended_configuration (manager, NULL); /* NULL-GError */
}
static void
@@ -1121,12 +1285,86 @@ on_config_changed (GConfClient *client,
start_or_stop_icon (manager);
}
+static void
+apply_intended_configuration (GsdXrandrManager *manager, const char *intended_filename)
+{
+ GError *my_error;
+
+ my_error = NULL;
+ if (!gnome_rr_config_apply_from_filename (manager->priv->rw_screen, intended_filename, &my_error)) {
+ if (my_error) {
+ if (g_error_matches (my_error, GNOME_RR_ERROR, GNOME_RR_ERROR_NO_MATCHING_CONFIG)) {
+ /* This is not an error; the user probably
+ * changed his monitors and then logged in
+ * again, thus restarting gnome-settings-daemon.
+ */
+ } else if (!g_error_matches (my_error, G_FILE_ERROR, G_FILE_ERROR_NOENT))
+ error_message (manager, _("Could not apply the stored configuration for monitors"), my_error, NULL);
+
+ g_error_free (my_error);
+ }
+ }
+}
+
+static void
+apply_stored_configuration_at_startup (GsdXrandrManager *manager)
+{
+ GError *my_error;
+ gboolean success;
+ char *backup_filename;
+ char *intended_filename;
+
+ backup_filename = gnome_rr_config_get_backup_filename ();
+ intended_filename = gnome_rr_config_get_intended_filename ();
+
+ /* 1. See if there was a "saved" configuration. If there is one, it means
+ * that the user had selected to change the display configuration, but the
+ * machine crashed. In that case, we'll apply *that* configuration and save it on top of the
+ * "intended" one.
+ */
+
+ my_error = NULL;
+
+ success = gnome_rr_config_apply_from_filename (manager->priv->rw_screen, backup_filename, &my_error);
+ if (success) {
+ /* The backup configuration existed, and could be applied
+ * successfully, so we must restore it on top of the
+ * failed/intended one.
+ */
+ restore_backup_configuration (manager, backup_filename, intended_filename);
+ goto out;
+ }
+
+ if (!g_error_matches (my_error, G_FILE_ERROR, G_FILE_ERROR_NOENT)) {
+ /* Epic fail: there (probably) was a backup configuration, but
+ * we could not apply it. The only thing we can do is delete
+ * the backup configuration. Let's hope that the user doesn't
+ * get left with an unusable display...
+ */
+
+ unlink (backup_filename);
+ goto out;
+ }
+
+ /* 2. There was no backup configuration! This means we are
+ * good. Apply the intended configuration instead.
+ */
+
+ apply_intended_configuration (manager, intended_filename);
+
+out:
+
+ if (my_error)
+ g_error_free (my_error);
+
+ g_free (backup_filename);
+ g_free (intended_filename);
+}
+
gboolean
gsd_xrandr_manager_start (GsdXrandrManager *manager,
GError **error)
{
- GError *my_error;
-
g_debug ("Starting xrandr manager");
manager->priv->rw_screen = gnome_rr_screen_new (
@@ -1162,15 +1400,7 @@ gsd_xrandr_manager_start (GsdXrandrManager *manager,
gdk_error_trap_pop ();
}
- my_error = NULL;
- if (!gnome_rr_config_apply_stored (manager->priv->rw_screen, &my_error)) {
- if (my_error) {
- if (!g_error_matches (my_error, G_FILE_ERROR, G_FILE_ERROR_NOENT))
- error_message (manager, _("Could not apply the stored configuration for monitors"), my_error, NULL);
-
- g_error_free (my_error);
- }
- }
+ apply_stored_configuration_at_startup (manager);
gdk_window_add_filter (gdk_get_default_root_window(),
(GdkFilterFunc)event_filter,
