The current usage requires configuring the web server via .htaccess, which isn't very portable (many hosting providers are using nginx nowadays). WebP can also be used without web server configuration using the <picture> element (see https://css-tricks.com/using-webp-images/#article-header-id-3), which is the same mechanism used by the responsive_image module provided in core.

Here's a patch that works in combination with the responsive_image module. Each <source> entry is duplicated with a copy pointing to the webp URL.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

floretan created an issue. See original summary.

floretan’s picture

Updated patch to handle the case where originally a single source is originally available. In this case the responsive_image module generates a single image tag, which we want to prevent since we have at least two sources.

floretan’s picture

Status: Needs review » Needs work

I didn't properly remove the image style derivatives while testing. This patch works only if the webp images have been generated already, I'll update the patch to handle the case where the first call is done to the webp url.

Bart Vanhoutte’s picture

Updated README.md to reflect this feature.

floretan’s picture

Status: Needs work » Needs review
FileSize
4.91 KB

The two last pieces that needed to be added were:

  1. Generate both the png/jpg/jpeg and webp derivative when the webp url is requested
  2. Move that check for the source file before the token validation, so that we can generate the token for the right file

The attached patch should do just that and seems to be working fine.

floretan’s picture

One important limitation of the current patch: it doesn't work in combination with the imageapi_optimize module.

Bart Vanhoutte’s picture

Because it tries to optimize WebP images? Or am I missing something?

mansspams’s picture

Updated patch that fixes $derivative_uri is undefined variable error.

mansspams’s picture

Nope, wrong fix previously.

thejimbirch’s picture

The patch in #9 in combination with a responsive image seems to be outputting correctly, but the image is giving me a page not found.

Any suggestions on how to proceed?

Thanks!

mherchel’s picture

@thejimbirch Make sure your GD library supports it. Check your watchdog log for any PHP errors. The error I seem to be getting is Error: Call to undefined function Drupal\webp\imagewebp() in Drupal\webp\Webp->createWebpCopy().

You can also run gd_info() to get output to see if WebP is supported. http://php.net/manual/en/function.gd-info.php

sahaj’s picture

I have similar issue as @thejimbirch, the image code is rendered correctly (i.e. sites/default/files/styles/pack_card_thumb/public/2018-06/pic-d4b6b8e55f3ca6a78486e6ee79575b83.webp?h=7540f041&itok=S72y1wrq), and the image is generated in that folder, only it is not displayed on the page.

My GD library is supporting, according to PHPinfo.

JohnAlbin’s picture

I'm not seeing what sahaj is seeing. My problem sounds similar to thejimburch's bug description.

  1. Picture element contains the correct webp URLs
  2. Image is not generated, so the webp URL is a 404.

In my case this is because I have the imageapi_optimize module installed. In webp's RouteSubscriber.php file, there is a specific exception made so that public image styles do NOT make webp versions if the imageapi_optimize module is installed.

I don't know what the purpose of those lines are, but if I remove them, the image gets created as expected. I'm not sure if removing those lines is the correction solution, but this new patch makes that change. Otherwise, it is the same as the patch in #9.

JohnAlbin’s picture

JohnAlbin’s picture

[Edit: sorry for the issue thrashing. I was trying to get the issue queue to stop showing me as the "author" in the git commit message since I barely modified the code that you all wrote.]

JohnAlbin’s picture

One weird thing that this latest patch does is it will generate a derivative example.jpg and an example.jpg.webp file, but the responsive image HTML references example.webp. So, while the "example.webp" image doesn't actually exist in the files directory, the web server still serves the example.jpg.webp file. It's odd, but it doesn't cause me any issues. However, maybe that is what sahaj is experiencing? Sahaj, can you check if the file written to the files directory has a different file name than what is put in the HTML?

saschaeggi’s picture

FileSize
6.83 KB

@johnalbin @thejimbirch @sahaj I fixed the filename issue of the generated webp file from e.g. *.jpg.webp to *.webp and also added a check if the file exists before adding an entry to the responsive image array. works for me, should also work for you too.

RaphaelBriskie’s picture

This patch needs to check if the srcset attribute exists so it doesn't throw errors.

