WebP support - ImageAPI Optimize WebP

There's a patch so updating status.

Postponing this on the other issue.

Patch fails to apply after a change in this commit for D9 support.

I am leaving this marked Postponed per #4, but I am attaching a re-rolled patch with an interdiff for the change I made to get it to keep it in line with the D9 changes.

Note that WebP support just landed in core See #2340699: Let GDToolkit support WEBP image format.

It would be nice if we could hard-code the webp support in the meantime until #3173364: Provide a hook so modules can provide more "derived-from" candidates lands

Since there hasn't been much progress on 2340699 it seems like a fine idea to add this support here.

I wonder what module would theoretically provide the webp hook even once 2340699 lands. Maybe stage_file_proxy would which seems like it adds to the argument to add this support here for now.

@mherchel @markdorison are you vouching for this patch? Is it RTBC in your perspective?

Looking at the first comment:

> has been updated to work with the ImageAPI Optimize WebP module instead.

It seems like we should ideally support whatever are the most common and support several of them (i.e. both vs instead).

@markdorison are you vouching for this patch? Is it RTBC in your perspective?

@greggles We have been using the #5 patch on one of our sites since September 2020 without issue, so from our perspective this is RTBC.

I haven't tested out the patch yet. Note that I'm currently using the regular Webp module.

I'll make a point to test this out in the coming days.

It's not working for me.

• I applied the patch via composer, and verified that the patch did apply successfully.
• I'm using the WebP module (as opposed to ImageAPI Optimize WebP )

Below are the errors that I'm seeing. What am I doing wrong?

The webp module converts image derivatives only, so the code that tries to grab the original image should be skipped if we are dealing with a webp image. Here's a simple check to skip it, which seems to make it work in my testing.

Thanks @marcoscano!

Just tested this out and it works wonderfully!

Hoping to get this committed but I have one question:

+++ b/src/EventSubscriber/ProxySubscriber.php
@@ -139,49 +139,60 @@ class ProxySubscriber implements EventSubscriberInterface {
+        exit;

Why are we calling exit; in this case? Is that intentional?

Why are we calling exit; in this case? Is that intentional?

We are not really changing it here, are we? The git diff makes it look more complex than it should, but this chunk is already part of the original code: https://git.drupalcode.org/project/stage_file_proxy/-/blob/8.x-1.x/src/E...

@marcoscano You are correct. Thanks for pointing that out!

patch #12 doesn't work for me, added a small fix

Patch #12 worked for me against 8.x-1.1
Patch# could not be applied.

I made https://www.drupal.org/project/stage_file_proxy/releases/8.x-1.2 that includes this code so folks don't have to mess with patch files on the stable release.

Patch #19 seems like it could be problematic since it looks for .webp anywhere in the string and is case sensitive (but I don't use webp a ton, so not sure).

This doesn't work for me with imageapi_optimize_webp enabled. This seems to be due to it skipping the download of original files in this block:

      if ($original_path && !$is_webp) {
        if (file_exists($original_path)) {
          // Imagecache can generate it without our help.
          return;
        }
        if ($config->get('use_imagecache_root')) {
          // Config says: Fetch the original.
          $fetch_path = StreamWrapperManager::getTarget($original_path);
        }
      }

This seems to be because the remote server hasn't yet generated it's image style from the original. I'm not sure why this isn't triggering the remote to generate it's imagecache, but either way it seems strange not to honour the use_imagecache_root setting.

Example:

• Incoming Request: https://dev.example.com/sites/default/files/styles/thumbnail/public/some... [404 -> Proxied]
• Proxied from origin: https://example.com/sites/default/files/styles/thumbnail/public/someimag... [404]
• Proxied from origin: https://example.com/sites/default/files/styles/thumbnail/public/someimag... [404]
• Original (not attempted): https://example.com/sites/default/files/someimage.jpg [200]

I had a similar problem as @mattjones86 and the patch in #19 resolved it for me. It's possible that should be reconsidered.

I'm using Drupal 9.3.14's built in webp image style and all my images are generated as "image_name.jpg.webp", or the equivalent, and I've tried the latest stable release and the dev release with this patch and I can't get it to work. I notice in the patch it was looking for a different pattern so I adjusted, but still couldn't get it work.

Thanks!

I was experiencing infinite 302 redirection loops, my settings are "imagecache root" enabled / "hotlink" disabled.

Patch #19 solved the issue for me too.

[#417107-21] is basically the same thing.

Patch #19 working for me too Drupal 9.4.7 after drush if --all

@couloir007 have you tried flushing image cache

Patch failes to install with version 1.3 and 1.4

In latest version 2.0.2 for Drupal 10, this patch/fix did not work properly anymore.

Hereby a rerolled patch which should apply to 2.0.2 and fix the issue.