Merge lp:~larsu/indicator-messages/lp1365408 into lp:indicator-messages/13.10

[Lars Karlitski]

Support X-Ubuntu-SymbolicIcon in desktop files

Support X-Ubuntu-SymbolicIcon in desktop files

As a way for applications to supply symbolic icons which are not in the theme but specified by file name.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:420
http://jenkins.qa.ubuntu.com/job/indicator-messages-ci/129/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-messages-utopic-amd64-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-messages-utopic-armhf-ci/9
        deb: http://jenkins.qa.ubuntu.com/job/indicator-messages-utopic-armhf-ci/9/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-messages-ci/129/rebuild

review: Approve (continuous-integration)

[Renato Araujo Oliveira Filho (renatofilho)]

Works nice.

review: Approve

[Ted Gould (ted)]

review: Needs Information

[Lars Karlitski (larsu)]

Sorry I replied inline a couple of days ago but apparently didn't hit "Save Comment" up here.

[Ted Gould (ted)]

On Fri, 2014-09-19 at 07:10 +0000, Lars Uebernickel wrote:

> I've tried to keep it semantically similar to the 'Icon' key, which
> takes an absolute path and doesn't look at 'Path'. Icons are also
> something that is definitely part of the installed app, not its
> runtime directory. What benefit would we get from adding 'Path'?

I see what you're saying there, in thinking of Path as the runtime
directory. The way that we're building the desktop files for click
packages is that Path is the directory of the package. So path would be
the appropriate place to look for any relative icon locations. But,
that's overloading the key in that case.

So I guess that means we can think of this a couple ways:

Long term: We really need the messaging menu to get proper Click package
support. We've used the legacy FD.o dirs to limp through, but we should
do the work to do this right. It should be able to read the symbolic
icon name and figure out how to resolve that properly. That's not going
to happen for 14.10.

Short term: Perhaps we should add special handling for the generated
desktop files for the symbolic icon that resolves the directories and
provides absolute paths. Work arounds on top of work arounds, but that
would resolve the directory issue of the symbolic icon today.

I'll go ahead and add a UAL task to the bug and handle that in the
desktop hook, as I think that makes sense for the short term, but we
should definitely look at how to make messaging menu more click aware
sooner rather than later.

[Ted Gould (ted)]

review: Approve

=== modified file 'src/im-application-list.c'
--- src/im-application-list.c	2014-07-30 22:33:13 +0000
+++ src/im-application-list.c	2014-09-16 15:02:35 +0000
@@ -886,14 +886,40 @@
 }
 
 static GIcon *
-get_symbolic_app_icon (GAppInfo *info)
+get_symbolic_app_icon (GDesktopAppInfo *info)
 {
+  gchar *x_symbolic_icon;
   GIcon *icon;
   const gchar * const *names;
   gchar *symbolic_name;
   GIcon *symbolic_icon;
 
-  icon = g_app_info_get_icon (info);
+  /* If X-Ubuntu-SymbolicIcon exists, use that. It is always interpreted
+   * as a filename.
+   *
+   * Simply appending -symbolic to the normal icon doesn't work for
+   * icons specified by file name, because the symbolic icon might be in
+   * a different format (symbolic icons tend to be svg and normal icons
+   * png). Also, icons specified by file names don't allow for fallbacks
+   * (without stating the file from this process), and we'd want to fall
+   * back to the normal app icon.
+   *
+   * See lp: #1365408
+   */
+  if ((x_symbolic_icon = g_desktop_app_info_get_string (info, "X-Ubuntu-SymbolicIcon")))
+    {
+      GFile *file;
+
+      file = g_file_new_for_path (x_symbolic_icon);
+      symbolic_icon = g_file_icon_new (file);
+
+      g_object_unref (file);
+      g_free (x_symbolic_icon);
+
+      return symbolic_icon;
+    }
+
+  icon = g_app_info_get_icon (G_APP_INFO (info));
   if (icon == NULL)
     return NULL;
 
@@ -1014,7 +1040,7 @@
       im_application_list_update_root_action (app->list);
     }
 
-  app_icon = get_symbolic_app_icon (G_APP_INFO (app->info));
+  app_icon = get_symbolic_app_icon (app->info);
 
   g_signal_emit (app->list, signals[MESSAGE_ADDED], 0,
                  app->id, app_icon, id, serialized_icon, title,