Comment 24 for bug 129219

Revision history for this message
In , Karlt (karlt) wrote :

Comment on attachment 813885
0001-Bug-417952-Open-Containing-Folder-doesn-t-highlight-.patch

Thank you for working out how to do this, Nelson. This looks like the right
approach.

Putting this in the nsGIOService is fine, even though the current
implementation doesn't use GIO, because GIO's GDBus will be the preferred DBus
library once Mozilla is no longer supporting older systems, and the method is
similar to other existing methods on nsGIOService.

The interface can be simplified to just one additional method on nsIGIOService
that returns NS_ERROR_NOT_AVAILABLE or similar when FileManager1 is not
available.

ListActivatableNames doesn't sound like what we want here. I assume that is
looking for /usr/share/dbus-1/services/org.freedesktop.FileManager1.service,
but the interface may be provided by a non activatable application such as a
daemon in the desktop environment. That is in fact the appropriate way to
support the interface because it means that different applications can be
selected by the user [1]. What Nautilus does [2][3] is unfortunate because it
means that people that don't want Nautilus used to reveal a file will have to
uninstall nautilus from the system, removing the option of other users using
Nautilus.

The simplest approach would be to use the synchronous dbus_g_proxy_call() so
that we know whether the call succeeds. Given this particular use of DBus
where the user is wanting to open a new application, or at least a new window,
which is going to take time, the additional round trip time via the DBus
daemon is not going to be major.

Perhaps the call could set a flag if it fails, in order to save checking the
DBus call each time for systems without FileManager1, or perhaps there is a
way to register for notifications when the interface becomes available, but
that may be more trouble than it is worth. Similarly I imagine there is a
notification when the interface is no longer available (the "destroy" signal
maybe?) so the call would need only be synchronous the first time, but that
can be a future optimization when necessary.

Gecko file style is to use spaces for indentation, with no tabs.

Call dbus_connection_set_exit_on_disconnect(dbusConnection, false) in case
this is the first use of the connection.

>+ char *uris[2] = { (char*)PromiseFlatCString(uri).get(), NULL };

The PromiseFlatCString usage is not good because the pointer is used after the
PromiseFlatCString is destroyed.
I expect uri.get() can be used here.

Please use const_cast instead of (char*), or, perhaps better, make this
const char *uris[2].

(In reply to Nelson Benítez León from comment #12)
> some file managers when
> passed a full uri would try to 'run' the file (as was thunar in xfce or
> konqueror in kde)

I was surprised by that behavior too. I think there is a KDE bug report on
that.

[1] http://lists.freedesktop.org/archives/xdg/2011-May/011971.html
[2] https://mail.gnome.org/archives/commits-list/2011-December/msg05097.html
[3] https://git.gnome.org/browse/nautilus/tree/data/Makefile.am?id=b3434e8bec9231b21f34fbe4bfc5e05b8d2e592b#n28