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

webp handling and storage adapters #21146

Open
1 task done
elijahsgh opened this issue Sep 27, 2024 · 7 comments
Open
1 task done

webp handling and storage adapters #21146

elijahsgh opened this issue Sep 27, 2024 · 7 comments

Comments

@elijahsgh
Copy link

Issue Summary

The newest templates expect to be able to serve images via format="webp"
This means the newest templates will NOT work with any sort of storage adapters other than local. To make matters worse, the local storage adapter seems to label everything as a .jpg when they are .webp (this was, at some point, for backward compatibility I believe).

The problem here seems to be that ghost only generates the webp images on request. Instead this should be moved to generating the actual webp (with .webp extension) during upload. From what I understand, this means the storage adapter would automatically write the new images at the request of Ghost.

The .jpg naming, I believe, should also be considered a bug since this is very odd and unexpected operation.

I am labeling this as a bug and not a feature request because at this time Ghost simply does not work correctly with any new template including templates like Source. All of your uploaded images, when using a storage adapter, will not be found.

Steps to Reproduce

  1. Add a storage adapter
  2. Upload an image
  3. Use a recent template
  4. Your images are broken

Ghost Version

5.0x

Node.js Version

any

How did you install Ghost?

ghost install

Database type

MySQL 8

Browser & OS version

Any

Relevant log / error output

No errors - simply 404s for the images.

Code of Conduct

  • I agree to be friendly and polite to people in this repository
@github-actions github-actions bot added the needs:triage [triage] this needs to be triaged by the Ghost team label Sep 27, 2024
@linear linear bot removed the needs:triage [triage] this needs to be triaged by the Ghost team label Oct 17, 2024
@allouis allouis self-assigned this Oct 23, 2024
@allouis
Copy link
Contributor

allouis commented Oct 23, 2024

Hey @elijahsgh Could you please expand a bit on what the issue is with Ghost? I'm not clear on what is Ghost doing incorrectly that doesn't allow these storage adapters to work.

As for the local storage adapter renaming to jpg, I believe this is working as intended - I'll check with the rest of the team and get back to you

@elijahsgh
Copy link
Author

If you use any storage adapter other than local you will not have a formats/webp directory. Modern themes attempt to serve images from that location.

It appears that when an image is uploaded the storage adapter is called to store the raw image and new image but it is NOT called to store the webp versions newer templates are serving. It seems the webps are created when the image is first retrieved from Ghost - which will never happen in the case of a storage adapter.

With a storage adapter the expectation is images will be served through, for example, a bucket serving all requests to /content so Ghost will never be triggered to generate the webp.

In addition to that, it seems like the webps in local storage are renamed to jpg but when I upload a PNG it keeps the .PNG extension. It's very confusing and unnecessary since we are past the point where this was required for compatibility with certain browsers.

With all of this going on if you add a storage adapter (S3, GCP, etc) modern templates WILL NOT work because they attempt to serve webp from fomats/webp which doesn't exist - so all of your uploaded article images are 404.

It seems to me to make more sense to have Ghost generate the webp at the same time that it generates the resolutions. The resolutions work as expected (calls the storage adapter save / saveraw). If the webps were generated at this time and used save/saveraw from the storage adapter then this issue would be resolved, I think.

@brvnnghi
Copy link

I'd like to follow this issue, with a different kind of upload error (video upload), but I guess the cause is the same.

Step to reproduce:

  1. Add a storage adapter
  2. Upload a video
  3. See 404 error, but preview / save anyway
  4. Video is still playable, file is uploaded, but no thumbnail

This does not happen with local storage. My guess is same with Elijah, if Ghost generate the thumbnail before uploading both (video & thumb) to storage adapter it should be fine.

It seems to me to make more sense to have Ghost generate the webp at the same time that it generates the resolutions. The resolutions work as expected (calls the storage adapter save / saveraw). If the webps were generated at this time and used save/saveraw from the storage adapter then this issue would be resolved, I think.

@allouis allouis removed their assignment Oct 28, 2024
Copy link
Member

kevinansfield commented Oct 28, 2024

It seems the webps are created when the image is first retrieved from Ghost - which will never happen in the case of a storage adapter

All resized/alternative format images in Ghost are generated at time of request, this is very much intentional because we don't know ahead-of-time which sizes a selected theme or the image's use within post content will require.

Are you able to share the storage adapter code that you are using? It sounds like perhaps it's missing some of the methods required for resized images to work fully, namely the saveRaw() method (LocalImagesStorage example).

Copy link
Member

kevinansfield commented Oct 28, 2024

With a storage adapter the expectation is images will be served through, for example, a bucket serving all requests to /content so Ghost will never be triggered to generate the webp.

This sounds like an incorrect assumption to me. Storage adapters are a means for Ghost to read/write to a storage layer. Requests for images, especially content/images/size/* which is a dynamic endpoint should always hit Ghost so that it can generate the requested size/format images.

Ghost can't know ahead-of-time what image sizes or formats the at-time-of-request loaded theme will use or specifics of how the image will be used within post content. As such, images are generated at time of request.

The only way to avoid image requests needing to go through Ghost entirely is to have your storage server handle the dynamic image generation itself by following the same URL format of /content/images/size/{wXhY}/format/{format}/{path}. Alternatively you can put a CDN in front of Ghost so it's served images get cached after that first image-creating request.

@kevinansfield kevinansfield added the P3 - Medium [triage] Bugs which we'll fix soon label Oct 28, 2024 — with Linear
@kevinansfield kevinansfield removed the P3 - Medium [triage] Bugs which we'll fix soon label Oct 28, 2024
Copy link
Member

@brvnnghi that sounds like a different issue, please open that separately so this issue can stay focused on the original topic. Thank you.

@elijahsgh
Copy link
Author

elijahsgh commented Oct 28, 2024

It seems the webps are created when the image is first retrieved from Ghost - which will never happen in the case of a storage adapter

All resized/alternative format images in Ghost are generated at time of request, this is very much intentional because we don't know ahead-of-time which sizes a selected theme or the image's use within post content will require.

Are you able to share the storage adapter code that you are using? It sounds like perhaps it's missing some of the methods required for resized images to work fully, namely the saveRaw() method (LocalImagesStorage example).

This is my storage adapter slightly tweaked from the one linked from the Ghost website:
https://github.com/elijahsgh/ghost-google-cloud-storage

It's not much different than the one literally advertised by Ghost as an integration that Ghost supports.

If you install this storage adapter, configure it, and set a path to the bucket then Ghost will never get image requests - which is what we want. This was fine until themes started including format="webp"

If there's an error with the storage adapter let me know and I'll fix it. Otherwise Ghost has a very serious bug because you cannot successfully use Ghost "out of the box" with a modern theme that includes format="webp" (which is almost all of them) with the storage adapter linked on the integrations page enabled.

I may be crazy or not noticing something in the editor that might fetch the images but it seems like during upload a lot of the resized image files are present in the bucket as soon as I upload. The dimensions for those images are based on whatever dimensions the theme has configured. The formats/webp stuff is never created.

Ghost can't know ahead-of-time what image sizes or formats the at-time-of-request loaded theme will use or specifics of how the image will be used within post content. As such, images are generated at time of request.

Can you explain this part a bit more, @kevinansfield ?
I don't see how that's working. If I use a theme with ex: width 780 and then change to one with width: 600, 900 (without the 780) then I also have missing images but only for posts that were created before I changed themes. What's going on there?

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

No branches or pull requests

4 participants