libappindicator

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

  1. fix-820080
  2. Merge into trunk
Proposed by Michael Vogt
Status: Merged
Approved by: Ted Gould
Approved revision: 226
Merged at revision: 221
Proposed branch: lp://staging/~mvo/libappindicator/fix-820080
Merge into: lp://staging/libappindicator
Diff against target: 63 lines (+39/-7)
1 file modified
src/app-indicator.c (+39/-7)
To merge this branch: bzr merge lp://staging/~mvo/libappindicator/fix-820080
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
Michael Vogt (community) Needs Resubmitting
Review via email: mp+77676@code.staging.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://staging/~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://staging/~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://staging/~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
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches