GNOME Bugzilla – Bug 549414
Cannot empty some ID3 tag fields
Last modified: 2009-03-10 05:30:12 UTC
Please describe the problem: Using the metadata editor, you cannot empty some fields. Steps to reproduce: 1. Open an MP3 file and rename their fields to: Album Artist: AA Track Artist: TA Album: A Title: T 2. Click Save: all changes saved successfully. 3. Now edit the metadata again, and empty all these fields. Actual results: Only the field "Title" is emptied. Expected results: All the fields should be made empty. Does this happen every time? Yes. Other information:
Created attachment 117453 [details] [review] A patch that just adds some tracing in order to find out the problem With this patch, if I edit an album tag to set the value "On Every Street" in place of the old value "Greatest Hits", I get this: ___________EditorTrack::Save()_________ track.AlbumTitle={Greatest Hits} AlbumTitle={On Every Street} ___________after assignment_________ track.AlbumTitle:{On Every Street} AlbumTitle:{On Every Street} However, if I change it to "" (string.Empty), I get this: ___________EditorTrack::Save()_________ track.AlbumTitle={Greatest Hits} AlbumTitle={} ___________after assignment_________ track.AlbumTitle:{Greatest Hits} AlbumTitle:{} So, the assignment doesn't take place! I've looked at the sources and I cannot even imagine how this may be happening. The only thing I could think of is AOP, but I doubt it's used on Banshee :-m
Created attachment 117460 [details] [review] Proposed patch Nevermind about last comment. This was a minor glitch in the DatabaseTrackInfo::CleanseString method. I would wonder if this was a feature instead of a bug but there are clear use cases for this bug to be resolver: a) User knows that one metadata field is incorrect (but doesn't know the correct one) so he wants to just leave it empty (for example, some dumb people just put the artist name in the album field). b) User prefers the field to be empty instead of containing useless information (for example "Greatest Hits", as opposed to having the original album). b) The user simply wants to clean the metadata.
Minor typos in last comment: s/resolver/resolved/ s/b) The/c) The/
The only side effect I could notice is that with your patch, if you put only spaces in a field, it will be stored as an empty string. Without the patch, the field is left unchanged. But I don't see anything wrong with the new behavior. So looks OK for me.
Please make patches from the parent of src/. I don't see the use-case for this really, but it seems fine (and will prevent those users w/ a use-case I can't think of from being confused when it doesn't work). Bertrand, would you commit?
Committed, thanks ! Andrés, a small request also : please give meaning full names to your patch files. I have lots of files in my patches directory, and I can't remember the 6 digits of bug number ;)
*** Bug 541456 has been marked as a duplicate of this bug. ***