For example, when used in conjunction with the Lazyload Images module (https://www.drupal.org/project/lazyload_images) it should really be using the data-srcset attribute.

/**
 * Implements template_preprocess_responsive_image().
 */
function webp_preprocess_responsive_image(&$variables) {
  $webp_sources = [];
  foreach ($variables['sources'] as $source) {
    /** @var \Drupal\Core\Template\Attribute $source */
    $webp_source = clone $source;
    if ($source->offsetExists('srcset')) {
      $original_url = $source->offsetGet('srcset')->value();
      $attribute    = 'srcset';
    }
    elseif ($source->offsetExists('data-srcset')) {
      $original_url = $source->offsetGet('data-srcset')->value();
      $attribute    = 'data-srcset';
    }
    if (isset($original_url)) {
      $webp_url = str_ireplace(['.png', '.jpg', '.jpeg'], '.webp', $original_url);
      $webp_source->offsetSet($attribute, $webp_url);
      $webp_source->offsetSet('type', 'image/webp');
      $webp_sources[] = $webp_source;
    }
  }

  // Add the new sources at the top of the list.
  $variables['sources'] = array_merge($webp_sources, $variables['sources']);

  // Never output a single image tag, because
  // we will always have at least two sources.
  $variables['output_image_tag'] = FALSE;
}
saschaeggi’s picture

@PaphaelBriskie you could to something like this in your theme:

function THEME_preprocess_responsive_image(&$variables) {
  $placeholder = 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7'; //1x1 Px placeholder

  foreach ($variables['sources'] as $source) {
    /** @var \Drupal\Core\Template\Attribute $source */
    $original_url = $source->offsetGet('srcset')->value();

    $source['data-srcset'] = $original_url;
    $source['srcset'] = $placeholder;
  }
}

Or just use your proposed function in your theme to override the default module's behavior -> THEME_preprocess_responsive_image() {}

saschaeggi’s picture

FileSize
6.65 KB

This is an update to my previous patch.

saschaeggi’s picture

FileSize
6.66 KB

And another round as there was an issue with applying the previous patch, sorry.

saschaeggi’s picture

Category: Feature request » Bug report
Priority: Normal » Major
szeidler’s picture

Patch in #21 was working great for me.

I tested it also in a setup, where we're lazyloading images with the Blazy module. Here I'm getting the following error.

Error: Call to a member function value() on null in webp_preprocess_responsive_image() (line 35 of modules/contrib/webp/webp.module).

While investigating i found, that the blazy module replaces the default srcset attribute with data-srcset, to apply their lazyloading processing.

Although it might not be the nicest solution and not directly this module's responsibility, I'm currently working around this by checking for the existence of the attribute, like in the following patch.

scott_euser’s picture

Assigned: Unassigned » scott_euser
scott_euser’s picture

I have tried to review this as thoroughly as possible, checked
markup and validated dev tools network download of webp for each of the following scenarios:

  1. Responsive image single style with Blazy on
  2. Responsive image single style with Blazy off
  3. Responsive image multiple styles with Blazy on
  4. Responsive image multiple styles with Blazy off

No php notices or warnings any more in any scenario.

All working well and producing expected markup, eg:

<picture>
  <!--[if IE 9]><video style="display: none;"><![endif]-->
  <source srcset="/sites/default/files/styles/cover_image_single/public/2019-02/Report-Cover-Big.webp?itok=wLPR-kp5 1x" media="only screen and (min-width: 1276px)" type="image/webp"/>
  <source srcset="/sites/default/files/styles/cover_image_single/public/2019-02/Report-Cover-Big.webp?itok=wLPR-kp5 1x" media="only screen and (max-width: 1275px)" type="image/webp"/>
  <source srcset="/sites/default/files/styles/cover_image_small/public/2019-02/Report-Cover-Big.webp?itok=0u7Xqtiw 1x" media="only screen and (max-width: 767px)" type="image/webp"/>
  <source srcset="/sites/default/files/styles/cover_image_single/public/2019-02/Report-Cover-Big.png?itok=wLPR-kp5 1x" media="only screen and (min-width: 1276px)" type="image/png"/>
  <source srcset="/sites/default/files/styles/cover_image_single/public/2019-02/Report-Cover-Big.png?itok=wLPR-kp5 1x" media="only screen and (max-width: 1275px)" type="image/png"/>
  <source srcset="/sites/default/files/styles/cover_image_small/public/2019-02/Report-Cover-Big.png?itok=0u7Xqtiw 1x" media="only screen and (max-width: 767px)" type="image/png"/>
  <!--[if IE 9]></video><![endif]-->
  <img class="c-media__image" src="/sites/default/files/styles/cover_image_single/public/2019-02/Report-Cover-Big.png?itok=wLPR-kp5" alt="report cover" typeof="foaf:Image" />
</picture>

What I did notice is if you have a Media entity referencing lets say Responsive Style A then you delete Responsive Style A, the responsive image module continues to produce a picture element with no styles, just image tag. $variables['sources'] is then undefined. Patch wraps this in isset.

Patch also checks that srcset is actually existing in case another unknown module removes srcset in favour of something other than data-srcset.

Just a couple minor coding standards nitpicks.

lindsay.wils’s picture

I have just tried to apply the latest patch with composer and it failed without any substantial messaging, other than it failed. Not sure of the correct procedure for logging this. Has anyone else successfully applied the patch? thanks. really interested in this feature

alexiscott’s picture

The patch applied for me with

"drupal/webp": "1.x-dev"
Xen’s picture

Patch doesn't like files with multiple extensions (file.jpeg.jpg for instance). New patch attached.

alexmoreno’s picture

patch applies for me, but I don't see the markup being generated.

This patch makes a lot of sense as it's the recommended way to serve webp images

iamdroid’s picture

@alexmoreno do you use image style mode in responsive image style settings?

markup provided only in image style mode because only in this mode used picture tag.
in sizes mode used img tag with scrset attribute, so markup can't be provided.

alexmoreno’s picture

Ignore my comment, you need to have responsive images installed and breakpoints configured.

Some docs would be good, but we can do that later updating the Readme

alexmoreno’s picture

Status: Needs review » Reviewed & tested by the community
alexmoreno’s picture

I found some issues in some long paths with arguments. This is what I changed:

  /**
   * @param $filepath
   *   Filepath to convert into .webp extension
   *
   * @return string
   *  file string with .webp extension
   */
  public function getWebpFilename($filepath) {
    $parts = parse_url($filepath);
    // We will deconstruct the path and add the arguments later.
    $path = $parts['path'];

    $webp_url = preg_replace('/\.(png|jpg|jpeg|PNG|JPG|JPEG)(\\?.*)?$/', '.webp\\2', $path);

    // If any arguments were in the filepath, add them now.
    if (isset($parts['query'])) {
      $webp_url = $webp_url . '?' . $parts['query'];
    }

    return $webp_url;
  }

Also added some phpunits:


  /**
   * @covers Drupal\webp\Webp::getWebpFilename
   */
  public function testgetWebpFilename() {
    $this->assertEquals("testimage.webp", $this->webp->getWebpFilename("testimage.jpg"));
    $this->assertEquals("testimage2.webp", $this->webp->getWebpFilename("testimage2.png"));
    $this->assertEquals("testimage2.webp", $this->webp->getWebpFilename("testimage2.jpeg"));
    $this->assertEquals("testimage2.webp", $this->webp->getWebpFilename("testimage2.jpg"));
    $this->assertEquals("testimage2.ext.webp", $this->webp->getWebpFilename("testimage2.ext.jpg"));
    $this->assertEquals("testimage2.ext.webp", $this->webp->getWebpFilename("testimage2.ext.jpg"));
    $this->assertEquals("testimage2.ext.ext.webp", $this->webp->getWebpFilename("testimage2.ext.ext.jpg"));
    $this->assertEquals("testimage2.ext.webp", $this->webp->getWebpFilename("testimage2.ext.jpg"));

    $this->assertEquals("url/sites/default/files/styles/large/public/2019-07/IMG_1949-orig_8.webp", $this->webp->getWebpFilename("url/sites/default/files/styles/large/public/2019-07/IMG_1949-orig_8.JPG"));
    $this->assertEquals("url/sites/default/files/styles/large/public/2019-07/IMG_1949-orig_7.webp?itok=vOpRgtYZ", $this->webp->getWebpFilename("url/sites/default/files/styles/large/public/2019-07/IMG_1949-orig_7.JPG?itok=vOpRgtYZ"));

  }

I'll upload a new patch tomorrow at some point in the day.

alexmoreno’s picture

Status: Reviewed & tested by the community » Needs work
alexmoreno’s picture

  • alexmoreno committed a9b3edd on 8.x-1.x authored by Xen
    Issue #2912442 by floretan, saschaeggi, mansspams, scott_euser, szeidler...
alexmoreno’s picture

Status: Needs work » Fixed
FileSize
1.47 KB

this has been merged now. It includes the unit tests as per the interdiff, added just for reference.

Thanks everyone

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

C. S. Comfort’s picture

Priority: Major » Normal
FileSize
2.07 KB

With the latest 8.x-1.x-dev release, I'm finding that when multiple paths are present the resulting srcset only includes one properly generated webp image path; the other(s) keep the original image format. For example:

<source srcset="/sites/default/files/styles/for_small_device_only/public/2019-08/field-camera.webp?itok=OBK1vveZ 1x, /sites/default/files/styles/for_small_device_only_2x/public/2019-08/field-camera.jpg?itok=nHuptn2j 2x" media="(max-width: 399px)" type="image/webp" />

This looks to be the result of the getWebpFilename($filepath) method that is attempting to parse the $filepath as a URL even though it's either a single path or a comma-separated set of paths that may also include pixel density parameters (e.g. 2x). As a result, I've run into some inconsistent behavior when the browser tries to load the appropriate image -- sometimes it loads the webp version, and others the original full-sized version. I'm not sure it's necessary to have getWebpFilename() parse the incoming string as a URL; rather, it seems simpler to just replace the original file extension with the webp version (which I believe was suggested in an earlier patch).

This is my first contributed patch, so please let me know if there's anything that I should do differently to keep with community expectations/guidelines.

C. S. Comfort’s picture

Updated my previous patch to include the appropriate (and accordingly named) unit tests.

alexmoreno’s picture

+++ b/src/Webp.php
@@ -152,27 +152,18 @@ class Webp {
+    $webp_srcset = str_ireplace(['.png', '.jpg', '.jpeg'], '.webp', $srcset);

will this work as well for the extensions in Capital? Just double checking if you tested that

Apart of that, looks great, thanks a lot for your contribution CS. Putting this back to needs review. I would have preferred to open a new ticket, but let's move forward

alexmoreno’s picture

Priority: Normal » Major
Status: Closed (fixed) » Needs review
Xen’s picture

From the docs:

str_ireplace — Case-insensitive version of str_replace()

So yes, I would assume it works with extensions in capital.

C. S. Comfort’s picture

As Xen mentioned, yes, str_ireplace should accommodate varying usage of capitals in the file extension. We could update the test assertions to explicitly test a number of variations, though currently the last four passing assertions do see a .JPG in the string.

acontia’s picture

Tested patch in #40.

It works for me. It solves the issue where multiple sources are specified as a single string separated by commas ", ". Without the patch they are not parsed correctly.

I think the patch might fail if the filename contains ".jpg". For example test.jpg.jgp would converted to test.webp.webp as opposed to test.jpg.webp

Maybe the regular expression should check that the character after ".jpg" is an "?" or ", " or end of string?

Xen’s picture

Status: Needs review » Needs work

OK, I should pay attention here.

Patch #39/#40 reverts the fix for handling multiple extensions (as seen in the wild) I added in #28. We only want to fix the last extension, that's why I changed str_ireplace to preg_replace.

And in order to make the replacement case insensitive, there's no need for duplicating the extensions, preg_replace('/\.(png|jpg|jpeg)(\\?.*)?$/i', '.webp\\2', $path); will do that, and also cover the "JpEg" case.

For those not reading regex, (\\?.*)?$ translates to "an optional questionmark followed by anything, at the end of string", and the \\2 then replaces whatever that parenthesis matched into the result string. In other words, the regex matches a dot, any of the extensions and then an optional query argument, and carries the query over to the result.

renguer0’s picture

I can't apply this patches because it shows corrupted patch at lines 62 (#39) and 102 (#40). I applied this manually but still don't working in my site (only on PNG images).
I'm applying them into lastest dev version. Maybe there could be something wrong in some another config? This is really weird.

EDIT: I see that errors with webp conversion on JPG files are caused by my hosting that's blocking that action. The module only works on PNG files.

Maybe someone knows if I can avoid it. I'm dealing with that and thinking into moving out from there because of this issue.

Sorry about this OT, maybe this answer helps someone in the same situation.

mherchel’s picture

@renguer0 - Yeah, it's a little off topic, but still relevant. It might be best to document the errors in another issue (so maybe the module can surface issues to the user beforehand).

Can you create an issue with 1) your host info, 2) errors you're seeing, 3) if you're using GD or imagemagic 4) relevant info from phpinfo() including GD or ImageMagick info?

