Merge lp:~tigrangab/slingshot/bug_1072036 into lp:~elementary-pantheon/slingshot/trunk

[Tigran Gabrielyan]

Fix fallback methods of finding an apps icon to work in all views. Fixes bug #1072036.

[David Gomes (davidgomes)]

my_function_call (tom, josh, peter
                  hello, new Game ());

That's how you align 2 lines (many editors do this automatically) when you split them. You have a lot to realign in your branch.

I can't test this bug cos I've never even experienced so I'll need a tester too. The code itself is well made, good job.

review: Needs Fixing

[Tigran Gabrielyan (tigrangab)]

If you need a test file: https://gist.github.com/tigrang/6312102/raw/09b075acc4e6f38e5eef35cf311b0664b8ce9573/test.desktop

Before patch:
In Grid view, the icon for "Testing Bug 1072036" will show. When you search "Test" it will not show.

After the patch:
It should work in both views.

[lp:~tigrangab/slingshot/bug_1072036]

377. By Tigran Gabrielyan on 2013-08-22

Clean up code

378. By Tigran Gabrielyan on 2013-08-22

Add warning/error messages to fallback methods

[David Gomes (davidgomes)]

>}),

Add a newline after those on the array with the different methods.

Diff lines 48 and 54, remove both.

Also, why not?

foreach (IconLoadFallbackMethod fallback in fallbacks) {
 try {
    fallback.load_icon()
 } catch (Error ) {

 }
}

to avoid some code repetition?

review: Needs Fixing

[Tigran Gabrielyan (tigrangab)]

That's what I did initially, but it doesn't catch the error that way, not sure why.

[lp:~tigrangab/slingshot/bug_1072036]

379. By Tigran Gabrielyan on 2013-08-24

Remove unneeded lines

380. By Tigran Gabrielyan on 2013-08-24

Add newlines between array items

[Sergey "Shnatsel" Davidoff (shnatsel)]

I can confirm this fixes the bug for the test launcher and for Skype. Great job, thanks!

review: Approve

[David Gomes (davidgomes)]

