Comment on attachment 826479 0001-Bug-417952-Open-Containing-Folder-doesn-t-highlight-.patch
Thanks. This is good. Just some minor things: >+ DBusGConnection* mDBusConnection = nullptr; >+ DBusConnection* dbusConnection = nullptr; >+ DBusGProxy* mDBusProxy = nullptr; >+ gboolean rv_dbus_call; Gecko uses the 'm' prefix on variables if they are member variables, but these are local variables, so please rename mDBusConnection and mDBusProxy to something without the prefix, but still starting with a lower-case letter. The initialization is unnecessary here, but this is C++ code, so these can be declared when they are first set, and that is Gecko style. e.g. DBusGConnection* mDBusConnection = dbus_g_bus_get(DBUS_BUS_SESSION, &error); Initializing out parameters, as you have with |error|, is often good (and may be necessary even - I haven't checked). >+ nsAutoCString uri("file://"); >+ uri.Append(aUri); Please use g_filename_to_uri(aPath, nullptr, nullptr) to do any necessary escaping. >+ const char *uris[2] = { (const char*) uri.get(), NULL }; The (const char*) can be removed now, I assume. Also, please use nullptr instead of NULL as nullptr provides more type safety than a plain 0. >+ } else if (giovfs && giovfs->OrgFreedesktopFileManager1ShowItems(mPath) == NS_OK) { Gecko style is usually to write NS_SUCCEEDED(giovfs->OrgFreedesktopFileManager1ShowItems(mPath)) > [noscript] void showURIForInput(in ACString uri); >+ >+ /* Open uri in file manager using org.freedesktop.FileManager1 interface */ >+ [noscript] void orgFreedesktopFileManager1ShowItems(in ACString uri); showURIForInput set a bad precedent here because the parameter is a path, not a uri. Please name the parameter for the new method "path", and update the comment. I assume that is easier than changing the caller to pass a uri. -- You received this bug notification because you are a member of Ubuntu Desktop Bugs, which is a bug assignee. https://bugs.launchpad.net/bugs/129219 Title: firefox download manager - Open containing folder with preselected file in nautilus/file manager To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/129219/+subscriptions -- desktop-bugs mailing list desktop-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/desktop-bugs