Merge lp:~robert-ancell/indicator-session/lp-861171 into lp:indicator-session/13.04

[Robert Ancell]

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:377
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/38/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=raring,flavor=amd64/38/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=raring,flavor=i386/38/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/38//rebuild/?

review: Approve (continuous-integration)

[Charles Kerr (charlesk)]

The code looks fine, but I'd like to get mpt to weigh in on the UI change...

[Sebastien Bacher (seb128)]

(sorry, changed the status on the wrong one)

[Matthew Paul Thomas (mpt)]

> +/* TRANSLATORS: This button appears on the logout dialog when
> + there are other users logged in. It will do a log out
> + in place of a restart / shutdown. */

That can't be right: there's already a Log Out button in the Log Out dialog. Did you mean "in the Restart and Shut Down dialogs"?

> +static const gchar * restart_other_session = N_("You need to close all sessions before restarting.");
> +static const gchar * shutdown_other_session = N_("You need to close all sessions before shutting down.");

Very few people will understand what "close all sessions" means. And even to those who do, it may seem like misdirection. Because if it it was reworded in plain language -- for example, "Two other accounts are logged in. They need to log out before the computer can restart." -- it would become obvious that Ubuntu is being unhelpful by not letting you just click a button to do that.

> + _("Ok"), GTK_RESPONSE_CANCEL,

Buttons should always use "OK", not "Ok". If you had used a stock GTK_OK button, that mistake wouldn't have happened.

review: Disapprove (design)

[lp:~robert-ancell/indicator-session/lp-861171]

378. By Robert Ancell on 2013-01-08

Merge with trunk

379. By Robert Ancell on 2013-01-08

Fix incorrect translator comment

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:379
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/42/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/42/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=raring,flavor=i386/42/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/42//rebuild/?

review: Needs Fixing (continuous-integration)

[lp:~robert-ancell/indicator-session/lp-861171]

380. By Robert Ancell on 2013-01-08

Update dialogs to match design

381. By Robert Ancell on 2013-01-08

Remove debugging

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:381
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/43/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/43/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=raring,flavor=i386/43/console
Coverity artifacts:
    http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/43//artifact/results/coverity/CID_10894.html
    http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/43//artifact/results/coverity/CID_10896.html
    http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/43//artifact/results/coverity/CID_10925.html
    http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=coverity-raring,flavor=amd64/43//artifact/results/coverity/CID_10927.html

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/43//rebuild/?

review: Needs Fixing (continuous-integration)

[Allan LeSage (allanlesage)]

Robert, sorry that Jenkins' Coverity scans are making a bit of a fuss above--I'll disable for the moment, please carry on :) .

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:381
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/44/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=raring,flavor=amd64/44/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=raring,flavor=i386/44/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/44//rebuild/?

review: Approve (continuous-integration)

[lp:~robert-ancell/indicator-session/lp-861171]

382. By Robert Ancell on 2013-01-11

Remove unused additional reponse handlers

383. By Robert Ancell on 2013-01-11

Call ConsoleKit directly when force shutdown/restart

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:383
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/45/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=raring,flavor=amd64/45/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-ci/./build=pbuilder,distribution=raring,flavor=i386/45/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/45//rebuild/?

review: Approve (continuous-integration)

[James M. Leddy (jm-leddy)]

Hi Mathew, is Robert's redesign any better?

[Matthew Paul Thomas (mpt)]

Sorry, I missed the notification about this. Thanks to Lars Uebernickel for the screenshots: http://imgur.com/z5Ppeq8 http://imgur.com/K3CzMrV

The "Log Out Instead" button has different spacing in the two, but otherwise that looks good. Thanks Robert!

review: Approve (design)

[James M. Leddy (jm-leddy)]

This is neither here nor there, but it looks to me that the reason the buttons are different sizes is because all of the buttons are the same size by default, and "Shut Down Anyway" is longer than "Restart Anyway", so the size of the "Cancel" and "Log Out Instead" buttons are different.

[Robert Ancell (robert-ancell)]

I'm assuming this all needs updating now we have the new logout dialogs?

[Sebastien Bacher (seb128)]

@Robert: the current dialog is still used out of unity (e.g in unity-greeter or gnome classic) and we are pondering reverting the unity dialogs for raring (there are several issues with those), so in any case it would be good to land those. If you are happy with the code I can file the FFe tomorrow

[Mathieu Trudel-Lapierre (cyphermox)]

Let's resubmit this against lp:indicator-session.

