Bugzilla – Bug 3703
Artwork View no longer supports removal of "placeholders"
Last modified: 2009-09-08 09:21:14 UTC
With the new GD library handling the resizing, there is a section of code in processCoverArtRequest() (lines 69-75 of Web::Graphics) that respond with the cover.png image when no valid artwork image data is found. The templates for artwork all look for item.coverThumb, but this is always valid since it is the track ID associated with the artwork to look up. I can't find any way to let a template know that the image object has failed. If I block the insertion of cover.png, then the page simply renders the alt text it it would for any invalid image data. My instinct at this point is to remove the preference in server settings->interface to allow or delete placeholders. As the gallery view is now just a different layout for the album list, it makes sense to always include all albums, even if the cover art isn't found. Users can always have the option of not using the gallery view or adding artwork for each of the missing covers. However, I'm creating this bug in order to leave the issue open for feedback, or ideas.
I think the best thing to do here would be to remove the preference. Also, I'd like to see, at least in the default skin, no placeholder images for albums with missing cover art. If a skin author wants to add a placeholder image, then they should be able to do that in the template. (Dan thinks it may be necessary to propagate an additional parameter to the templates that indicates whether a cover art image is available.
that used to be possible, but the server was changed to fallback to a placehodler image when a requested image fails to load (svn2205 in response to requests from CLI crew to always have an image in response). Eventually we lost ability for the server to know if an image worked or not (probably db optimisation to use the track id instead of cached filename, my best guess), including taking away the ability to request reduce the bandwidth by requesting a placeholder url instead of sending the placeholder data each time (bug 2778). The result is pushed to the output buffer without any ability to input it into the template becuase it is simply binary data where binary data is expected. As far as I can tell, all of the old framework for tracking the filenames of the images and tracking vailidity is long gone. Checking now woudl seem to require checking for an image ahead of time in order to set/clear the param, while retriving the data a second time when feeding it into the $outbuf. I think that if we do go through the effort of adding a mechanism to report if an image exists to the template, then there is no need to delete the param, as users can then easily decide for themselves if a skin should include placeholders or not. That function is already there for free, and they will be used to it. If I have a flag for a non-image, then I can very easily make this work. right now, processCoverArtRequest is called from generateHTTPResponse. The data is sent to: prepareResponseForSending($client, $params, $body, $httpClient, $response); which results in the call: addHTTPResponse($httpClient, $response, $body); note that the $params are now gone, as the only thing addHTTPResponse does now is to pipe the image data into the http stream. if I'm wrong, please let me know where I can wedge in a param.
Particularly if you display album/artist text below the artwork image, the preference to suppress placeholders doesn't really make a lot of sense. Why skip albums in a view just because there's no artwork? Personally, I prefer the placeholder image in the artwork view because it keeps the text below the artwork aligned and makes it much easier to read and browse. If this image is eliminated, then the html & CSS should try to keep the text vertically aligned on a row. You could always just use a transparent gif to accomplish this, but perhaps more visually appealing would be a simple transparent gif with a 1 pixel border.
So the artwork post-processing is back, as it's needed for correctness (damn infoFormat code). However, this bug is invalid, as we don't want to remove placeholders from the list because the gallery view is just a different view on Albums.