[Xfce-bugs] [Bug 5377] libxfce4gui session-client bug fixes, improvements, and some optimizations

[bugzilla-daemon at xfce.org]

http://bugzilla.xfce.org/show_bug.cgi?id=5377





--- Comment #5 from Brian J. Tarricone <bjt23 at cornell.edu>  2009-05-20 20:21:15 UTC ---
(In reply to comment #4)
> (In reply to comment #1)
> > (In reply to comment #0)
> 
> > > There was no way to cancel the shutdown from interact. For compatibility,
> > > I extended the session client structure with gboolean interact_2.
> > 
> > You can't do this.  Unfortunately, that struct is public.  If an app is
> > compiled against libxfcegui4 with the smaller structure size, and then
> > libxfcegui4 gets upgraded to a version with a larger structure size, they'll
> > crash.
> 
> Not necessarily; probably only if the session client attempts to access
> interact_2. That is, if an application sets interact style = errors|any (it's
> none by default), but without providing an interact callback, which seems
> unlikely. But I have to move the check/call of interact_2 in interact(SmcConn,
> SmPointer) after the check/call of interact.

Yeah, probably.  I'd hesitate to keep this, but I guess we can just bump
libxfcegui4's soname version.

At any rate, libxfcegui4 is hopefully going away in 4.8, so this may not
matter.  If we refactor this into a decent implementation for libxfce4ui, we
can do whatever we want with the API/ABI.  A change like this cannot go into
the stable 4.6 branch anyway.

> A caller may set a restart command for something so simple as to put the
> currently open file name after the program name, and receive it as argv[1] on
> restart, so a --display may be helpful. OTOH, as you state,

Helpful or not, if the app isn't expecting it...

> > The session client can't assume that the app in question even supports the
> > --display option.
> 
> And so then, should I revert the patch to add --display in client_session_new()
> only, as before, or modify it to never add display?..

It should maintain the current behavior for compatibility.  Always.

> > > client_session_new() initializes both clone and restart commands, so if the
> > > application sets a new restart command, it can not rely on the automatic
> > > restart -> clone copy. Altered client_session_new() to pass NULL, so the
> > > latest restart command is copied to clone.
> > 
> > That sounds reasonable, as long as the code will always use the restart
> > command (minus --sm-client-id) as the clone command if clone is NULL.
> 
> Not exactly. With this patch, if an application sets a restart command, and
> that command contains a --client-id argument, that application should also set
> a clone command without a client argument. Recognizing and removing argument(s)
> from a command explicitly set by the user didn't seem a good idea to me.

Well, you can't really have it both ways.  Currently the session client will
attempt to 'do the right thing', and we can't change behavior for compat
reasons.

Both CloneCommand and RestartCommand are required by the XSMP spec.  If the app
sets one or the other, but not both, we have to do the best we can to make it
work properly.  CloneCommand should *never* have a --sm-client-id param in it. 
RestartCommand *always* needs --sm-client-id.  In general, the app shouldn't
add --sm-client-id to either, but basically when the session client connects to
the session manager, we need to have the behavior I just described.  I don't
care *how* we get it, it just has to be that way.

-- 
Configure bugmail: http://bugzilla.xfce.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.