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

Revision history for this message
Aurélien Gâteau (agateau) wrote :

Works well, except if one tries to set KDE applications as defaults. See the end of the review for details.

Some questions, suggestions first:

# LauncherApplication::setDesktopFile()
- Should the if newDesktopFile != oldDesktopFile be something like this instead?

"""
if (newDesktopFile == oldDesktopFile) {
    return;
}
Q_EMIT desktopFileChanged(newDesktopFile);
"""

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

# 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.

# 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 ).

review: Needs Fixing

« Back to merge proposal