rosell-dk / webp-express

Wordpress plugin for serving autogenerated WebP images instead of jpeg/png to browsers that supports WebP
GNU General Public License v3.0
227 stars 64 forks source link

WP Offload (S3/Cloudfront) - WebP's not being created #161

Open willstocks opened 5 years ago

willstocks commented 5 years ago

So, I have just enabled the plugin and set it to "Just Convert" and configured as follows: image

Example URL: https://cdn.willstocks.com/wp-content/uploads/2017/04/28102535/rubber-duck-development-debugging-problem-solving-100x67.jpg - working! Example URL: https://cdn.willstocks.com/wp-content/uploads/2017/04/28102535/rubber-duck-development-debugging-problem-solving-100x67.jpg.webp - not working

For various reasons, I keep the images I upload on my server as well as in S3/Cloudfront (fallback). So if I check a URL that is "local": https://willstocks.co.uk/wp-content/uploads/2017/04/rubber-duck-development-debugging-problem-solving-100x67.jpg - works! https://willstocks.co.uk/wp-content/uploads/2017/04/rubber-duck-development-debugging-problem-solving-100x67.jpg.webp - does not work 😞

Is there something I'm doing wrong here?

willstocks commented 5 years ago

I also tried setting to "Standard" and then "mingle" with "append .webp" but every time I save it switches back to "in separate folder". I've now managed to get this to stick on "mingle" but it's still not generating a webp variant? content-type is always image/jpeg

rohilv commented 5 years ago

having same issue. same setup. (Amazon cloudfront + standard + mingle) gets redirected to a blank page and when I go back to the plugin setting it switches back to "in separate folder"

rosell-dk commented 5 years ago

Yes, I also noticed there was a bug when switching to "mingled" in Standard mode. I have fixed it. The fix will be part of next release, which will be released in a few days.

However, in "Just convert" mode, it always stores images "mingled". And I see that you have enabled "auto convert". So upon requesting the jpeg the first time, the webp ought to be generated. I have that functionality working on my test setups, but obviously that feature is broken on your setup.

I doubt that it has something to do with the CDN. Note however that when using WebP Express in Standard mode with Cloudfront, you need to configure Cloudfront to forward the accept header. Go to Distribution settings, find the Behavior tab, select the Behavior and click the Edit button. Choose Whitelist from Forward Headers and then add the "Accept" header to the whitelist.

In the upcoming version there is a new option which triggers the generation when a missing webp is visited. I consider that approach much better than the current "auto generate" feature. Hopefully that feature will work for you.

willstocks commented 5 years ago

Yes, it is very strange indeed!

Is there any debugging I can enable to try and work out why the auto-conversion doesn’t happen? I would be happy to spin up an exact replica staging environment if that would help diagnose in any way?

Re: the Cloudfront config - I have it set to accept all headers. For various reasons, my whitelist grew to the point where I had more than half of the headers whitelisted, therefore I just whitelisted them all! How exactly does WebP Express push new images up to Cloudfront/S3? There’s no integration configured - is it just hooking into WP Offload if it exists?

On Mon, 4 Feb 2019 at 19:31, Bjørn Rosell notifications@github.com wrote:

Yes, I also noticed there was a bug when switching to "mingled" in Standard mode. I have fixed it. The fix will be part of next release, which will be released in a few days.

However, in "Just convert" mode, it always stores images "mingled". And I see that you have enabled "auto convert". So upon requesting the jpeg the first time, the webp ought to be generated. I have that functionality working on my test setups, but obviously that feature is broken on your setup.

I doubt that it has something to do with the CDN. Note however that when using WebP Express in Standard mode with Cloudfront, you need to configure Cloudfront to forward the accept header. Go to Distribution settings, find the Behavior tab, select the Behavior and click the Edit button. Choose Whitelist from Forward Headers and then add the "Accept" header to the whitelist.

In the upcoming version there is a new option which triggers the generation when a missing webp is visited. I consider that approach much better than the current "auto generate" feature. Hopefully that feature will work for you.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rosell-dk/webp-express/issues/161#issuecomment-460380186, or mute the thread https://github.com/notifications/unsubscribe-auth/Apt9AvIpbinDLMh8UyOsHK2WG8mQZ1_9ks5vKIp3gaJpZM4aeYx2 .

