Re: gdk session management support

[raster redhat com]

On 16 Jan, Felix Bellaby scribbled:
->  Hi all,
->  
->  Here is a response to a mail that I sent to Owen Taylor 
->  about three patches needed in gdk in order for it to 
->  implement ICCCM session management correctly.
->  
->  Owen suggested that I open the patches to general discussion 
->  on this list in case anyone else has any comments.
->  
->  I have tested these patches against gnome-session, gnome-smproxy,
->  xsm and smproxy. I do not currently have access to a WM that
->  actually implements the ICCCM (mwm is an example). However, I
->  see no reason to believe that these patches will result in
->  any bugs elsewhere in gtk+ / gnome.

Enlightenment shoudl impliment all of ICCCM and honor whatever ICCCM
hints make any sens to it - if it does not I personally consider it a
bug and it shoudl be fixed.

E also impliments x11R6 session management - as a client AND as a WM
(icewm only impliments as a client - it does not attempt to save window
positions and restore on restart).

->  
->  Patch 1:
->   > > At present, the gdk_leader_window is created on gdk initialisation
->   > > and is, therefore, unique to the X client. The X11R6 ICCCM suggests
->   > > that each X client has a unique window on which to store its
->   > > session management properties. The gnome-client.c code currently 
->   > > uses the gdk_leader_window for this purpose and sets the
->   > > SM_CLIENT_ID property on the window.
->   > > 
->   > > However, the X11R6 ICCCM further requires that the client sets a
->   > > WM_CLIENT_LEADER property on each of its top level windows 
->   > > (i.e. the ones with WM_STATE properties) which provides a
->   > > reference to this unique window so that the WM can find the
->   > > session manager properties that it contains. 
->   > > 
->   > > gdk/gdkwindow:gdk_window_new seems like the logical place 
->   > > to set this property since it is responsible for setting
->   > > the other properties on the top level windows.
->   > > 
->   > > I can not see any advantage in providing additional 
->   > > flexibility to the programmer in how this property is set.
->  
->  Owen replied:
->   > [...] 
->   > I think setting the client leader property to point to 
->   > the GDK group leader should be fine. (Note that these 
->   > _are_ distinct concepts in the ICCCM. But people are
->   > more likely to want multiple groups in a single app 
->   > than multiple clients in a single app. So if we don't
->   > provide the former I don't see any reason to worry about 
->   > the latter).
->  
->  Indeed, the concepts are distinct. gdk already includes the 
->  ability to set multiple groups (gdk_window_set_group). 
->  The gdk_leader_window is only used as the DEFAULT group.
->  
->  Allowing am app to set more than one sm client id would 
->  violate the protocol. Therefore, I can see no reason whatsoever 
->  for allowing the creation of more than one WM_CLIENT_LEADER 
->  window per app.
->  
->  Patch 2:
->   > > In addition, it would be neater if gdk provided a function
->   > > to set and unset the SM_CLIENT_ID on the gdk_leader_window 
->   > > rather than having gnome-client.c do this using the Xlibs.
->   > > This is a FIXME hanging around in gnome_client.c: i.e.
->   > 
->   > You'll notice that the prototype for such a function is
->   > in gdk.h. I put it there, but forget to set the 
->  
->  Ahh... in that case the patch that I am offering is just
->  an implementation of this function.
->  
->  Patch 3:
->   > > gdk currently provides partial support for the X11R5 ICCCM
->   > > session management protocol. I do not think that providing 
->   > > full support for this deprecated protocol is really worth 
->   > > the effort. However, there is a bug in the support currently
->   > > provided that can be fixed easily.
->   > 
->   > Hmmm, since KDE uses X11R5 SM and we want to have at least
->   > a minimal level of cooperation with KDE apps, I think
->   > bugfixes in this area are useful.
->   >  
->   > > The X smproxy program expects to find the WM_COMMAND
->   > > and WM_CLIENT_MACHINE propeties in the same place.
->   > > gdk currently sets these properties on distinct windows:
->   > > the WM_CLIENT_MACHINE property is set on all the top level 
->   > > windows but the WM_COMMAND property goes onto the 
->   > > gdk_leader_window which is not a top level window.
->   > 
->   > Hmmm, I can't find any code that that sets WM_CLIENT_MACHINE
->   > anywhere in GDK.
->  
->  The XmbSetWMProperties function sets the WM_CLIENT_MACHINE
->  property as a side effect. It is used in gdk_window_new.
->  My patch uses this function in gdk_init as well to ensure
->  that the WM_CLIENT_MACHINE is also set on the gdk_leader_window.
->  
->  Any comments ?
->  
->  Felix
->  
->  Unified diffs:
->  --------------
->  --- gdk.c	Sat Jan 16 01:08:23 1999
->  +++ gdk.c~	Sat Jan 16 00:01:52 1999
->  @@ -382,8 +382,9 @@
->         gdk_progclass[0] = toupper (gdk_progclass[0]);
->       }
->     class_hint->res_class = gdk_progclass;
->  -  XSetClassHint(gdk_display, gdk_leader_window, class_hint);
->  -  XSetCommand(gdk_display, gdk_leader_window, argv_orig, argc_orig);
->  +  XmbSetWMProperties (gdk_display, gdk_leader_window,
->  +                      NULL, NULL, argv_orig, argc_orig, 
->  +                      NULL, NULL, class_hint);
->     XFree (class_hint);
->     
->     for (i = 0; i < argc_orig; i++)
->  @@ -871,6 +872,43 @@
->     return_val = DisplayHeightMM (gdk_display, gdk_screen);
->     
->     return return_val;
->  +}
->  +
->  +/*
->  + *--------------------------------------------------------------
->  + * gdk_set_sm_client_id
->  + *
->  + *   Set the SM_CLIENT_ID property on the WM_CLIENT_LEADER window
->  + *   so that the window manager can save our state using the
->  + *   X11R6 ICCCM session management protocol. A NULL value should 
->  + *   be set following disconnection from the session manager to
->  + *   remove the SM_CLIENT_ID property.
->  + *
->  + * Arguments:
->  + * 
->  + *   "sm_client_id" specifies the client id assigned to us by the
->  + *   session manager or NULL to remove the property.
->  + *
->  + * Results:
->  + *
->  + * Side effects:
->  + *
->  + *--------------------------------------------------------------
->  + */
->  +
->  +void
->  +gdk_set_sm_client_id (const gchar* sm_client_id)
->  +{
->  +  if (sm_client_id && strcmp (sm_client_id, ""))
->  +    {
->  +      XChangeProperty (gdk_display, gdk_leader_window,
->  +	   	       gdk_atom_intern ("SM_CLIENT_ID", FALSE),
->  +		       XA_STRING, 8, PropModeReplace,
->  +		       sm_client_id, strlen(sm_client_id));
->  +    }
->  +  else
->  +     XDeleteProperty (gdk_display, gdk_leader_window,
->  +	   	      gdk_atom_intern ("SM_CLIENT_ID", FALSE));
->   }
->   
->   void
->  --------------
->  --- gdkwindow.c	Sat Jan 16 01:08:28 1999
->  +++ gdkwindow.c~	Sat Jan 16 00:03:19 1999
->  @@ -233,6 +233,8 @@
->     gdk_xid_table_insert (&gdk_root_window, &gdk_root_parent);
->   }
->   
->  +static GdkAtom wm_client_leader_atom = GDK_NONE;
->  +
->   GdkWindow*
->   gdk_window_new (GdkWindow     *parent,
->   		GdkWindowAttr *attributes,
->  @@ -448,6 +448,14 @@
->   
->     XSetWMHints (private->xdisplay, private->xwindow, &wm_hints);
->   
->  +  if (!wm_client_leader_atom)
->  +     wm_client_leader_atom = gdk_atom_intern ("WM_CLIENT_LEADER", FALSE);
->  +
->  +  XChangeProperty (private->xdisplay, private->xwindow,
->  +	   	   wm_client_leader_atom,
->  +		   XA_WINDOW, 32, PropModeReplace,
->  +		   (guchar*) &gdk_leader_window, 1);
->  +
->     if (attributes_mask & GDK_WA_TITLE)
->       title = attributes->title;
->     else
->  --------------
->  
->  

-- 
--------------- Codito, ergo sum - "I code, therefore I am" --------------------
raster@rasterman.com       /\___ /\ ___/||\___ ____/|/\___  raster@redhat.com
Carsten Haitzler           | _ //__\\ __||_ __\\ ___|| _ /  Red Hat Advanced
218/21 Conner Drive        || // __ \\_ \ | |   \ _/_|| /   Development Labs
Chapel Hill NC 27514 USA   ||\\\/  \//__/ |_|   /___/||\\   919 547 0012 ext 282
+1 (919) 929 9443, 801 4392   For pure Enlightenment   http://www.rasterman.com/

              \|/ ____ \|/  For those of you unaware. This face here is in fact
	      "@'/ ,. \@"   a Linux Kernel Error Message.
	      /_| \__/ |_\
		 \__U_/