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

Comments

jenlampton created an issue. See original summary.

jenlampton’s picture

Issue summary: View changes
jenlampton’s picture

Issue summary: View changes
jenlampton’s picture

Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new9.73 KB

Attached 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.

jenlampton’s picture

Status: Needs review » Needs work

I'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!

jenlampton’s picture

Status: Needs work » Needs review
StatusFileSize
new17.86 KB

Okay, 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 :)

  • dfca807 committed on 7.x-1.x
    Issue #3224531: Webp support in GD
    
nipany’s picture

Version: 7.x-1.x-dev » 7.x-1.1
Status: Needs review » Fixed
nipany’s picture

Status: Fixed » Closed (fixed)
jenlampton’s picture

Thanks 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 :)