Skip to content

Conversation

@patrickfournier
Copy link
Collaborator

@patrickfournier patrickfournier commented Nov 2, 2025

This pull request enhances the image_process plugin by updating image processing logic for non-responsive images to automatically add width and height attributes to <img> tags based on the processed image actual dimensions. The tests have been updated to reflect this new behavior, ensuring that images without pre-existing size attributes receive them, while existing attributes remain unchanged.

Having width and height attributes helps the browsers to compute the page layout more quickly, since it does not have to wait for the images to be loaded to know their size.

Image processing logic improvements:

  • The process_image function now returns the processed image's width and height, and process_img_tag uses these values to set width and height attributes on <img> tags if they are not already present.

Test suite updates:

  • Test cases in test_image_process.py have been updated to expect width and height attributes in processed <img> tags, and mocks for process_image now return fake image dimensions.
  • Test test_path_normalization now use BeautifulSoup to parse and assert on individual tag attributes, making them more robust to attribute order and formatting.

For images with a single source, add the width and height attributes to the img tag in order to help browsers compute the page layout more quickly.

If the width and height attributes were already present on the img tag, they are left untouched.
@patrickfournier patrickfournier changed the title Add width and height attributes on simple images. Add width and height attributes on non-responsive images. Nov 2, 2025
@justinmayer justinmayer changed the title Add width and height attributes on non-responsive images. feat: Add width and height attributes on non-responsive images Nov 2, 2025
Copy link
Contributor

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems sensible to me. Any thoughts, @minchinweb?

@minchinweb
Copy link
Member

minchinweb commented Nov 2, 2025

I'm just able to do a "desktop" (ok, phone) review at the moment, but it looks good. Thanks for the PR, and thanks for including tests!

I was vaguely aware that Exif dimensions could mis-match the "physical" dimensions, as it came up on Immich's list of "Cursed Knowledge", bit hadn't drawn the connection it could affect us here.

(For future reference,) Is there a reason not to add these dimensions to images that don't already have them?

Also, should add a note to our list of "known issues" that Exif and "physical" dimensions will drift of ExifTool isn't installed?

Edit: Oops, comment got added to the wrong issue!

process_image((path.source, destination, process), settings)
image_size = process_image((path.source, destination, process), settings)
if image_size:
if "width" not in img.attrs:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is adding a width attribute to the HTML <img> tag, right? (i.e. nothing to do with the Exif tags)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. Here, img is the HTML image tag parsed from the HTML generated from the user content.

@minchinweb
Copy link
Member

Look good to me!

I had to look at this on my computer to make sense of it, but I was mostly over-complicating things.

I added a doc string for the function, so it that looks good on your side, please merge.

@justinmayer Will you merge this one? Also, with these two, it's probably time to cut a release; are you up for that?

@justinmayer
Copy link
Contributor

Yes, I will issue a release tomorrow. Many thanks to @patrickfournier for the enhancement and to @minchinweb for the review and doc-string addition. Much appreciated! 💫

@justinmayer justinmayer merged commit b4ed367 into pelican-plugins:main Nov 3, 2025
12 checks passed
@justinmayer
Copy link
Contributor

Release issued 🎉 https://github.com/pelican-plugins/image-process/releases/tag/3.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants