[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 #4 from Dimitar Zhekov <hamster at mbox.contact.bg>  2009-05-20 17:43:02 UTC ---
(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.

> Of course, *most* users of the library probably use _new_full() which will
> somewhat get around this problem, but, regardless, adding this struct member
> changes ABI.  No good.

Well, xfce panel, xfce4-settings-helper, xfdesktop, xfwm and Thunar seem to
work fine. I haven't really seen any xfce session client applications with
interactive session save and can't tell about them.

> > The --display (if missing) was appended only to the automatically generated
> > client_session_new() restart/clone commands, and not to any commands set
> > by the application. So I moved appending to set_clone_restart_commands().
> 
> I don't think this is correct.  If the caller sets a clone/restart command,
> the library should send *exactly* what was passed to it.  If you want a
> --display parameter in your clone/restart command, and you're setting one
> manually, you should add --display.

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,

> 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?..

> > 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.

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