Re: startup-notification support for nautilus as a launchee

[Elijah Newren <newren gmail com>]

On Mon, 14 Feb 2005 12:07:03 +0100, Alexander Larsson <alexl redhat com> wrote:

> Since this is clearly better than what we currently do, i committed
> this. I removed the leak FIXME and freed startup_id in the
> NautilusWindow finalizer, and I fixed up the coding style/indentation to
> match Nautilus coding style. Also, I removed the g_warning on launching
> without a startup id. This was triggering every time you launch nautilus
> from a terminal, which I think is a bit harsh. Do other gnome apps do
> this too?

Most gnome apps don't decide to forward a request to open a new window
to previous instances, meaning that gtk+ can handle everything for
them and not getting a startup_id is no big deal.  nautilus,
gnome-terminal, epiphany, etc. are different.  Of those apps, I'm only
familiar with the gnome-terminal code, and yes, it has such a warning.
 But then again, I was the one that put it there so you can't trust
that case too much for any meaning.  ;-)  The main reason I have it
there is you'll notice that any nautilus window launched from a
terminal does not get focus and is stacked lower than the focus
window.

As Crispin pointed out, though, perhaps an ugly hack is in order--if
no startup-id is provided then ping the xserver for a timestamp
(gdk_x11_get_server_time) and use that for the new window instead of
spewing a warning.  It'd still partially break
focus-stealing-prevention (since the timestamp would be too late), but
given that nautilus windows (hopefully) are only ever launched due to
user interaction, it may be better than the alternative.  I can code
it up if you want, it'd only be a few lines.

> Some questions remain:
> 
> on existing window presenting you do this:
> 
> end_startup_notification (GTK_WIDGET (existing_window),
>                           existing_window->details->startup_id,
>                           startup_id);
> 
> Is this really right? I'm not totally sure how startup notification
> works, but doesn't this re-trigger the startup_id that was originally
> used to start nautilus, instead of the new startup (startup_id) that was
> requested?

No, this isn't right.  It's totally bogus for the reason you point out
and causes one of the two problems Dennis pointed out when he tested
the patch (see bug 166242).  New patch attached, which also includes a
fair amount of cleaning up that fixing this problem allows.

