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

Photon Image CDN doesn't work well with blocks #35005

Closed
asafm7 opened this issue Jan 13, 2024 · 7 comments
Closed

Photon Image CDN doesn't work well with blocks #35005

asafm7 opened this issue Jan 13, 2024 · 7 comments
Labels
[Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal [Status] Priority Review Triggered The guild in charge of triage has been notified of this issue in Slack Triaged [Type] Bug When a feature is broken and / or not performing as intended

Comments

@asafm7
Copy link

asafm7 commented Jan 13, 2024

Impacted plugin

Jetpack

Quick summary

In some (or most) cases, image sizes aren't being passed by Gutenberg in a way that can be utilized by Jetpack image CDN.

Steps to reproduce

I couldn't find a similar ticket. I hope it isn't a duplicate.

I believe there are a few issues with Jetpack and Gutenberg:

  • Giving explicit dimensions to an Image block doesn't add explicit width and height attributes to be utilized by Jetpack (just an inline style attribute).
  • If I'm correct, when choosing a "Resolution" in the Block Editor the width and height attributes are being ignored by Jetpack. This makes things even more complicated and confusing.
  • A Gallery block doesn't allow to pass any sort of explicit sizing.

Notes:

  • I'm using add_filter('jetpack_photon_noresize_mode', '__return_true'); which might make things a bit more complicated.
  • I think this also makes the Image CDN ignore the featured image Cover block (maybe because the post-thumbnail size used by the block isn't actually registered).
  • I contacted the Gutenberg team regarding adding explicit width and height attributes but could not convince them it is important (I believe it is a good practice, regardless of Jetpack).

I assume most of the adjustments will need to be made on the Gutenberg side. But as I tried and failed to motivate them, I thought you might have a better chance.

A clear and concise description of what you expected to happen.

No response

What actually happened

No response

Impact

Most (> 50%)

Available workarounds?

No but the platform is still usable

Platform (Simple and/or Atomic)

No response

Logs or notes

No response

@asafm7 asafm7 added [Type] Bug When a feature is broken and / or not performing as intended Needs triage Ticket needs to be triaged labels Jan 13, 2024
@github-actions github-actions bot added [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] High [Status] Priority Review Triggered The guild in charge of triage has been notified of this issue in Slack labels Jan 13, 2024
@asafm7
Copy link
Author

asafm7 commented Jan 14, 2024

I think WooCommerce product image, and post featured image also don't work well with Jetpcak when in a loop.

@jeherve jeherve added [Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins [Pri] Normal [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! Triaged and removed [Pri] High Needs triage Ticket needs to be triaged labels Jan 15, 2024
@matticbot matticbot moved this from Needs Triage to Triaged in Automattic Prioritization: The One Board ™ Jan 15, 2024
@jeherve
Copy link
Member

jeherve commented Jan 15, 2024

I cannot seem to be able to reproduce the issue on my end. The width and height attributes are properly added, as well as the srcset attributes, whether I rely on the jetpack_photon_noresize_mode filter or not.

Does this happen with specific image blocks on your end, or with all of them?

@asafm7
Copy link
Author

asafm7 commented Jan 15, 2024

Thanks @jeherve

I'm pretty sure Gutenberg doesn't add HTML width and height.

I've verified it many times. Also, it is an open Gutenberg Github issue: WordPress/gutenberg#23244

Gutenberg only sets width and height as inline CSS.

Maybe what you see is something Jetpack is doing retroactively - copying CSS properties into HTML attributes, with JavaScript:
https://github.com/Automattic/jetpack/blob/25418db817d839c81a18962c8a52f0a68164ac30/projects/packages/image-cdn/src/js/image-cdn.js#L16C2-L16C2

But this happens too late, and doesn't affect the transform params on the Jetpack URL:

image

This is the original HTML from the browser's page source. You can see the original width and height, which match the URL:

image

The 800 width is set as a fallback, as I use the jetpack_content_width filter. But it isn't the width set on the editor for this image (which is 512).

(BTW, I'm not sure I understand the purpose of changing the HTML attributes so late. It seems to be more confusing than helpful. It is probably too late to help with the Cumulative Layout Shift).

Let me know if I missed something.

And to answer your question, it happens to me with all image blocks. Actually, all image-related blocks as well, such as galleries and covers with images.

@jeherve
Copy link
Member

jeherve commented Jan 15, 2024

I'm pretty sure Gutenberg doesn't add HTML width and height.

It does, but only in certain scenarios. That's indeed the related issue where this has been discussed quite a bit in the past few years, and I believe that's also what is going the problem on your screenshot.

doesn't affect the transform params on the Jetpack URL

On my end the parameters match when using the Image block, because the image block does pass info about the image size:

Screenshot 2024-01-15 at 13 18 37

However, they do not match when using the Text & Media block, like on your own screenshot, because in that block there is no additional size information passed from the block, the resizing happens with CSS:

image

This was discussed a bit in this issue:

WordPress/gutenberg#29362

I'm afraid there isn't much we can do to change that in Jetpack itself, since we can only infer size from the information that is available in the markup.

I'll consequently close this issue.

@jeherve jeherve closed this as completed Jan 15, 2024
@github-actions github-actions bot removed the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Jan 15, 2024
@asafm7
Copy link
Author

asafm7 commented Jan 15, 2024

Thanks @jeherve

Just the better understand the case, about your first screenshot, is it possible you are using one of the built-in resolutions, rather than a custom width and height?

image

@jeherve
Copy link
Member

jeherve commented Jan 15, 2024

is it possible you are using one of the built-in resolutions, rather than a custom width and height?

Yup, I was using one of the built-in resolutions. If you use a custom width and height, resizing happens via CSS.

@asafm7
Copy link
Author

asafm7 commented Jan 15, 2024

Yes, so Gutenberg never passes custom width and height as HTML attributes. Hopefully, they will change it in the future. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal [Status] Priority Review Triggered The guild in charge of triage has been notified of this issue in Slack Triaged [Type] Bug When a feature is broken and / or not performing as intended
Development

No branches or pull requests

2 participants