Module breaks against Drupal 11 10.3

PHP Fatal error:  Declaration of Drupal\imageapi_optimize_webp\Controller\ImageStyleDownloadController::deliver(Symfony\Component\HttpFoundation\Request $request, $scheme, Drupal\image\ImageStyleInterface $image_style) must be compatible with Drupal\image\Controller\ImageStyleDownloadController::deliver(Symfony\Component\HttpFoundation\Request $request, $scheme, Drupal\image\ImageStyleInterface $image_style, string $required_derivative_scheme)
CommentFileSizeAuthor
#26 imageapi_optimize_webp-bc-break-3453941-26.patch5.04 KBv.dovhaliuk
#23 imageapi_optimize_webp-bc-break-3453941-23.patch2.44 KBv.dovhaliuk
#6 3453941-bc-break.patch1.71 KBdjdevin
#5 3453941-bc-break.patch703 bytesdjdevin

Issue fork imageapi_optimize_webp-3453941

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

djdevin created an issue. See original summary.

bharath-kondeti’s picture

bharath-kondeti commented

Is this issue on Drupal 11 or Drupal 10 ?
This module does not have a release for Drupal 11 yet. It must be throwing from a patch from this issue https://www.drupal.org/project/imageapi_optimize_webp/issues/3448709

djdevin’s picture

djdevin
he/him
Primary language English
Location Philadelphia
commented

Sorry this is actually Drupal 10.3!

But there is an issue in core to not break BC here: https://www.drupal.org/project/drupal/issues/3450244

However, from the issue:

The class and method in question are 100% internal. Per Drupal API policy no one should be extending, calling, or otherwise using the method unless they are willing to accept breaking changes in point releases.
...
this is an intended hard break

So we may have to fix it here.

djdevin’s picture

djdevin
he/him
Primary language English
Location Philadelphia
commented
Issue summary: View changes
djdevin’s picture

djdevin
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Needs work
StatusFileSize
new 3453941-bc-break.patch703 bytes

Setting as needs work, since this is a BC break.

djdevin’s picture

djdevin
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new 3453941-bc-break.patch1.71 KB
michaellander’s picture

michaellander commented

Thanks for the patch. We'll likely need to create a 2.1 and make a requirement for 10.3 and above.

What I've found is the approach with this module and some of the others in the space are a bit dated(and require overriding a core controller which created this issue to begin with. I'm working on a potential new alternative that I think would be foundationally stronger, but in the meantime we'll keep supporting this.

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Title: PHP Fatal error: Declaration of Drupal\imageapi_optimize_webp\Controller\ImageStyleDownloadController::deliver » Declaration of Drupal\imageapi_optimize_webp\Controller\ImageStyleDownloadController::deliver() must be compatible with Drupal\image\Controller\ImageStyleDownloadController::deliver()
Issue summary: View changes
siramsay’s picture

siramsay commented

I've applied the patch at #6 to a couple of site so far and can report it is working with 10.3.1.

Can we get a 2.1 version? Is there anyway I can help with that?

michaellander changed the visibility of the branch 3453941-declaration-of-drupalimageapioptimizewebpcontrollerimagestyledownloadcontrollerdeliver to hidden.

michaellander’s picture

michaellander commented
michaellander’s picture

michaellander commented

Hiding patch and moving to branch.

michaellander opened merge request !14

michaellander opened merge request !15

michaellander’s picture

michaellander commented
Version: 2.x-dev » 2.1.x-dev
michaellander’s picture

michaellander commented
Status: Needs work » Fixed
danielwirz’s picture

danielwirz commented

Hi there
I also did a successful test of the patch. I also tested the 2.1-dev branch on my projects - also successful.
What is the timeline for the 2.1 release? Can I help you with this in any way?

michaellander’s picture

michaellander commented

@danielwirz, am wanting to include #3448709: Automated Drupal 11 compatibility fixes for imageapi_optimize_webp in a 2.1 release so we can confidently support Drupal 11. If you can help test the patch in that issue on 10.3 and 11, that'd be great.

michaellander’s picture

michaellander commented

Also going to test #3466630: Attempt to remove ImageStyleDownloadController override completely, which would really simplify things. In which case maybe we just move to 3.x if it works.

michaellander’s picture

michaellander commented

If we can get some more eyes on #3466630: Attempt to remove ImageStyleDownloadController override completely, I believe it'll make this update much easier overall. I think it allows us to keep this in 2.1, and should give us a path forward for modules like ImageAPI Optimize AVIF to run alongside this module without needing glue modules like ImageAPI Optimize AVIF & WebP

danielwirz’s picture

danielwirz commented

@michaellander thanks for the feedback. i'll test the patch next week and give a feedback

Status: Fixed » Closed (fixed)

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

v.dovhaliuk’s picture

v.dovhaliuk commented
StatusFileSize
new imageapi_optimize_webp-bc-break-3453941-23.patch2.44 KB

Since using MR as a patch is a security issue, I'm adding a patch as a temporary solution until a new release of the module is added.

radio_babylon’s picture

radio_babylon commented

so is this ever going into the module release version? i really need to be able to update some sites that break when i do... thanks!

radio_babylon’s picture

radio_babylon commented

so i am guessing that is a 'no' and i need to remove the module then.

v.dovhaliuk’s picture

v.dovhaliuk commented
StatusFileSize
new imageapi_optimize_webp-bc-break-3453941-26.patch5.04 KB

Rerolled patch from #23 to add compatibility with Drupal 11.

chucksimply’s picture

chucksimply commented

So is this already in the latest 2.1 released in Jan 2025? Patches aren't working.

Edit: Looks like no. Can we get some clarity as to the status of this MR? Do we need an updated patch for 2.1?

Edit 2: Patch #26 applies to 2.0.0 successfully. Fixes the issue and makes it compatible with D11

weseze’s picture

weseze commented

For Drupal 10.3.x the latest version of this module (2.1.0) is working correctly.
The extra parameter ($required_derivative_scheme) was added in this release.
So not patch is needed for Drupal core 10.3x and 2.1.0 of this module.

As for Drupal11 compatibility: I do not know, have not tested this yet. But looking at the code it should work without the need for this patch.