Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect aspect-ratio set on inserted images #335

Closed
vonscriptor opened this issue Dec 10, 2024 · 5 comments
Closed

Incorrect aspect-ratio set on inserted images #335

vonscriptor opened this issue Dec 10, 2024 · 5 comments
Labels

Comments

@vonscriptor
Copy link

Description

It seems that inserted images will be given the generic 1200/1200 aspect ratio. On the initial insert of an image it appears to output the correct dimensions for the aspect ratio, but if you view source it'll show the generic aspect ratio values. The incorrect value doesn't seem to be updated until another save is made, like adding caption text to the image.

Steps to reproduce

  1. Insert an image, save entry, correct aspect ratio should be used.
  2. Edit the entry and add a caption to the image. An incorrect aspect ratio is applied (e.g. 1200/1200)

Additional info

  • Craft version: 5.5.5
  • PHP version: 8.2
  • Database driver & version:
  • Plugins & versions:
@i-just
Copy link
Contributor

i-just commented Dec 10, 2024

Hi, thanks for reaching out! It sounds like the default max image length that comes from the HTML Purifier (the default value is 1200).

You can change it via the HTML.MaxImgLength property. For example, setting "HTML.MaxImgLength": 6000 in the HTML Purifier json configuration file used by your CKEditor field will raise this limit to 6000px.

I hope this helps!

I’ll close this now, but feel free to reach out if you run into any further issues!

@i-just i-just closed this as completed Dec 10, 2024
@lindseydiloreto
Copy link

lindseydiloreto commented Dec 11, 2024

Hi @i-just, thanks for the clarification.

Would this be considered a breaking change? It only started happening after a recent update. Prior to that, I don't believe any aspect ratio was being added automatically.

Per our client, it seems to be forcing non-square image to appear as squares...

@vonscriptor
Copy link
Author

Hi @i-just, I appreciate the info—didn't know about that config setting!

IMO, that doesn't seem like the best way to handle the issue though as it becomes a guessing game on picking a MaxImgLength larger than what you assume might be uploaded. In any case, if the upload dimensions exceed the max then rather than just omitting the aspect-ratio value it provides an incorrect one.

@lindseydiloreto, an additional quick "fix" to avoid the issue is to set aspect-ratio: auto !important;, though not my preferred solve either.

Side note, because I think it could help others… I realized a CKEditor field can make use of the Assets > Default Transform to then truly limit the max dimensions. I can then create and apply an htmlpurifier config to match those max dimension taking the guessing game out of it. Unfortunately for me, I also discovered a bug with my web host's image transform plugin which doubles up on the transform parameters in the image URL and fails to return the image—hoping to get that fixed too. Otherwise, this is a powerful way to handle rich text uploads that I wish I realized sooner, haha!

I offer two potential ways to address the root issue of the incorrect aspect-ratio value:

  1. If the dimensions exceed the HTML Purifier HTML.MaxImgLength value then omit the aspect-ratio. However, this just avoids the issue and doesn't solve it.
  2. An aspect ratio value doesn't technically need to match the dimensions, instead it could be reduced to a value within the HTML Purifier default while maintaining an aspect-ratio value. E.g. if the image dimensions are 1920x1080 exceeding the default it could be reduced and set to 16/9.

@i-just
Copy link
Contributor

i-just commented Dec 12, 2024

According to the CKEditor’s documentation aspect-ratio was added in version 40.0.0 of the CKEditor 5. We updated the CKEditor plugin past v40.0.0 in version 3.7.0 and version 4 of the plugin has been using v40+ since the beginning.

Here’s a bit more info about why CKEditor 5 insists on using width, height and aspect-ratio attributes.

@lindseydiloreto, are the image dimensions (both width and height) above the HTML.MaxImgLength value? I can trigger what you describe if I set HTML.MaxImgLength to 500 and then upload an image that’s 1000x700. After saving the entry with CKEditor field, the editor will show that image with width and height set to 500, and therefore, the aspect-ratio will be set to 500/500. If I then choose to transform the image via a transform that restricts the width to 200px and keeps the height as auto, the image will show as a 200x140 rectangle.

@vonscriptor, you can disable HTML.MaxImgLength HTML Purifier setting by setting it to null if that’s what you want.

I hope this helps!

@twitcher07
Copy link

twitcher07 commented Jan 2, 2025

@vonscriptor @lindseydiloreto
I know this is kind of a work around and not an ideal solution (since it involves a plugin) but I will install the image resizer plugin and make sure the max width and height match the HTML.MaxImgLength value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants