GNOME Bugzilla – Bug 564355
Uri field is using a different format in the DB depending if the file is relative or not, so smart playlists that use FileLocation don't work as expected
Last modified: 2009-04-22 08:02:05 UTC
Please describe the problem: While the "File location" column displays things like: /home/user/My Music/Coldplay - Clocks.mp3 The DB stores: file:///home/user/My%20Music/Coldplay%20-%20Clocks.mp3 As a result, many confused users are filing bugs like bug 548128 or bug 560934. I wanted to create a new bug in order to expose the main reason of all of this bugs and post several issues along with their steps to reproduce. Steps to reproduce: 1. Have a song in /home/user/.../whatever.mp3 2. Create a smartplaylist with filter File Location Starts With '/home' Actual results: whatever.mp3 is not included in the playlist. Expected results: whatever.mp3 should be included in the playlist. Does this happen every time? Yes. Other information: Steps to reproduce: 1. Have a song in /home/user/Music/Rock and Roll/dang dang.mp3 2. Create a smartplaylist with filter File Location Contains 'Rock and Roll" Actual results: dang dang.mp3 is not included in the playlist. Expected results: dang dang.mp3 should be included in the playlist.
Working on this. I just don't have the bugzilla rights to set to ASSIGNED.
Ok this is going to be tricky. It turns out that the file:///home/.../Hello%20World.mp3 format is not always used. For songs inside /home/user/Music, a normal string is used. We need to uniform this.
Created attachment 128760 [details] [review] proposed patch v1 Diff against r5038 (includes ChangeLog). This fixes FileLocation search when files are outside /home/user/Music and the search contains characters affected by escaping (for example a space). Please review, thanks.
Don't review last patch, consider it obsolete because this bug depends on bug 570312, which I reopened recently. I left there some patches.
Marking patch as obsolete as per above comment.
Created attachment 129700 [details] [review] Proposed patch v2 Ok, new version after the bug on which this depends is marked FIXED again. Beware: the only file that doesn't apply cleanly in this patch is the DB Migrator, because I'm assuming the previous patch I've proposed for bug 573484 is going to be committed as well (so the DB version will be 26, not 25). It includes unit tests and ChangeLog entry as well.
Hugh, I just realised there's a mistake, I'm creating a new version...
(In reply to comment #7) > Hugh, I just realised there's a mistake, I'm creating a new version... Disregard this comment, the patch is fine.
Gabriel has told me on IRC that he has integrated the latest patch in his git branch: http://banshee-project.org/~gburt/banshee.git I'll check it out and post a new patch for the remaining cases we need to cover in this bug.
I've attached the patch to bug #404827, as it's part of adding per-library import folders.
(In reply to comment #10) > I've attached the patch to bug #404827, as it's part of adding per-library > import folders. Hey Gabriel, I don't see any part of the patch in that patch. Maybe what you meant is that the patch here will be obsolete when that patch lands? I haven't tested it yet though.
(In reply to comment #11) > (In reply to comment #10) > > I've attached the patch to bug #404827, as it's part of adding per-library > > import folders. > > Hey Gabriel, I don't see any part of the patch in that patch. Maybe what you > meant is that the patch here will be obsolete when that patch lands? I haven't > tested it yet though. > I've seen the commit now, and I confirm this bug is now fixed for *all* the cases. I wonder if you didn't like my patch in the first place because you were thinking it was worth it to use absolute URIs for all cases in the end: well, indeed, we already had talked about this in IRC and I was advocating that approach, but you told me it had to have a reason so, that's why I was adding this patch to fix some cases, and later I was going to add a second patch (which depended on this one) to fix the rest of them. The 1st was changing the encoding format and the second would have changed to all-absolute-URIs because there was still a use case which was broken with the some-absolute-some-relative-and-with-different-encoding approach that we had in trunk. So please, next time, let's talk about this (more), so I don't waste time on patches that are going to become obsolete. Thanks.
*** Bug 548128 has been marked as a duplicate of this bug. ***
*** Bug 560934 has been marked as a duplicate of this bug. ***