> @@ -345,10 +351,12 @@ restore_one_window_callback (const char
>         if (eel_strlen (location) > 0) {
>                 window = nautilus_application_present_spatial_window (shell->details->application,
>                                                                       NULL,
> +                                                                     NULL, /* FIXME: Need startup_id? */
>                                                                       location,
>                                                                       screen);
>         } else {
>                 window = nautilus_application_create_navigation_window (shell->details->application,
> +                                                                       NULL, /* FIXME: Need startup_id? */
>                                                                         screen);
>                 nautilus_window_go_home (window);
>         }
> 
> Does the session launch its processes using startup notification?

The FIXMEs were there because I had no clue under what circumtances
those lines were called.  I'm pretty sure that the session doesn't
launch its processes with startup-notification, so I think those
comments can just be removed (which I also did in the attached patch)

Elijah

Index: src/nautilus-application.c
===================================================================
RCS file: /cvs/gnome/nautilus/src/nautilus-application.c,v
retrieving revision 1.228
diff -p -u -r1.228 nautilus-application.c
--- src/nautilus-application.c	14 Feb 2005 10:59:23 -0000	1.228
+++ src/nautilus-application.c	14 Feb 2005 23:24:39 -0000
@@ -1002,51 +1002,16 @@ sn_error_trap_pop (SnDisplay *display,
 	gdk_error_trap_pop ();
 }
 
-static gboolean
-id_string_has_timestamp (const char *startup_id_string)
-{
-	char * time_str;
-	
-	time_str = g_strrstr (startup_id_string, "_TIME");
-	
-	return time_str != NULL;
-}
-
-static guint32
-get_timestamp_from_id_string (const char *startup_id_string)
-{
-	char   *time_str;
-	gchar  *end;
-	gulong  retval;
-
-	retval = 0;
-	time_str = g_strrstr (startup_id_string, "_TIME");
-	g_assert (time_str != NULL);
-
-	errno = 0;
-
-	/* Skip past the "_TIME" part */
-	time_str += 5;
-
-	retval = strtoul (time_str, &end, 0);
-	if (end == time_str || errno != 0) {
-		g_warning ("startup_id is messed up\n");
-	}
-
-	return retval;
-}
-
 static void
 end_startup_notification (GtkWidget  *widget, 
-			  const char *startup_id_to_end,
-			  const char *startup_id_with_timestamp)
+			  const char *startup_id)
 {
 	SnDisplay *sn_display;
 	SnLauncheeContext *context;
 	GdkDisplay *display;
 	GdkScreen  *screen;
 
-	g_return_if_fail (startup_id_to_end != NULL);
+	g_return_if_fail (startup_id != NULL);
   
 	if (!GTK_WIDGET_REALIZED (widget)) {
 		gtk_widget_realize (widget);
@@ -1069,20 +1034,17 @@ end_startup_notification (GtkWidget  *wi
       
 	context = sn_launchee_context_new (sn_display,
 					   gdk_screen_get_number (screen),
-					   startup_id_to_end);
+					   startup_id);
 
-	if (startup_id_with_timestamp == NULL) {
-		sn_launchee_context_setup_window (context,
-						  GDK_WINDOW_XWINDOW (widget->window));
-		startup_id_with_timestamp = startup_id_to_end;
-	}
+	sn_launchee_context_setup_window (context,
+					  GDK_WINDOW_XWINDOW (widget->window));
 
 	/* Now, set the _NET_WM_USER_TIME for the new window to the timestamp
 	 * that caused the window to be launched.
 	 */
-	if (id_string_has_timestamp (startup_id_with_timestamp)) {
+	if (sn_launchee_context_get_id_has_timestamp (context)) {
 		gulong startup_id_timestamp;
-		startup_id_timestamp = get_timestamp_from_id_string (startup_id_with_timestamp);
+		startup_id_timestamp = sn_launchee_context_get_timestamp (context);
 		gdk_x11_window_set_user_time (widget->window, startup_id_timestamp);
 	} else {
 		/* Comment this out for now, as it warns way to often.
@@ -1126,8 +1088,8 @@ nautilus_application_present_spatial_win
 
 		if (eel_uris_match (existing_location, location)) {
 			end_startup_notification (GTK_WIDGET (existing_window),
-						  existing_window->details->startup_id,
 						  startup_id);
+
 			gtk_window_present (GTK_WINDOW (existing_window));
 			if (new_selection) {
 				nautilus_view_set_selection (existing_window->content_view, new_selection);
@@ -1138,8 +1100,7 @@ nautilus_application_present_spatial_win
 
 	window = create_window (application, NAUTILUS_TYPE_SPATIAL_WINDOW, startup_id, screen);
 	end_startup_notification (GTK_WIDGET (window),
-				  startup_id,
-				  NULL);
+				  startup_id);
 	if (requesting_window) {
 		/* Center the window over the requesting window by default */
 		int orig_x, orig_y, orig_width, orig_height;
@@ -1183,8 +1144,7 @@ nautilus_application_create_navigation_w
 	
 	window = create_window (application, NAUTILUS_TYPE_NAVIGATION_WINDOW, startup_id, screen);
 	end_startup_notification (GTK_WIDGET (window),
-				  startup_id,
-				  NULL);
+				  startup_id);
 
 	return window;
 }
Index: src/nautilus-shell.c
===================================================================
RCS file: /cvs/gnome/nautilus/src/nautilus-shell.c,v
retrieving revision 1.59
diff -p -u -r1.59 nautilus-shell.c
--- src/nautilus-shell.c	14 Feb 2005 10:59:23 -0000	1.59
+++ src/nautilus-shell.c	14 Feb 2005 23:24:39 -0000
@@ -351,12 +351,12 @@ restore_one_window_callback (const char 
 	if (eel_strlen (location) > 0) {
 		window = nautilus_application_present_spatial_window (shell->details->application, 
 								      NULL,
-								      NULL, /* FIXME: Need startup_id? */
+								      NULL,
 								      location,
 								      screen);
 	} else {
 		window = nautilus_application_create_navigation_window (shell->details->application,
-									NULL, /* FIXME: Need startup_id? */
+									NULL,
 									screen);
 		nautilus_window_go_home (window);
 	}