rosell-dk commented 5 years ago

Ah, sorry, I did don't know WP Offload, and it didn't strike me that it was a plugin. Is it this one you are using: https://wordpress.org/plugins/amazon-s3-and-cloudfront/ ?

willstocks commented 5 years ago

That's the plugin I'm using - how does WebP Express currently handle Cloudfront?

willstocks commented 5 years ago

Hey @rosell-dk!

I noticed there have been a few updates - I’ve not yet had a chance to update the plugin on my site (had a few other projects on the go in recent weeks) - will WebP Express now play nice with generating and storing images saved to Cloudfront via S3 Offload?

willstocks commented 5 years ago

Per my comment on #179:

OK, so images do seem to convert (as long as I call them directly at my server) - now I just need to figure out how to get around my "domain-sharding" issue, where I'm uploading the images directly to AWS S3 and then serving them from there (via Cloudfront) via WP Offload Media (previously "S3 Offload") - https://en-gb.wordpress.org/plugins/amazon-s3-and-cloudfront/.

There are three issues I can see at the moment:

  1. When visiting my site, the files are never called directly from my server. They're always called directly from the Cloudfront URL I'm using. Therefore, the webp conversion never happens. i. I have one thought here - could WebP Express hook into media upload and do the conversion upon a new image being uploaded, rather than waiting for the image to be called via a page visit?
  2. When WebP Express DOES convert the images to .webp, S3 Offload doesn't trigger/realize this and upload them to AWS. i. I believe there might be a way for me to hook into S3 Offload once WebP Express has generated and stored the WebP image. I might be able to find something in their documentation? See: https://github.com/deliciousbrains/wp-amazon-s3-and-cloudfront-tweaks
  3. Rewriting the URL's... so at the moment they serve from https://cdn.willstocks.com, NOT my primary site domain (https://willstocks.co.uk) and I haven't done anything relating to WP_CONTENT_URL because it's all handled via S3 Offload. I'm not sure what would happen if I changed the URL, as it's not only the uploads that would be affected, it would be everything? And I don't serve everything in wp-content from my CDN. Therefore, I'm wondering if there's a way that I can: i. check for images on *.willstocks.com/_file path_.jpg ii. check whether a webp exists at the same _file path_.jpg locally iii. If it does, change the output url file path to .webp rather than .jpeg or .png

Point 3 - there might be an even simpler method to this to be honest, based on S3 Offload's documentation?

Apologies - I know this adds quite a lot of work/complexity to WebP Express. However, I do feel it would be useful to support, as S3 Offload has fairly decent/wide usage. It's at over 50,000 installs at the moment and always growing!

willstocks commented 5 years ago

@rosell-dk - I raised a support question with WP Offload with regards to this - please see: https://wordpress.org/support/topic/question-is-there-a-way-to-trigger-upload-not-via-media/

willstocks commented 5 years ago

Don't suppose you've had any thoughts about this one @rosell-dk

rosell-dk commented 5 years ago

Sorry for not responding to you. I'm a bit overwhelmed by the number of support questions and currently only get to answer half of them. These tend to be the ones that are easily answered and the ones that affects many. I'm currently focused on finishing the next version of webp-convert, the "motor" that drives WebP Express. There are some valuable improvements I made a while ago, but I got sidetracked with implementing feature requests for WebP Express. But I'm back on track with finalizing webp-convert 2.0, and eager to get that next release ready. But as it is a new major version, I need to make sure I get the API exactly right. And as I have been doing lots of refactoring, I also need to do extended testing, and should not be rushing things.

Anyway, it just occurred to me that maybe the additions of "Bulk Convert" and "Convert upon upload" features in the 0.13 release is all you need?

willstocks commented 5 years ago

Thanks @rosell-dk - you may be correct actually... the bulk convert might be the silver bullet!

The only thing I can think is - obviously WebP Express is doing the rewrites/HTML altering based on local images being served from my server, not images that are being stored on my/a CDN... is there any way around this?

rosell-dk commented 5 years ago

Usually one have the CDN mirroring local files. Sorry for not having taken time to understand what you are trying to achieve. But are you trying to avoid storing the webp files on your server? - So it is only on the CDN? Actually, this could be implemented for the rewrites without much effort, and I actually considered such an option a while ago. What I have in mind will not work for HTML alterering, though.

