Merge lp:~aacid/indicator-sound/absolute_icon_paths into lp:indicator-sound/12.10

Proposed by Albert Astals Cid
Status: Merged
Approved by: Conor Curran
Approved revision: 334
Merged at revision: 334
Proposed branch: lp:~aacid/indicator-sound/absolute_icon_paths
Merge into: lp:indicator-sound/12.10
Diff against target: 24 lines (+12/-2)
1 file modified
src/metadata-widget.c (+12/-2)
To merge this branch: bzr merge lp:~aacid/indicator-sound/absolute_icon_paths
Reviewer Review Type Date Requested Status
Conor Curran (community) Approve
jenkins (community) continuous-integration Approve
Review via email: mp+128875@code.launchpad.net

Commit message

Use the file path directly if it's an absolute path

Description of the change

Use the file path directly if it's an absolute path

To post a comment you must log in.
jenkins (martin-mrazik+qa) wrote :
review: Approve (continuous-integration)
Conor Curran (cjcurran) :
review: Approve
Lars Karlitski (larsu) wrote :

The g_file_test is pointless: gdk_pixbuf_new_from_file_at_scale will just return NULL if the file doesn't exist (or couldn't be read). Also, the file might be deleted between the two calls.

`pix` might be NULL, which is okay for gtk_image_set_from_pixbuf, but g_object_unref will print a warning. It would be preferable to print the actual error (pass the GError into gdk_pixbuf_new_from_file_at_scale).

Why didn't you just use gtk_image_set_from_file?

Albert Astals Cid (aacid) wrote :

> gdk_pixbuf_new_from_file_at_scale will just return NULL if the file doesn't exist (or couldn't be read)
I thought that for that reason is better to fall back to the else case that will give me the default "not found" icon

> Also, the file might be deleted between the two calls.
True

> Why didn't you just use gtk_image_set_from_file?
Because it might not be of the size we want

> `pix` might be NULL, which is okay for gtk_image_set_from_pixbuf, but g_object_unref will print a warning. It
> would be preferable to print the actual error (pass the GError into gdk_pixbuf_new_from_file_at_scale).
Want me to create a new merge request for this?

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/metadata-widget.c'
2--- src/metadata-widget.c 2012-03-30 17:16:34 +0000
3+++ src/metadata-widget.c 2012-10-10 07:54:28 +0000
4@@ -804,8 +804,18 @@
5 dbusmenu_menuitem_property_get ( priv->twin_item,
6 DBUSMENU_METADATA_MENUITEM_PLAYER_ICON ));
7 }
8-
9- gtk_image_set_from_icon_name(GTK_IMAGE (priv->player_icon), app_panel->str, GTK_ICON_SIZE_MENU);
10+
11+ const GtkIconSize icon_size = GTK_ICON_SIZE_MENU;
12+ if (g_path_is_absolute(app_panel->str) && g_file_test (app_panel->str, G_FILE_TEST_IS_REGULAR)){
13+ gint width, height;
14+ gtk_icon_size_lookup (icon_size, &width, &height);
15+ GdkPixbuf *pix = gdk_pixbuf_new_from_file_at_scale(app_panel->str, width, height, TRUE, NULL);
16+ gtk_image_set_from_pixbuf (GTK_IMAGE (priv->player_icon), pix);
17+ g_object_unref (pix);
18+ }
19+ else{
20+ gtk_image_set_from_icon_name(GTK_IMAGE (priv->player_icon), app_panel->str, icon_size);
21+ }
22
23 g_string_free ( app_panel, TRUE);
24 g_string_free ( banshee_string, TRUE);

Subscribers

People subscribed via source and target branches