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.
This is related to #540486: Error with PHP 5.3, but not fully handled there
When using PHP 5.3, after applying either the #9 or #12 patches in the above issue, the imagecache_actions actions don't work at all. I have at least observed the lack of rounded corners.
This is also related to Drupal core's #360605: PHP 5.3 Compatibility.
Core handled it by trying to fix the values to pass them by reference, but
João
Comment | File | Size | Author |
---|---|---|---|
#24 | imagecache_actions-648950-3846070.patch | 888 bytes | skilip |
#21 | 2010-12-07-imagecache_actions-648950.patch | 1.78 KB | mikl |
#12 | imagecache_actions_overlay.patch | 620 bytes | Volx |
#7 | imagecache_actions_648950.patch | 666 bytes | jcnventura |
#5 | imagecache_actions_648950.patch | 4.15 KB | jcnventura |
Comments
Comment #1
jcnventura CreditAttribution: jcnventura commentedCorrection, some of them work, as the text is working fine.. The problem seems to be the layer parameter that is passed to imageapi_image_overlay()
Comment #2
dman CreditAttribution: dman commentedTricky.
I've just tried reading through the whole core thread, and I've not yet figured out what the proper fix is. It appears PHP5.3 requires us to change the function signature in a way that is incompatible with all earlier code. That's not fun.
Comment #3
jcnventura CreditAttribution: jcnventura commentedAs you can read about in http://pt.php.net/manual/en/migration53.incompatible.php (6th bullet), the real reason for this problem is that function arguments were declared by-reference, but instead passed by-value. Previously, PHP just accepted it anyway, but now it doesn't. So now, PHP programmers need to know what they're doing when adding a '&' in a function declaration. I believe that by changing it correctly, it won't be incompatible with PHP < 5.3.
All you need to do is to correct the code to pass that argument by reference or to correct the imageapi_image_overlay() function declaration. Note that I tried to do the latter to remove the warning, but rounded-corners don't work for me when doing that..
I don't know enough of the module's code to change it to properly pass the layer by reference.
João
Comment #4
BerdirCorrect, as stated in the linked php.net upgrade handbook (that part was written by me, actually ;)), it is in most cases not necessary to change the function signature. Instead, you need to fix the calling code, more details can be found here: #360605-200: PHP 5.3 Compatibility.
However, there are some cases when & needs to be removed, for example for theme functions, which can never recieve a param by reference. Also consider that if you recieve this warning in PHP 5.3, by reference *never* worked. Older PHP version just silently call your function with by value, the only change in PHP 5.3 is that these params are set to NULL and a warning is displayed.
Comment #5
jcnventura CreditAttribution: jcnventura commentedThe following code adds the missing '&', except in the call to imageapi_gd_image_overlay() where it is removed.
I've tested this with my patch for #540486: Error with PHP 5.3 and they're working fine.
João
Comment #6
BerdirAdding & to normal function calls should not be necessary, in fact, it should result in a PHP Notice. Only call_user_func_array() calls need to be changed to include the &.
Comment #7
jcnventura CreditAttribution: jcnventura commentedYou're right.. I have reverted most of the changes, except for one (removing the '&' from imageapi_gd_image_overlay) and everything works as before, and I no longer get the following warning:
That line is indeed a call_user_func_array() call.
João
Comment #8
dman CreditAttribution: dman commentedI can't actually test the problem ... but IIRC there is a reason why that function used to use a reference. When 'overlaying' in reverse (that last flag), it's the overlay image that gets modified.
The logic there may be a little screwy and need restructuring though.
I'll need to run it through the test suite...
(Yes, I have a test suite!)
Comment #9
BerdirThe issue is that, as I've said above, when called with call_user_func_array() without an explicit "&", it was never by-reference, older PHP version simply made it silenty by-value.
If I understood it correctly, the function is used in your code and also by imageapi.module. So you either need to change imageapi.module to call it by reference or change your application logic.
Comment #10
Blackbird CreditAttribution: Blackbird commentedIn my case (adding a watermark), for the call to imageapi_gd_image_overlay() to work I had to remove the ampersand on the first 2 arguments.
Comment #11
basvredelingChange in patch #7 works for me too, would be nice if this could be committed.
Comment #12
Volx CreditAttribution: Volx commentedChanging the function call of imageapi_toolkit_invoke() in imageapi_image_overlay() also works and might be a better solution, since it doesn't change the function signature.
Comment #13
iancawthorne CreditAttribution: iancawthorne commentedSubscribing.
Comment #14
kabojnk CreditAttribution: kabojnk commentedCan we expect one of these patches to be committed? It's been nearly a year now since the inception of this report.
If it's as simple as fixing a reference operator in a function parameter, why not just commit it rather than pointing the finger at php 5.3 telling Zend to fix it first? Is it not backwards compatible to 5.2 or does it cause problems with benchmarking? Or has it already been committed to the 6.x-2.x branch?
Comment #15
WeeDz CreditAttribution: WeeDz commentedWhen, please when?? ;)
Comment #16
scottrouse CreditAttribution: scottrouse commentedConfirming that adding the
&
as indicated in the patch in #12 above fixes this issue on a server running PHP 5.3.2-1ubuntu4.5.Comment #17
cybis CreditAttribution: cybis commented#12 fixed it for me.
Comment #18
parasolx CreditAttribution: parasolx commentednone above doesn't solve problem if using php 5.3.3
error of imageapi_gd_image_overlay() still appear even i applied all patch above.
Comment #19
anrikun CreditAttribution: anrikun commentedDoes the solution at #789900: warning: Parameter 2 to imageapi_gd_image_overlay() expected to be a reference, value given in (#9) works better?
Comment #20
anrikun CreditAttribution: anrikun commentedAny news about this issue?
Comment #21
mikl CreditAttribution: mikl commentedHere's a patch that should apply on 6.x-1.7 for those who would rather use the stable version.
Comment #22
dunx CreditAttribution: dunx commentedPatch #21 good for me on 6.x-1.7
Comment #23
skilip CreditAttribution: skilip commentedApplying the patch om #21 raised another reference error in theme_imagecacheactions_rgb_form(). The only thing I needed to remove the reference error in imageapi_gd_image_overlay() was changing:
to:
I'm not sure why the second argument $layer was referenced originally though.
Comment #24
skilip CreditAttribution: skilip commentedComment #25
dman CreditAttribution: dman commentedThe second layer was referenced because 'overlay' worked two directions. Sometimes the $layer was put on the $image and the $image returned, sometimes the other way around - in which case the $layer was manipulated and was being used by reference.
HOWEVER that 'logic' needs review, feels a bit mixed up after so many years of other code changes, and may not even be the case any more. Both the internal algorithms and the imagecache API have changed since it was written this way
Comment #26
Mascher CreditAttribution: Mascher commentedI see different team about Errors imageapi with php 5.3 - http://drupal.org/node/540486
It's no some problem?
Comment #27
nohup CreditAttribution: nohup commented+1 for #12
fixes on php 5.3.3-1ubuntu9.1
Comment #28
ryan_courtnage CreditAttribution: ryan_courtnage commentedsubscribe
Comment #29
brycesenz CreditAttribution: brycesenz commentedThe patch in #12 works for me as well. Changing the status to needs review, since it seems that this patch is working for a fair number of us.
Comment #30
Maciej Lukianski CreditAttribution: Maciej Lukianski commentedI can confirm that the patch works for me as well. Using php5.3 on Ubuntu 10.4
Comment #31
BeatnikDude CreditAttribution: BeatnikDude commentedMe: Ubuntu 10.04: PHP 5.3.2: Drupal 6.20: ImageCache 6.x-2.0-beta10
#12 did the trick. Lets commit!
Comment #32
eric.chenchao CreditAttribution: eric.chenchao commentedsubscribe
Comment #33
timb CreditAttribution: timb commentedI am using the patch in #12 on multiple installs. Would love to see this committed
Comment #34
dman CreditAttribution: dman commentedfine...
http://drupal.org/node/1064426
:-}
I was sorta thinking the next version release would have more in it, but OK, one line update, why not. It is broken for some folk...