What is needed is simply to have the webp-on-demand.php delete the generated webp file after it is served. Implementing this requires just one line of code (but implementing a setting for it requires more coding).

Here is why that would work: With a pull zone setup and a plugin that alters html such that images are pointed to the CDN, the CDN will get the request and then contact the origin server (your server). And then the CDN will cache it for as long as you have set it up to. So, in that case, there is no need for the image to reside on your server. When the cache expires, your server will be contacted again, WebP Express will regenerate the webp, serve it to the CDN, and delete it again. Of course, your CDN must be able to handle varied image responses for this idea to work.

rosell-dk commented 5 years ago

Btw: In 0.13 you can easily manually delete all the webp files on your local server - there is a new button for that.

rosell-dk commented 5 years ago

The single line that would be needed is:

@unlink($destination)

– And you would put that line as the last line in wp-content/plugins/webp-express/wod/webp-on-demand.php

willstocks commented 5 years ago

Usually one have the CDN mirroring local files. Sorry for not having taken time to understand what you are trying to achieve. But are you trying to avoid storing the webp files on your server?

My CDN is setup to mirror (that way, in the event of CDN failure I can fallback to local assets!)

I'm using AWS S3 + Cloudfront, which is Origin Push, not Pull - so when an asset is requested via Cloudfront, it does not hit my server at any point.

willstocks commented 5 years ago

TL;DR - WP Offload basically amends URL's to content that is stored in S3/Cloudfront via something along the lines of the following:

There are a couple of functions that check img src and content: protected function get_urls_from_img_src (\classes\as3cf-filter.php L317) and protected function get_urls_from_content (\classes\as3cf-filter.php L386)

Which then calls:

protected function url_needs_replacing( $url ) {
    if ( str_replace( $this->get_bare_upload_base_urls(), '', $url ) === $url ) {
        // Remote URL, no replacement needed
        return false;
    }

    // Local URL, perform replacement
    return true;
}

/**
 * Get an array of bare base_urls that can be used for uploaded items.
 *
 * @return array
 */
private function get_bare_upload_base_urls() {
    $base_urls = array();

    $uploads  = wp_upload_dir();
    $base_url = $this->as3cf->maybe_fix_local_subsite_url( $uploads['baseurl'] );
    $base_url = AS3CF_Utils::remove_scheme( $base_url );
    $domain   = AS3CF_Utils::parse_url( $uploads['baseurl'], PHP_URL_HOST );

    /**
     * Allow alteration of the local domains that can be matched on.
     *
     * @param array $domains
     */
    $domains = apply_filters( 'as3cf_local_domains', (array) $domain );

    if ( ! empty( $domains ) ) {
        foreach ( array_unique( $domains ) as $match_domain ) {
            $base_urls[] = substr_replace( $base_url, $match_domain, 2, strlen( $domain ) );
        }
    }

    return $base_urls;
}

So ultimately, they're already replacing the URL in the content with the CDN base URL that's configured for us in their plugin.

Because that's happening, at no point are image requests being served from my server 😞 therefore I can't control the rewrite via htaccess etc...

Spence1115 commented 5 years ago

@willstocks-tech just a quick thing, did you manage to get anywhere with this? I've got a similar setup and am trying to get the images to upload to the S3 server after conversion (via both bulk convert and convert on upload). If I can do that, then I'm fine, but struggling to get that to work.

willstocks commented 5 years ago

@Spence1115 No, I haven't managed to get it working yet. I did log an issue with the guys at WP Offload (see here). I think I got the webp conversion to work and upload to S3, but rewriting the URL in my content where an image did exist as a webp I couldn't get sorted

In summary they said:

There’s a few ways this could be accomplished.

Ideally the plugin you’re using to generate those WebP files would properly register the thumbnail sizes and then ensure that wp_update_attachment_metadata is called with the correct relative file paths in the data. WP Offload Media automatically handles re-offloading a Media Library item and its thumbnail sizes when they’re updated.

Otherwise you could register those sizes and trigger the above yourself.

Another option (and probably easiest) is to use WP Offload Media’s as3cf_attachment_file_paths filter to specify that there’s more files to be offloaded than those in the attachment’s metadata. Check out the Tweaks plugin…

