Code review comment for lp://qastaging/~osomon/unity-2d/preferred-applications

> # LauncherApplication::setDesktopFile()
> - Should the if newDesktopFile != oldDesktopFile be something like this
> instead?
>
> """
> if (newDesktopFile == oldDesktopFile) {
> return;
> }
> Q_EMIT desktopFileChanged(newDesktopFile);
> """

For some reason which I still need to investigate, applying this change seems to break the update of icons for newly created web favorites.

> - Not really related to this MR but g_object_unref(m_appInfo) should be called
> before creating a new GDesktopAppInfo.

I believe you fixed this one already, in a separate merge request.

> # places/HomeButtonApplication.qml
>
> - Having both desktopFile and desktop_file is quite confusing.
> application.desktop_file should be renamed to application.desktopFile and code
> inside it should use parent.desktopFile to disambiguate.

This is a more general problem with naming consistency of the properties of some classes, particularly LauncherApplication. 'desktop_file' should be renamed 'desktopFile' indeed, but I don’t think it’s a good idea to do this change here and now. Renaming should be performed in a separate branch and fix all the properties, not just this one.
I tried to make the code more explicit by using parent.desktopFile nevertheless, but this doesn’t seem to work: 'parent' is undefined in this block of code…

> # Problem with KDE applications
>
> Assuming desktopFile == binaryName + ".desktop" makes it unpractical to set
> KDE applications as default. To set a KDE4 application as default I have to
> prefix its name with "kde4-" in gnome-default-application-properties.
>
> The reason behind this is that KDE4 desktop files are all installed in
> /usr/share/applications/kde4, meaning their desktop file id is
> "kde4-$app.desktop" (see http://standards.freedesktop.org/menu-
> spec/1.0/go01.html#term-desktop-file-id ).

We have a more general issue with KDE4 applications throughout unity-2d’s applications. I know this solution is far from perfect, but it’s a cheap one that doesn’t involve writing custom C++ code to infer the correct desktop file from the path of an executable. It’s also the way it’s done in Unity, meaning Unity will fail to display KDE4 applications too. I can definitely give it a try, if we think the effort is worth the benefit. Do we?