review: Needs Resubmitting

Unmerged revisions

383. By Robert Ancell on 2013-01-11

Call ConsoleKit directly when force shutdown/restart

382. By Robert Ancell on 2013-01-11

Remove unused additional reponse handlers

381. By Robert Ancell on 2013-01-08

Remove debugging

380. By Robert Ancell on 2013-01-08

Update dialogs to match design

379. By Robert Ancell on 2013-01-08

Fix incorrect translator comment

378. By Robert Ancell on 2013-01-08

Merge with trunk

377. By Robert Ancell on 2012-11-29

Don't allow shutdown/restart when other sessions are open

=== modified file 'src/dialog.c'
--- src/dialog.c	2012-10-03 19:35:57 +0000
+++ src/dialog.c	2013-01-11 04:02:22 +0000
@@ -64,6 +64,17 @@
 static const gchar * restart_auth = N_("Restart Instead…");
 static const gchar * body_logout_update = N_("Some software updates won’t apply until the computer next restarts.");
 
+/* TRANSLATORS: This button appears on the restart/shutdown dialog when
+   there are other users logged in. It will do a log out
+   in place of a restart / shutdown. */
+static const gchar * other_session = N_("Log Out Instead");
+static const gchar * restart_other_session = N_("Other users are logged in. Restarting the computer will log them out without warning.");
+static const gchar * restart_greeter_other_session = N_("Other users are logged in. You must log out these sessions before restarting.");
+static const gchar * shutdown_other_session = N_("Other users are logged in. Shutting down the computer will log them out without warning.");
+static const gchar * shutdown_greeter_other_session = N_("Other users are logged in. You must log out these sessions before shutting down.");
+static const gchar * restart_anyway = N_("Restart Anyway");
+static const gchar * shutdown_anyway = N_("Shut Down Anyway");
+
 static const gchar * icon_strings[LOGOUT_DIALOG_TYPE_CNT] = {
 	/* LOGOUT_DIALOG_LOGOUT, */ 	"system-log-out",
 	/* LOGOUT_DIALOG_RESTART, */	"system-restart",
@@ -164,10 +175,42 @@
 	return allowed;
 }
 