https://github.com/deliciousbrains/wp-amazon-s3-and-cloudfront-tweaks/blob/cb7c645d84969d58b43dd43b918c6c5c75218099/amazon-s3-and-cloudfront-tweaks.php#L396

You could then trigger the re-offload via wp_update_attachment_metadata as necessary, but don’t need to adjust the data.

Hope that helps.

Spence1115 commented 5 years ago

I think I got the webp conversion to work and upload to S3, but rewriting the URL in my content where an image did exist as a webp I couldn't get sorted

That'd be enough for me - my front end isn't wordpress, using it headless, so I've got react plugins to fetch the .webp version when compatible and that works fine locally.

Any pointers on how you got the conversion uploading to S3?

willstocks commented 5 years ago

I will have to dig out my old code for you @Spence1115 😄

I believe I got it working after 0.13 was released, which included the bulk convert option as well as the "convert on upload" feature (@rosell-dk?).

I'll see if I can dig out any tweaks I had to make to get it uploading to S3 via WP Offload.

juizmill commented 5 years ago

In my case I am not able to send the .webp files to S3, the plugin creates in the local folder but it is not sent, only the.jpg, png, gif files are sent. Could anyone help?

xiekevin commented 5 years ago

@juizmill Here's what I did to send the .webp files to S3. I assume you're using both WebP Express and WP Offset.

