Merge lp:~mvo/libappindicator/fix-820080 into lp:libappindicator

Proposed by Michael Vogt
Status: Merged
Approved by: Ted Gould
Approved revision: 226
Merged at revision: 221
Proposed branch: lp:~mvo/libappindicator/fix-820080
Merge into: lp:libappindicator
Diff against target: 63 lines (+39/-7)
1 file modified
src/app-indicator.c (+39/-7)
To merge this branch: bzr merge lp:~mvo/libappindicator/fix-820080
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
Michael Vogt Needs Resubmitting
Review via email: mp+77676@code.launchpad.net

Description of the change

This checks for the long item name before using it to ensure that apps that do not provide a "-panel" fallback icon still work.

This probably needs a additional check for "icon_theme_path" to ensure its catching custom icon theme paths, I'm happy to add this once I get approval for the general approach.

(this is against the right branch this time)

To post a comment you must log in.
Revision history for this message
Javier Jardón (jjardon) wrote :

Tested in several configurations: XFce, GNOME Classic, Ubuntu and seems to work ok.

Revision history for this message
Ted Gould (ted) wrote :

The approach is fine. The problem is you pulled out the distinction in the case between get_icon() and get_attention_icon(). So the attention icon won't work. But I think the test is a good idea.

review: Needs Fixing
Revision history for this message
Michael Vogt (mvo) wrote :

Oh, indeed, a rather silly bug, I fix this right away.

lp:~mvo/libappindicator/fix-820080 updated
222. By Michael Vogt

unbreak attention_icon

223. By Michael Vogt

src/app-indicator.c: honor icon_theme_path in the fallback

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks for the review, the attention icon issue is fixed now plus it should honor the custom_icon_path now too.
Please double check if the g_object_unref() at the end if good enough for not leaking the custom GtkIconTheme.

review: Needs Resubmitting
lp:~mvo/libappindicator/fix-820080 updated
224. By Michael Vogt

simplify

225. By Michael Vogt

src/app-indicator.c: do not add the same icon_theme_path again, gtk3 apparently does not check this

Revision history for this message
Michael Vogt (mvo) wrote :

I updated this again to avoid adding the same path multiple times. Gtk itself does not care it seems:

>>> theme = Gtk.IconTheme.get_default()
>>> print theme.get_search_path()
['/home/egon/.icons', '/home/egon/.local/share/icons', '/usr/share/ubuntu/icons', '/usr/share/gnome/icons', '/usr/local/share/icons', '/usr/share/icons', '/usr/share/ubuntu/pixmaps', '/usr/share/gnome/pixmaps', '/usr/local/share/pixmaps', '/usr/share/pixmaps']
>>> theme.append_search_path("foo")
>>> theme.append_search_path("foo")
>>> theme.append_search_path("foo")
>>> print theme.get_search_path()
['/home/egon/.icons', '/home/egon/.local/share/icons', '/usr/share/ubuntu/icons', '/usr/share/gnome/icons', '/usr/local/share/icons', '/usr/share/icons', '/usr/share/ubuntu/pixmaps', '/usr/share/gnome/pixmaps', '/usr/local/share/pixmaps', '/usr/share/pixmaps', 'foo', 'foo', 'foo']

lp:~mvo/libappindicator/fix-820080 updated
226. By Michael Vogt

src/app-indicator.c: hide first in passive mode and add comment about it

Revision history for this message
Ted Gould (ted) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/app-indicator.c'
2--- src/app-indicator.c 2011-07-22 19:04:23 +0000
3+++ src/app-indicator.c 2011-09-30 16:17:26 +0000
4@@ -1523,20 +1523,52 @@
5 GtkStatusIcon * icon = GTK_STATUS_ICON(data);
6 gchar *longname = NULL;
7
8+ /* add the icon_theme_path once if needed */
9+ GtkIconTheme *icon_theme = gtk_icon_theme_get_default();
10+ if (self->priv->icon_theme_path != NULL)
11+ {
12+ gchar **path;
13+ gint n_elements, i;
14+ gboolean found=FALSE;
15+ gtk_icon_theme_get_search_path(icon_theme, &path, &n_elements);
16+ for (i=0; i< n_elements || path[i] == NULL; i++)
17+ {
18+ if(g_strcmp0(path[i], self->priv->icon_theme_path) == 0)
19+ {
20+ found=TRUE;
21+ break;
22+ }
23+ }
24+ if(!found)
25+ gtk_icon_theme_append_search_path(icon_theme, self->priv->icon_theme_path);
26+ g_strfreev (path);
27+ }
28+
29 switch (app_indicator_get_status(self)) {
30 case APP_INDICATOR_STATUS_PASSIVE:
31- longname = append_panel_icon_suffix(app_indicator_get_icon(self));
32- gtk_status_icon_set_visible(icon, FALSE);
33- gtk_status_icon_set_from_icon_name(icon, longname);
34+ /* hide first to avoid that the change is visible to the user */
35+ gtk_status_icon_set_visible(icon, FALSE);
36+ longname = append_panel_icon_suffix(app_indicator_get_icon(self));
37+ if (gtk_icon_theme_has_icon (icon_theme, longname))
38+ gtk_status_icon_set_from_icon_name(icon, longname);
39+ else
40+ gtk_status_icon_set_from_icon_name(icon, app_indicator_get_icon(self));
41 break;
42 case APP_INDICATOR_STATUS_ACTIVE:
43- longname = append_panel_icon_suffix(app_indicator_get_icon(self));
44- gtk_status_icon_set_from_icon_name(icon, longname);
45+ longname = append_panel_icon_suffix(app_indicator_get_icon(self));
46+ if (gtk_icon_theme_has_icon (icon_theme, longname))
47+ gtk_status_icon_set_from_icon_name(icon, longname);
48+ else
49+ gtk_status_icon_set_from_icon_name(icon, app_indicator_get_icon(self));
50 gtk_status_icon_set_visible(icon, TRUE);
51 break;
52 case APP_INDICATOR_STATUS_ATTENTION:
53- longname = append_panel_icon_suffix(app_indicator_get_attention_icon(self));
54- gtk_status_icon_set_from_icon_name(icon, longname);
55+ /* get the _attention_ icon here */
56+ longname = append_panel_icon_suffix(app_indicator_get_attention_icon(self));
57+ if (gtk_icon_theme_has_icon (icon_theme, longname))
58+ gtk_status_icon_set_from_icon_name(icon, longname);
59+ else
60+ gtk_status_icon_set_from_icon_name(icon, app_indicator_get_icon(self));
61 gtk_status_icon_set_visible(icon, TRUE);
62 break;
63 };

Subscribers

People subscribed via source and target branches