renguer0’s picture

Forget it @mherchel but thanks anyway. I'm in SiteGround that blocks the generation of webp (only enables that from PNG) on their basic plan. I'm moving to Digital Ocean and manage the server by my own, the module with last patch works well for me.

This answer maybe can helps to someone that have the same problem. I lost a lot of time trying different things and falling into a headache :P

I hope that merging the last patch with the recomendations from #46 by Xen, could resolve that issue.

saschaeggi’s picture

@xen can you create a new patch with your changes?

alexmoreno’s picture

could someone add a patch with the latest recommendations if needed? If not there is not really anything to do here. Could you have time for that @Xen?

Thanks.

Xen’s picture

@saschaeggi/@alexmoreno

I'm trying, but first I have to figure out what the latest patch really does in order to not break that.

alexmoreno’s picture

I built some tests for that part, they should come handy for the changes you are trying to do. Shout if I can help somehow

Thanks a lot :-)

Xen’s picture

Xen’s picture

Status: Needs work » Needs review
saschaeggi’s picture

Status: Needs review » Reviewed & tested by the community

@xen your patch works like a charm for me

saschaeggi’s picture

@xen can you include this into dev?

Xen’s picture

@saschaeggi

I'm not a maintainer on this project, so no. alexmoreno can.

alexmoreno’s picture

I’ll get it done this weekend, it was in my todo list

saschaeggi’s picture

@alexmoreno any update on this?

alexmoreno’s picture

sorry, Drupalcon and work got in the middle. Will get it reviewed and merged soon

  • alexmoreno committed fbb0af3 on 8.x-1.x authored by Xen
    Issue #2912442 by saschaeggi, floretan, Xen, C. S. Comfort, mansspams,...
alexmoreno’s picture

merged, thanks all

alexmoreno’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.