I was able to get WP Offload to upload newly added media files to S3 by installing the tweaks plugin (https://github.com/deliciousbrains/wp-amazon-s3-and-cloudfront-tweaks) and adjusting the functions object_meta and attachment_file_paths inside amazon-s3-and-cloudfront-tweaks.php.

I uncommented these two lines in amazon-s3-and-cloudfront-tweaks.php:

add_filter( 'as3cf_object_meta', array( $this, 'object_meta' ), 10, 4 );
add_filter( 'as3cf_attachment_file_paths', array( $this, 'attachment_file_paths' ), 10, 3 );

Defined function object_meta as:

  function object_meta( $args, $post_id, $image_size, $copy ) {
    // This sets the ContentType to image/webp so that uploads to S3 pass verification
    $extension = pathinfo( $args['Key'], PATHINFO_EXTENSION );
    if ( in_array( $extension, array( 'webp' ) ) ) {
      $args['ContentType'] = 'image/webp';
    }

    return $args;
  }

Defined function attachment_file_paths as:

  function attachment_file_paths( $paths, $attachment_id, $meta ) {
    // This adds webP files to be uploaded as well
    foreach ( $paths as $file ) {
      $pathinfo   = pathinfo( $file );
      // Change this to the path of your converted webp file
      $extra_file = $pathinfo['dirname'] . '/' . $pathinfo['filename'] . '.' . $pathinfo['extension'] . '.' . 'webp';

      if ( file_exists( $extra_file ) ) {
        $paths[] = $extra_file;
      }
    }

    return $paths;
  }

Newly uploaded media files should now be uploaded in both .jpg or .png and .webp.

willstocks commented 5 years ago

@xiekevin I know we've briefly spoken about this on the WP Offload support req on the WP forum - just wanted to shout and say thanks for this, it's great!!! 😁

How are you then handling the rewriting of the HTML markup for in-content images? Are you using WebP Express' built-in functionality?

xiekevin commented 5 years ago

Hi @willstocks-tech! I am not using WebP Express' built in functionality and wrote a custom plugin (so that we can maintain that component with more control), but I think it is not possible to do the HTML markup replacement for our setup with WebP natively. You can try and select the options 'Replace with Image URLs, Only do the replacements in webp enabled browsers, and Reference webps that hasn't been converted yet' but I think it won't replace CDN links as it won't recognize them as valid image URLs to replace.

Our plugin just checks for any link with our CDN domain in it that ends in jpg, png, jpeg, in the src and srcset attributes and appends a .webp to the end for any requests that has image/webp in the Accept Header. I largely borrowed how that plugin is structured by the way the WebP Express library is structured. I mainly just modified some conditions so that it recognizes our CDN links.

We're also using W3 Total Cache, so you'll have to monkey patch your caching plugin (if you're using one) to vary the cache based on Accept Header. For W3 Total Cache we changed PgCache.php and tacked on an extra variable based on whether or not the accept header contained image/webp to the cache key.

We also implemented some client side javascript to fallback to non .webp incase the .webp could not be found on the CDN for whatever reason.

willstocks commented 5 years ago

Ahhh OK thanks @xiekevin - That's the bit that has been annoying me so far (I gave up a couple of months back!).

In an ideal world, I really wanted to swap out the existing WP markup (plus I use lazysizes... so excuse that extra markup) of:

<img 
  class="blur-up lazyautosizes lazyloaded" 
  alt="My alt value here" 
  src=" https://cdn.mydomain.com/wp-content/uploads/...-100x124.jpg" 
  data-srcset="
    https://cdn.mydomain.com/wp-content/uploads/...-100x124.jpg 100w,     
    https://cdn.mydomain.com/wp-content/uploads/...-242x300.jpg 242w,     
    https://cdn.mydomain.com/wp-content/uploads/...-420x521.jpg 420w, 
    https://cdn.mydomain.com/wp-content/uploads/...-768x952.jpg 768w, 
    https://cdn.mydomain.com/wp-content/uploads/...-826x1024.jpg 826w, 
    https://cdn.mydomain.com/wp-content/uploads/...-840x1041.jpg 840w, 
    https://cdn.mydomain.com/wp-content/uploads/...-1130x1401.jpg 1130w, 
    https://cdn.mydomain.com/wp-content/uploads/.jpg 1136w" 
  data-sizes="auto" 
  sizes="690px" 
  srcset="
    https://cdn.mydomain.com/wp-content/uploads/...-100x124.jpg 100w,     
    https://cdn.mydomain.com/wp-content/uploads/...-242x300.jpg 242w,     
    https://cdn.mydomain.com/wp-content/uploads/...-420x521.jpg 420w, 
    https://cdn.mydomain.com/wp-content/uploads/...-768x952.jpg 768w, 
    https://cdn.mydomain.com/wp-content/uploads/...-826x1024.jpg 826w, 
    https://cdn.mydomain.com/wp-content/uploads/...-840x1041.jpg 840w, 
    https://cdn.mydomain.com/wp-content/uploads/...-1130x1401.jpg 1130w, 
    https://cdn.mydomain.com/wp-content/uploads/.jpg 1136w" 
>

into something along the lines of:

<picture class="blur-up lazyautosizes lazyload">
  <source type="image/webp" media="(max-width: 100px)" srcset="https://cdn.mydomain.com/wp-content/uploads/...-100x124.webp">
  <source type="image/jpeg" media="(max-width: 100px)" srcset="https://cdn.mydomain.com/wp-content/uploads/...-100x124.jpg">
  <source type="image/webp" media="(max-width: 242px)" srcset="https://cdn.mydomain.com/wp-content/uploads/...-242x300.webp">
  <source type="image/jpeg" media="(max-width: 242px)" srcset="https://cdn.mydomain.com/wp-content/uploads/...-242x300.jpg">
  <source type="image/webp" media="(min-width: 420px)" srcset="https://cdn.mydomain.com/wp-content/uploads/...-420x521.webp">
  <source type="image/jpeg" media="(min-width: 420px)" srcset="https://cdn.mydomain.com/wp-content/uploads/...-420x521.jpg">
  <!-- the rest of the responsive image sizing here -->
  <source type="image/webp" media="(min-width: 1136px)" srcset="https://cdn.mydomain.com/wp-content/uploads/.webp">
  <source type="image/jpeg" media="(min-width: 1136px)" srcset="https://cdn.mydomain.com/wp-content/uploads/.jpg">
  <!-- now a fallback <img> tag with only jpg's for where the browser doesn't support <picture> -->
  <img 
    class="blur-up lazyautosizes lazyload" 
    alt="My alt value here" 
    src=" https://cdn.mydomain.com/wp-content/uploads/...-100x124.jpg" 
    data-srcset="
      https://cdn.mydomain.com/wp-content/uploads/...-100x124.jpg 100w,     
      https://cdn.mydomain.com/wp-content/uploads/...-242x300.jpg 242w,     
      https://cdn.mydomain.com/wp-content/uploads/...-420x521.jpg 420w, 
      https://cdn.mydomain.com/wp-content/uploads/...-768x952.jpg 768w, 
      https://cdn.mydomain.com/wp-content/uploads/...-826x1024.jpg 826w, 
      https://cdn.mydomain.com/wp-content/uploads/...-840x1041.jpg 840w, 
      https://cdn.mydomain.com/wp-content/uploads/...-1130x1401.jpg 1130w, 
      https://cdn.mydomain.com/wp-content/uploads/.jpg 1136w" 
    data-sizes="auto" 
    sizes="690px" 
    srcset="
      https://cdn.mydomain.com/wp-content/uploads/...-100x124.jpg 100w,     
      https://cdn.mydomain.com/wp-content/uploads/...-242x300.jpg 242w,     
      https://cdn.mydomain.com/wp-content/uploads/...-420x521.jpg 420w, 
      https://cdn.mydomain.com/wp-content/uploads/...-768x952.jpg 768w, 
      https://cdn.mydomain.com/wp-content/uploads/...-826x1024.jpg 826w, 
      https://cdn.mydomain.com/wp-content/uploads/...-840x1041.jpg 840w, 
      https://cdn.mydomain.com/wp-content/uploads/...-1130x1401.jpg 1130w, 
      https://cdn.mydomain.com/wp-content/uploads/.jpg 1136w" 
  >
</picture>

So far I haven't found anything that handles this correctly - and as you've mentioned I wonder if W3TC is interfering in some way (I also use W3TC!). I wondered whether this could be handled via the WP Offload plugin as well as it's already doing the URL rewriting, but started to get way beyond my knowledge/capabilities when it comes to PHP!

xiekevin commented 5 years ago

I see, I think if you used some regex expressions matching on image tags you could get the job done although I totally understand the headache. That was the most difficult part for me as well and the replacement for us was much simpler. I think when you get down to it, it is easier than you'd imagine.

You just need to use a filter at the_content to modify the body and a couple other places. Let me dig through the code I used from WebP Express to help get that done in a little bit.

We are using lazy sizes too and since you're using the picture tag, W3TC shouldn't affect it since browsers that don't understand the webp formats inside the picture tag will just ignore them--so you're good on that front. You can also look into picturefill which polyfills the picture tag so that you can potentially eliminate the fallback img tag, although it is not perfect.

I don't think this can be done within the WP Offload plugin. I think to modify the HTML to what you'd like unfortunately you'll most likely have to invest in creating a custom plugin or find another plugin to help with the re-writing. You'll want the priority of the hooks that run from that plugin to be HIGH i.e. 99999 so that it runs last after WP Offload has swapped out regular links with CDN links.

willstocks commented 5 years ago

Lol, regex is a dark art in my mind 😉 I've never fully been able to get to grips with it in all honesty!

Let me dig through the code I used from WebP Express to help get that done in a little bit.

That would be incredibly kind of you and would provide a great starting point for me! Do you have an example of your new output to hand at all? Is it similar to what I was aiming for above (the <picture> element)?

I'm not concerned about polyfilling the picture tag to be honest as I'd include the original img tag within which would provide the necessary fallback to the .jpg image for unsupported browsers in theory. I'm in the fortunate position of only having to support modern browsers 😁

willstocks commented 5 years ago

@rosell-dk - would the "Alter HTML" functionality of WebP Express eventually handle cases like this? Subdomained assets?

I'm considering going down the simplest route of just rewriting my img tags to convert the .jpg to .webp (or just append, whichever is necessary) and then using webpjs for browsers that don't support webp. This seems like the simplest interim solution, while waiting for WordPress to eventually deliver the <picture> feature that has been in the works for quite a while (see here)!

willstocks commented 5 years ago

@rosell-dk - EWWW manage this in an interesting way, see: https://github.com/nosilver4u/ewww-image-optimizer/commit/66e3ebe39ccc98c5ad0df3f57381bf359edceaa5

zkingdesign commented 2 years ago

@xiekevin does the filter posted by you above still work? Thanks

GizzyZexx commented 1 year ago

@rosell-dk antes que nada gracias por crear ese plugin tan increíble.

Me gustaría saber, si hay alguna solución actualmente para conectar ambos plugins.

Gracias.