Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Image Captcha in Drupal 10 beta 1 causes PHP fatal error in CaptchaImageResponse::prepare
PHP Fatal error: Declaration of Drupal\image_captcha\Response\CaptchaImageResponse::prepare(Symfony\Component\HttpFoundation\Request $request) must be compatible with Symfony\Component\HttpFoundation\Response::prepare(Symfony\Component\HttpFoundation\Request $request): static in /var/www/web/modules/contrib/captcha/image_captcha/src/Response/CaptchaImageResponse.php on line 71
Steps to reproduce
Enable image_captcha and visit admin/config/people/captcha/image_captcha and expand the example.
Issue fork captcha-3314552
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:
changes
Comments
Comment #2
AnybodyIndeed this is important to fix for Drupal 10!
Comment #3
srijeeta CreditAttribution: srijeeta at Acquia for Drupal India Association commentedComment #4
Anybody@Grevil: Wondering why no test fails with this error? Is there no test accessing the configuration page and checking its status code? Then that should be added here.
Comment #6
Anybody@eporama does the attached MR fix the issue for you?
@Grevil: Beside adding a test that is expected to fail in 8.x-1.x currently, I guess the same fix and test should be added to 2.x?
Also I'm unsure if this might break Drupal 8 / 9 installations.
So my guess would be to
Comment #7
AnybodyComment #8
AnybodyComment #9
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedI think, we should only apply these changes in 2.x, as they won't be fully compatible with Drupal 8 / 9 instances using PHP<8.0, as the "Symfony\Component\HttpFoundation\Response" class added loads of return type hinting in their newest release used in D10. This is usually not a big Problem as they can be simply added and even if the parent method doesn't have these return type hints, it won't throw an error.
The big problem is, that the static return type hint is a PHP 8 feature, so if we add the hinting in 8.x, we will make it incompatible with Drupal Sites using
So I am against adding this to 8.x.
Instead, let's apply the changes in 2.x and make the module only compatible with Drupal 10. As it requires at least PHP 8.1.
Comment #10
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedSetting this to "Normal", as Drupal 10 has not released yet.
Comment #11
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commented@eporama try this patch. Not sure if it will work, but the MR will definitely not!
Comment #12
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commented@Anybody there aren't any image_captcha tests currently in 2.x nor in 8.x, I will add some basic tests for the module inside #3316486: [2.x] Code review, cleanup and tests!
Comment #13
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedAlso, "sendContent()" originally returned void, but now needs to return "static", so we definitely need to make 2.x Drupal 10 only.
Comment #14
AnybodyOkay, let's do it that way and document it somewhere. Perhaps even on the module page, linking the corresponding issue for documentation?
Comment #15
AnybodyComment #16
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedWe should also remove jquery/once here! :)
Comment #17
Anybody@Grevil: Re #16 done in #3324411: Remove unused jquery/once
Comment #18
Anybody@Grevil: I switched the 2.x tests to D10, so please review and commit this first! :)
Comment #19
AnybodyComment #20
AnybodyComment #22
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedAll done!
Comment #27
AnybodyLGTM!
Comment #28
AnybodyThanks for merging :D
Comment #29
Grevil CreditAttribution: Grevil as a volunteer and at DROWL.de commentedFixed, sorry for the bad commit message!