+/* Checks with console kit how many sessions are open */
+static guint
+ck_get_n_sessions (void)
+{
+	gchar **sessions = NULL;
+	guint n_sessions = 0;
+
+	ConsoleKitManager * ck_proxy = console_kit_manager_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM,
+	                                                                           G_DBUS_PROXY_FLAGS_NONE,
+	                                                                           "org.freedesktop.ConsoleKit",
+	                                                                           "/org/freedesktop/ConsoleKit/Manager",
+	                                                                           NULL,
+	                                                                           NULL);
+	if (ck_proxy != NULL)
+	{
+		console_kit_manager_call_get_sessions_sync (ck_proxy, &sessions, NULL, NULL);
+		n_sessions = g_strv_length (sessions);
+
+		g_object_unref (ck_proxy);
+	}
+
+	return n_sessions;
+}
+
+static inline gboolean
+is_greeter_mode (void)
+{
+	return !g_strcmp0 (g_getenv ("INDICATOR_GREETER_MODE"), "1");
+}
+
 LogoutDialog *
 logout_dialog_new (LogoutDialogType type)
 {
-	GtkWidget * image = gtk_image_new_from_icon_name(icon_strings[type], GTK_ICON_SIZE_DIALOG);
+	guint n_sessions = ck_get_n_sessions ();
+
+	GtkWidget * image = gtk_image_new_from_icon_name(n_sessions > 1 ? GTK_STOCK_DIALOG_WARNING : icon_strings[type], GTK_ICON_SIZE_DIALOG);
 	gtk_widget_show(image);
 
 	LogoutDialog * dialog = LOGOUT_DIALOG(g_object_new(LOGOUT_DIALOG_TYPE,
@@ -205,7 +248,31 @@
 		button_text = g_dpgettext2 (NULL, "button auth", button_auth_strings[type]);
 	}
 
-	if (restart_required) {
+	if ((type == LOGOUT_DIALOG_TYPE_RESTART || type == LOGOUT_DIALOG_TYPE_SHUTDOWN) && n_sessions > 1) {
+		if (!is_greeter_mode ()) {
+			if (type == LOGOUT_DIALOG_TYPE_RESTART) {
+				g_object_set(dialog, "text", _(restart_other_session), NULL);
+        	    button_text = _(restart_anyway);
+			} else {
+				g_object_set(dialog, "text", _(shutdown_other_session), NULL);
+	            button_text = _(shutdown_anyway);
+			}
+			gtk_dialog_add_buttons(GTK_DIALOG(dialog),
+			                       _(other_session), GTK_RESPONSE_HELP,
+			                       _("Cancel"), GTK_RESPONSE_CANCEL,
+                                   button_text, GTK_RESPONSE_YES,
+			                       NULL);
+		} else {
+			if (type == LOGOUT_DIALOG_TYPE_RESTART) {
+				g_object_set(dialog, "text", _(restart_greeter_other_session), NULL);
+			} else {
+				g_object_set(dialog, "text", _(shutdown_greeter_other_session), NULL);
+			}
+			gtk_dialog_add_buttons(GTK_DIALOG(dialog),
+			                       GTK_STOCK_OK, GTK_RESPONSE_CANCEL,
+			                       NULL);
+		}
+	} else if (restart_required) {
 		const gchar * restart_req;
 		if (allowed) {
 			restart_req = restart_updates;

=== modified file 'src/gtk-logout-helper.c'
--- src/gtk-logout-helper.c	2012-11-29 22:09:57 +0000
+++ src/gtk-logout-helper.c	2013-01-11 04:02:22 +0000
@@ -124,7 +124,7 @@
 }
 
 static void
-session_action (LogoutDialogType action)
+session_action (LogoutDialogType action, gboolean force)
 {
 	GError * error = NULL;
 	GVariant *result = NULL;
@@ -133,11 +133,21 @@
 		g_debug("Asking Session manager to 'Logout'");
 		result = call_gnome_session ("Logout", g_variant_new ("(u)", 1), &error);
 	} else if (action == LOGOUT_DIALOG_TYPE_SHUTDOWN) {
-		g_debug("Asking Session manager to 'RequestShutdown'");
-		result = call_gnome_session ("RequestShutdown", g_variant_new ("()"), &error);
+		if (force) {
+			consolekit_fallback(action);
+			return;
+		} else {
+			g_debug("Asking Session manager to 'RequestShutdown'");
+			result = call_gnome_session ("RequestShutdown", g_variant_new ("()"), &error);
+		}
 	} else if (action == LOGOUT_DIALOG_TYPE_RESTART) {
-		g_debug("Asking Session manager to 'RequestReboot'");
-		result = call_gnome_session ("RequestReboot", g_variant_new ("()"), &error);
+		if (force) {
+			consolekit_fallback(action);
+			return;
+		} else {
+			g_debug("Asking Session manager to 'RequestReboot'");
+			result = call_gnome_session ("RequestReboot", g_variant_new ("()"), &error);
+		}
 	} else {
 		g_warning ("Unknown session action");
 	}
@@ -236,12 +246,15 @@
 		dialog = GTK_WIDGET(logout_dialog_new(type));
 	}
 
+	gboolean force = FALSE;
 	if (dialog != NULL) {
 		GtkResponseType response = gtk_dialog_run(GTK_DIALOG(dialog));
 		gtk_widget_hide(dialog);
 
 		if (response == GTK_RESPONSE_OK) {
 			g_debug("Dialog return response: 'okay'");
+		} else if (response == GTK_RESPONSE_YES) {
+			g_debug("Dialog return response: 'yes'");
 		} else if (response == GTK_RESPONSE_HELP) {
 			g_debug("Dialog return response: 'help'");
 		} else {
@@ -249,17 +262,27 @@
 		}
 
 		if (response == GTK_RESPONSE_HELP) {
-			type = LOGOUT_DIALOG_TYPE_RESTART;
+			if (type == LOGOUT_DIALOG_TYPE_LOG_OUT)
+				type = LOGOUT_DIALOG_TYPE_RESTART;
+			else
+				type = LOGOUT_DIALOG_TYPE_LOG_OUT;
 			response = GTK_RESPONSE_OK;
 		}
 
+		/* If we know we're going to need PolicyKit authorization then call ConsoleKit directly
+		 * rather than using Gnome session - it will just log us out instead and not prompt */
+		if (response == GTK_RESPONSE_YES) {
+			response = GTK_RESPONSE_OK;
+			force = TRUE;
+		}
+      
 		if (response != GTK_RESPONSE_OK) {
 			g_debug("Final response was not okay, quiting");
 			return 0;
 		}
 	}
 
-	session_action(type);
+	session_action(type, force);
 	g_debug("Finished action, quiting");
 
 	return 0;