Comment 44 for bug 867424

Revision history for this message
In , Oliver-henshaw (oliver-henshaw) wrote :

Created attachment 8757425
(2/2) - Stop using libgnome and libgnomeui on Linux v5.

Changes from v4
---------------

Made myself the author, since I deserve the blame even though I don't
deserve the credit. Added a note that Chris Coulson wrote the original
patch. If there's a better way to handle this, perhaps someone with
mercurial-fu can do the magic?

Addressed review comments:

> ::: toolkit/xre/nsNativeAppSupportUnix.cpp
> @@ +429,5 @@
> > +
> > + --gArgc;
> > +}
> > +
> > +static void setSMValue(SmPropValue& val, const nsCString& data)
>
> Please capitalize function names and add a newline after the return type
> declaration in implementations.
>
> Also, this block of code isn't guarded by MOZ_X11. Systems without X11 won't
> be able to resolve SmPropValue.

Done.

>
> @@ +436,5 @@
> > + val.length = data.Length();
> > +}
> > +
> > +static void setSMProperty(SmProp& prop, const char* name, const char* type,
> > + int numVals, SmPropValue vals[])
>
> Style and guard as described above.
and done.

> @@ +491,5 @@
> > + char *arg = *curarg;
> > + if (arg[0] == '-' && arg[1] == '-') {
> > + arg += 2;
> > + if (!strcmp(arg, "sm-disable")) {
> > + RemoveArg(curarg);
>
> Is there any reason we can't just increment argv / decrement argc here
> instead of shifting the char* pointers in RemoveArg?
The argument to remove might be at an arbitrary position in argv, so I
don't think there's a better implementation. Actually, it's copied from
toolkit/xre/nsAppRunner.cpp so might it be worth a followup
de-duplicating it?

>
> @@ +576,5 @@
> > + char errbuf[256];
> > + mSessionConnection = SmcOpenConnection(nullptr, this, SmProtoMajor,
> > + SmProtoMinor, mask, &callbacks,
> > + prev_client_id.get(), &client_id,
> > + 256, errbuf);
>
> Replace 256 with sizeof(errbuf).

Done

Note
----

An easier way to test interaction during shutdown (i.e.
"quit-application-requested") by having multiple tabs open and set
browser.showQuitWarning to true, then configuring the session manager to
not save state (so it only asks for SmSaveGlobal)