Merge lp:~stolowski/unity-lens-music/artwork-for-files-only into lp:unity-lens-music

[Paweł Stołowski]

Ignore iradio tag (radio stations) when parsing Rhythmbox database; check if track uri is a local file, this prevents unexpected network queries when radio stations are enabled back in the future.

Ignore iradio tag (radio stations) when parsing Rhythmbox database; check if track uri is a local file, this prevents unexpected network queries when radio stations are enabled back in the future.

[Paweł Stołowski (stolowski)]

Please don't approve globally until Omer (originator of the bug) verifies it fixes the problem for him!

[Omer Akram (om26er)]

I created a new user account and the issue is 100% reproducible there for me and this branch actually fixes the problem but now songs without coverart look like this: http://ubuntuone.com/4e6tlFE5HKBAsmZZu5EV6W

review: Needs Fixing

[Jussi Pakkanen (jpakkane)]

Looks sensible to me. Approving with the assumption that the coverart issue above is either fixed or deemed not to be serious enough.

review: Approve

[Paweł Stołowski (stolowski)]

@Omer: two questions:
- did you compile music lens with --prefix=/usr ?
- do you have /usr/share/unity/6/album_missing.png (from unity-common package) on your system?

[Omer Akram (om26er)]

Its installed in /usr/local and /usr/share/unity/6/album_missing.png does exist.

[Paweł Stołowski (stolowski)]

> Its installed in /usr/local and /usr/share/unity/6/album_missing.png does
> exist.

You need to compile it with --prefix=/usr, otherwise it won't find album_missing.png (this icon is a recent addition); I think that's why you have broken icon if covert art is missing.

[Omer Akram (om26er)]

Then I guess we should merge this in as it clearly fixes the issue for me without any apparent regression.

[Omer Akram (om26er)]

review: Approve (functional)

=== modified file 'src/rhythmbox-collection.vala'
--- src/rhythmbox-collection.vala	2012-09-18 13:10:34 +0000
+++ src/rhythmbox-collection.vala	2012-09-20 16:19:22 +0000
@@ -114,7 +114,7 @@
                   for (int i = 0; attr_names[i] != null; i++)
                   {
                     if (attr_names[i] == "type" && (attr_values[i] == "song"
-                        || attr_values[i] == "iradio"))
+                      /*|| attr_values[i] == "iradio"*/)) // disable radio stations
                       accepted_element_name = attr_values[i];
                   }
                   if (accepted_element_name == null) return;
@@ -336,14 +336,17 @@
                     ChecksumType.MD5, combined)));
             if (FileUtils.test (filename, FileTest.EXISTS)) return filename;
 
-            // Try Nautilus thumbnails
-            try
+            if (track.uri.has_prefix ("file://"))
             {
+              // Try Nautilus thumbnails
+              try
+              {
                 File artwork_file = File.new_for_uri (track.uri);
                 var info = artwork_file.query_info (FILE_ATTRIBUTE_THUMBNAIL_PATH, 0, null);
                 var thumbnail_path = info.get_attribute_string (FILE_ATTRIBUTE_THUMBNAIL_PATH);
                 if (thumbnail_path != null) return thumbnail_path;
-            } catch {}
+              } catch {}
+            }
 
             // Try covers folder
             string artwork = Path.build_filename (

=== modified file 'tests/unit/test-rhythmbox-parser.vala'
--- tests/unit/test-rhythmbox-parser.vala	2012-08-13 10:36:17 +0000
+++ tests/unit/test-rhythmbox-parser.vala	2012-09-20 16:19:22 +0000
@@ -36,6 +36,9 @@
     return 0;
   }
 
+  /* FIXME: this test is broken - it passes if iradio tag is not parsed
+   * because all checks are done in a callback that's never called in such case.
+   */
   private static void test_radios ()
   {
     var parser = new RhythmboxCollection.XmlParser ();