File gnome-settings-daemon-bnc467558-bgo545115-randr... of Package gnome-settings-daemon

Overview Repositories Revisions Requests Users Attributes Meta

File gnome-settings-daemon-bnc467558-bgo545115-randr-confirmation.diff of Package gnome-settings-daemon (Revision 0ae41db67e0e58f908c289ed6c9cc65f)

Currently displaying revision 0ae41db67e0e58f908c289ed6c9cc65f , Show latest

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,

Places

File gnome-settings-daemon-bnc467558-bgo545115-randr-confirmation.diff of Package gnome-settings-daemon (Revision 0ae41db67e0e58f908c289ed6c9cc65f)

Places