Created attachment 87794 [details] [review]
the patch

Hardcoding "BINDIR/gnome-open" is not exactly nice. Why not try to spawn gnome-open and if that fails, try again with xdg-open? But pick them from the user's binary search path instead of hardcoding BINDIR.

+	ret = g_spawn_command_line_async (cmdline, &error);

Should use gdk_spawn_on_screen so the help opens on the right screen. You can get the screen from the status icon with gtk_status_icon_get_screen (#if GTK_CHECK_VERSION (2, 11, 0); using the default screen for ≦ 2.10 is ok).

Also, the URL is UTF-8. Does it need to be escaped to survive being passed to the command line ?

+		g_message("failed to show url: %s", error->message);

This isn't exactly user-friendly; if the user wants to visit the link and it fails, he should at least get a dialogue box showing the error.

Created attachment 87957 [details] [review]
updated patch

Sven and Christian, thanks for the comments.
I think I address them in this updated patch, except the one regarding escaping.
The URL is known and contains ASCII only so I am not sure it needs any precautions
but I may be missing something

If we're going to drop support for GTK+ < 2.6.0, there's still another check in other-network-dialog.c that should go, and configure needs to start enforcing versions.  If you want to drop the libgnomeui dependency, we can probably do that by switching the #include in main.c to reference libgnome instead.

I'm rather indifferent here.  There's not much gain to dropping support for older GTK+s since we're not really that tied to a GNOME release at this point.  Is the libgnomeui dependency/gtk version that big of an issue?

The main goal is dropping dependencies on libgnome and libgnomeui as per project Ridley and getting rid of the ~20 additional shared objects linking to those.
This will make it more appropriate for including with Xfce in distros while saving resident memory and imporving startup time regardless of the desktop env used.
The bumping of GTK req is a side effect leading to cleaner code in the case of n-m. I'll post a refreshed patch which deals with the two other files you mentioned if the idea seems ok.thanks.

If you're going to go through and remove *both* libgnome and libgnomeui dependencies, then yes, I'm much happier with the change.  But removing only one seems silly.  The bug is titled "reduce" which seems to imply you don't want to remove the dependency completely which matched the patch, and that made even less sense to me :)

Ah, I remember now. I was going to post patches incrementally one file at a time, to see if the maintainers agree with the basic idea of dropping the deps, and started with the one that is less intrusive.
And while it would still leave the deps it is a step towards the goal.

The other changes imply using gtk_init instead of gnome_program_init (this entails no bug buddy integration) the other is not using gnome client at all and letting it be a regular autostarted app. I am not sure what using GnomeClient buys nm-applet.

We need to keep bug-buddy integration.  It's found some important crashes for us already.  Is there no way to do bug-buddy without using libgnome*?  If not, then I don't think we should worry about this bug for right now.

there have been talks of making bug buddy a separate GTK_MODULE for gnome 2.20
but I am not sure what the progress is

The GTK_MODULE work on bug-buddy is being done in bug 388441.

Sadly, that looks stalled...

this doesn't have to be an all-or-nothing patch.

- the GTK <2.6 cleanup is good regardless of anything else=
- the GnomeClient usage is separate as well, I suppose it was one of those
copy-paste so it's better safe than sorry approaches ;)
- the bug-buddy if the only GNOME dep left can be dependent on a --enable-gnome
switch as many GNOME apps already have, thus making it already acceptable as
default for Xfce, or distros with their own crash handlers (ex ubuntu)

Please consider these as separate issues. The legacy cruft that is present in almost all GNOME apps accumulated (or more correctly became legacy) one by
one during the years, and that is the most sensible way of fixing it too.

If all apps only replace their remaining libgnome APIs when all are replacable
at once, the pace is a lot slower and there are less incentives for occasional
contributors to help things move forward. So fight 'broken windows' :)

So here's my take on things.  We still have users of NM who use GTK+ 2.4 on older distros (the bulk of NM's dependencies are "big" but are fairly constrained: kernel, wireless-tools, dbus, dhcdbd, hal, udev, wpa_supplicant).

Forcing users to upgrade GTK+ is slightly more intrusive because of various changes to GTK+ over the years.  ABI bumps, new dependencies on pango/cairo, filename encoding differences, etc. and essentially require a rebuild of the entire stack of things that build against GTK+.

Since we can't get rid of the libgnome* stuff, we're inflicting major change on some users for very little gain in this circumstance.

The 3 entry points to libgnome* you outlined above are very small and constrained.  We aren't going to add any more libgnome* entry points, so I believe that it will remain just as small and easy to do when the bug buddy GTK_MODULE stuff is done.  When we can get rid of all the entry points and actually be rid of the libgnome* dependencies, I'll be more than happy to bump the requirements since we get a real gain from doing that.

are there enough users with old distros (gtk 2.4) that are going to build new kernels dbus and use nm 0.7 but not upgrade gtk ? I personally find this unrealistic. For such a userbase which I am sure it's <1% of the whole you are indirectly affecting the rest. But you're the maintainer so I agree we can just drop this bugreport

I'm all for dropping GTK < 2.6 support on trunk (ie, NM 0.7).

Created attachment 90379 [details] [review]
drop gtk 2.6

patch that drops GTK+ < 2.6 support

Created attachment 90436 [details] [review]
drop GnomeClient

the session and die callbacks look like nops and the RESTART_ANYWAY thing is done by autostart.

Committed to trunk, thanks.