Fix exception from gallery RSS with WebP images #3674
Conversation
|
It belatedly crosses my mind that perhaps, if the samplesite demo gallery is used throughout the automated tests, then perhaps its gallery ought to contain not just a placeholder WebP, but one example of every supported image type (jpg, webp, png, etc). This would help the automated tests to surface any problems in a highly-visible way (e.g. Presumably this would have exposed the WebP issue a long time ago? And including things like TIFF images here would be desirable, so we can assert they are also correctly handled.) However, the samplesite is not just test data. It is also prominently user-visible in many places. These uses are better suited to a demo that shows things working (e.g. TIFF images here would be a bad idea, many users would not be able to see them, depending on which browser they are using.) I realize I'm a newcomer to this project (although have been using it happily for years), so I feel foolish making disruptive suggestions, but: I would consider making the samplesite a separate thing from the test data. One can be optimized to look good, the other to be fragile. Initially, I would simply copy the samplesite into nikola/tests/data or somesuch, then modify the test data gallery to contain one of each image type. I'm happy to submit a PR. Thoughts welcome! :-) |
nikola/plugins/task/galleries.py
Outdated
| @@ -725,6 +725,9 @@ def gallery_rss(self, img_list, dest_img_list, img_titles, lang, permalink, outp | |||
| def make_url(url): | |||
| return urljoin(self.site.config['BASE_URL'], url.lstrip('/')) | |||
|
|
|||
| # WebP is not yet an official MIME type but we need to recognize them | |||
| mimetypes.add_type('image/webp', '.webp') | |||
There was a problem hiding this comment.
I’m not sure if this is the best placce for this, considering the enclosure feature may also trigger an exception. Perhaps a better place would be somewhere in nikola.py, in Nikola.__init__ so that the mimetype is always defined, even if we are not rendering a gallery RSS file.
There was a problem hiding this comment.
Hi. I can do this. Am AFK now but will try later.
I think this would add more complexity and maintenance burden than it’s worth. We might end up making a change that works with the test site, but breaks the demo site. Converting a sample image to PNG/GIF, and perhaps adding some simple SVG would be beneficial for both the demo site and tests. |
tartley
force-pushed
the
inject-webp-mimetype
branch
from
4a8f1bf to
29e1eb0
Compare
|
Be aware I force pushed this branch at this point, as mentioned in one of the above comments. |
|
Force pushes are fine. Can you try placing the mimetype registration in a more global place, as suggested in the review comment? |
I added this as a new checklist item to #3672 |
|
As far as I know, this PR is ready, and completes checklist item "Inject a hardcoded 'image/web' mimetype at runtime, before .guess_type() gets called" on #3671. Thank you for your attention. Sorry it's been so wordy for such a small change. |
WebP images in a gallery cause `nikola build` to fail with an unhandled AttributeError exception, as described in getnikola#3671. This happens because the MIME type of WebP images is not known, and the resulting None value causes the RSS generation code to choke. This PR fixes that, by augmenting Python's list of known mimetypes with a hardcoded WebP one, before we ask for the mimetype of each image in a gallery. This addresses the above issue's checklist item "*Inject a hardcoded 'image/web' mimetype at runtime, before .guess_type() gets called.*", but does not complete the issue.
Doing it early like this makes the added WebP mimetype available to the several other places we call things like 'mimetypes.guess_type'.
tartley
force-pushed
the
inject-webp-mimetype
branch
from
f3a4d5f to
c98b387
Compare
|
Let me know if the baseline test failure is something I can help with (the error says "maintainers only", so I'm assuming you want to be hands-on yourselves. Thanks! |
The webp was previously generated by exporting from GIMP. This didn't
copy many of the EXIF metadata fields from the replaced jpg. This caused
a problem in the next PR, as a new test for RSS content is sensitive to
items like the image creation time, which is read from the EXIF
DateTimeDigitized.
Maybe GIMP can be cajoled into preserving this EXIF data, but instead,
this time I did the conversion using command line tool 'cwebp' (from
apt package webp):
cwebp tesla_conducts_lg.jpg -metadata all -o tesla_conducts_lg.webp
Pull Request Checklist
Description
WebP images in a gallery cause
nikola buildto fail with an unhandled AttributeError exception, as described in #3671. This happens because the MIME type of WebP images is not known, and the resulting None value causes the RSS generation code to choke.This PR fixes that, by augmenting Python's list of known mimetypes with a hardcoded WebP one, before we ask for the mimetype of each image in a gallery.
I don't think the content of the generated gallery RSS feed is tested yet. In the issue comments, I talk about adding a new test for that. But instead, I noticed that converting one of the images in the samplesite demo gallery into a WebP causes the site build to fail during the test suite, due to the above problem, causing many test failures. Adding this fix to the galleries.py file fixes all those tests again.
Also, running a manual build on a site containing webp galleries now works.
This addresses the above issue's checklist item "Inject a hardcoded 'image/web' mimetype at runtime, before .guess_type() gets called.", but does not complete the issue.