public IconLoadFallbackMethod(IconLoadFallback fallback) {
↓
public IconLoadFallbackMethod (IconLoadFallback fallback) {

That's the last bit, then it's a merge man, great job!

review: Needs Fixing

[lp:~tigrangab/slingshot/bug_1072036]

381. By Tigran Gabrielyan on 2013-08-26

Fix yet another CS issue

[Tigran Gabrielyan (tigrangab)]

Sorry about that :) This coding style is really odd/different to me. I'll get used to it eventually.

[David Gomes (davidgomes)]

Great job.

review: Approve

=== modified file 'src/Backend/App.vala'
--- src/Backend/App.vala	2013-04-17 18:32:40 +0000
+++ src/Backend/App.vala	2013-08-26 17:15:32 +0000
@@ -38,7 +38,6 @@
         public signal void launched (App app);
 
         public App (GMenu.TreeEntry entry) {
-
             name = entry.get_display_name ();
             description = entry.get_comment () ?? name;
             exec = entry.get_exec ();
@@ -47,10 +46,9 @@
             desktop_path = entry.get_desktop_file_path ();
             keywords = Unity.AppInfoManager.get_default ().get_keywords (desktop_id);
             generic_name = entry.get_generic_name ();
-            
+
             update_icon ();
             Slingshot.icon_theme.changed.connect (update_icon);
-
         }
 
         public App.from_command (string command) {
@@ -68,40 +66,79 @@
         }
 
         public void update_icon () {
-
-            try {
-                icon = Slingshot.icon_theme.load_icon (icon_name, Slingshot.settings.icon_size,
-                                                        Gtk.IconLookupFlags.FORCE_SIZE);
-            } catch (Error e) {
-                try {
-                    if (icon_name.last_index_of (".") > 0)
-                        icon = Slingshot.icon_theme.load_icon (icon_name[0:icon_name.last_index_of (".")],
-                                                               Slingshot.settings.icon_size, Gtk.IconLookupFlags.FORCE_SIZE);
-                    else
-                        throw new IOError.NOT_FOUND ("Requested image could not be found.");
-
-                } catch (Error e) {
+            icon = load_icon (Slingshot.settings.icon_size);
+            icon_changed ();
+        }
+
+        private delegate void IconLoadFallback ();
+
+        private class IconLoadFallbackMethod {
+            public unowned IconLoadFallback load_icon;
+
+            public IconLoadFallbackMethod (IconLoadFallback fallback) {
+                load_icon = fallback;
+            }
+        }
+
+        public Gdk.Pixbuf load_icon (int size) {
+            Gdk.Pixbuf icon = null;
+            var flags = Gtk.IconLookupFlags.FORCE_SIZE;
+
+            IconLoadFallbackMethod[] fallbacks = {
+                new IconLoadFallbackMethod (() => {
                     try {
-                        icon = new Gdk.Pixbuf.from_file_at_scale (icon_name, Slingshot.settings.icon_size,
-                                                                  Slingshot.settings.icon_size, false);
+                        icon = Slingshot.icon_theme.load_icon (icon_name, size, flags);
                     } catch (Error e) {
-                        try {
-                            icon = Slingshot.icon_theme.load_icon ("application-default-icon", Slingshot.settings.icon_size,
-                                                                   Gtk.IconLookupFlags.FORCE_SIZE);
-                        } catch (Error e) {
-                            icon = Slingshot.icon_theme.load_icon ("gtk-missing-image", Slingshot.settings.icon_size,
-                                                                   Gtk.IconLookupFlags.FORCE_SIZE);
+                        warning ("Could not load icon. Falling back to method 2");
+                    }
+                }),
+
+                new IconLoadFallbackMethod (() => {
+                    try {
+                        if (icon_name.last_index_of (".") > 0) {
+                            var name = icon_name[0:icon_name.last_index_of (".")];
+                            icon = Slingshot.icon_theme.load_icon (name, size, flags);
                         }
-                    }
-                }
+                    } catch (Error e) {
+                        warning ("Could not load icon. Falling back to method 3");
+                    }
+                }),
+
+                new IconLoadFallbackMethod (() => {
+                    try {
+                        icon = new Gdk.Pixbuf.from_file_at_scale (icon_name, size, size, false);
+                    } catch (Error e) {
+                        warning ("Could not load icon. Falling back to method 4");
+                    }
+                }),
+
+                new IconLoadFallbackMethod (() => {
+                    try {
+                        icon = Slingshot.icon_theme.load_icon ("application-default-icon", size, flags);
+                     } catch (Error e) {
+                         warning ("Could not load icon. Falling back to method 5");
+                     }
+                }),
+
+                new IconLoadFallbackMethod (() => {
+                     try {
+                        icon = Slingshot.icon_theme.load_icon ("gtk-missing-image", size, flags);
+                     } catch (Error e) {
+                        error ("Could not find a fallback icon to load");
+                     }
+                })
+            };
+
+            foreach (IconLoadFallbackMethod fallback in fallbacks) {
+                fallback.load_icon ();
+                if (icon != null)
+                    break;
             }
 
-            icon_changed ();
-
+            return icon;
         }
 
         public void launch () {
-
             try {
                 if (is_command) {
                     debug (@"Launching command: $name");
@@ -114,7 +151,6 @@
             } catch (Error e) {
                 warning ("Failed to launch %s: %s", name, exec);
             }
-
         }
 
     }

=== modified file 'src/Widgets/SearchItem.vala'
--- src/Widgets/SearchItem.vala	2012-09-09 18:14:52 +0000
+++ src/Widgets/SearchItem.vala	2013-08-26 17:15:32 +0000
@@ -23,6 +23,7 @@
 
     public class SearchItem : Button {
 
+        private Backend.App app;
         private Pixbuf icon;
         private string icon_name;
         private Label name_label;
@@ -33,7 +34,7 @@
         public signal void launch_app ();
 
         public SearchItem (Backend.App app) {
-
+            this.app = app;
             get_style_context ().add_class ("app");
 
             icon = app.icon;
@@ -56,33 +57,15 @@
             add (Utils.set_padding (vbox, 5, 0, 0, 78));
 
             this.launch_app.connect (app.launch);
-
         }
 
         protected override bool draw (Cairo.Context cr) {
-
             Allocation size;
             get_allocation (out size);
 
             base.draw (cr);
 
-            Pixbuf scaled_icon = null;
-            try {
-                scaled_icon = Slingshot.icon_theme.load_icon (icon_name, icon_size,
-                                                        Gtk.IconLookupFlags.FORCE_SIZE);
-            } catch (Error e) {
-                try {
-                    scaled_icon = new Gdk.Pixbuf.from_file_at_scale (icon_name, icon_size, icon_size, false);
-                } catch (Error e) {
-	            	try {
-                        scaled_icon = Slingshot.icon_theme.load_icon ("application-default-icon", icon_size,
-                                                               Gtk.IconLookupFlags.FORCE_SIZE);
-	        	    } catch (Error e) {
-                        scaled_icon = Slingshot.icon_theme.load_icon ("gtk-missing-image", icon_size,
-                                                               Gtk.IconLookupFlags.FORCE_SIZE);
-	            	}
-                }
-            }
+            Pixbuf scaled_icon = app.load_icon (icon_size);
 
             height_request = icon_size + 10;
 
@@ -91,7 +74,6 @@
             cr.paint ();
 
             return true;
-
         }
 
         private string fix (string text) {