Problem/Motivation
This module is not working for Drupal 7 on Pantheon (which provides wepP support in GD). I would like to get it working for one of my Pantheon sites that could really benefit from wepP images.
Steps to reproduce
See https://www.drupal.org/project/imagewebp/issues/3095228#comment-14144990
Proposed resolution
I'd like to do a cleanup on the code in this module to bring it inline with Drupal coding standards, properly namespace all the functions, and see if we can get this module working as described.
Remaining tasks
* properly namespace all functions
* add docblocks to all internal functions
* update coding standards
* prevent PHP notices
* investigate failures and fix direct file path access bug
User interface changes
* "was already existed" became "already exist."
API changes
* none
Data model changes
* none
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | imagewebp-clean_up_get_working-3224531-6.patch | 17.86 KB | jenlampton |
| #4 | imagewebp-clean_up_get_working-3224531-4.patch | 9.73 KB | jenlampton |
Comments
Comment #2
jenlamptonComment #3
jenlamptonComment #4
jenlamptonAttached is a patch that contains a minor rewrite of the module, including:
* properly namespace all functions
* add docblocks to internal functions
* update coding standards
* prevent PHP notices
* investigate failures and fix direct file path access bug
Much of this patch was based on improvements by Péter Király from the Backdrop CMS version of this module.
Comment #5
jenlamptonI've got a few more improvements to the image rendering process, but need to go afk for a little while. I'll post a new patch shortly!
Comment #6
jenlamptonOkay, here's an updated version that will output the webP image as a picture tag with srcset, so that browsers that don't support webP can still render the fallback img tag instead.
The previous approach assumed caching was disabled, or that every browser viewing the site would be the same. This will work for caching across different browsers since the markup will be the same for all.
I also included a little more cleanup (function renames, more documentation, etc). And I think this patch also includes work from https://www.drupal.org/project/imagewebp/issues/3224539, but I need to get on a plane so want to post my work here before it gets lost :)
Comment #8
nipany commentedComment #9
nipany commentedComment #10
jenlamptonThanks for the quick review @nipany! I have a few more improvements I'm working on, but I'll create another issue when they are ready :)