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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jcnventura’s picture

Correction, 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()

dman’s picture

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

jcnventura’s picture

As 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

Berdir’s picture

Correct, 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.

jcnventura’s picture

Status: Active » Needs review
FileSize
4.15 KB

The 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

Berdir’s picture

Adding & 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 &.

jcnventura’s picture

You'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:

warning: Parameter 2 to imageapi_gd_image_overlay() expected to be a reference, value given in sites/all/modules/imageapi/imageapi.module on line 165.

That line is indeed a call_user_func_array() call.

João

dman’s picture

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

Berdir’s picture

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

Blackbird’s picture

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

basvredeling’s picture

Change in patch #7 works for me too, would be nice if this could be committed.

Volx’s picture

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

iancawthorne’s picture

Subscribing.

kabojnk’s picture

Can 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?

WeeDz’s picture

When, please when?? ;)

scottrouse’s picture

Status: Needs review » Reviewed & tested by the community

Confirming that adding the & as indicated in the patch in #12 above fixes this issue on a server running PHP 5.3.2-1ubuntu4.5.

cybis’s picture

#12 fixed it for me.

parasolx’s picture

Issue tags: +PHP 5.3.3

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

anrikun’s picture

Status: Reviewed & tested by the community » Needs work
anrikun’s picture

Any news about this issue?

mikl’s picture

Issue tags: -PHP 5.3.3 +PHP 5.3 compatibility
FileSize
1.78 KB

Here's a patch that should apply on 6.x-1.7 for those who would rather use the stable version.

dunx’s picture

Patch #21 good for me on 6.x-1.7

skilip’s picture

Applying 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:

function imageapi_gd_image_overlay(&$image, &$layer, $x, $y, $alpha = 100, $reverse = FALSE) {

to:

function imageapi_gd_image_overlay(&$image, $layer, $x, $y, $alpha = 100, $reverse = FALSE) {

I'm not sure why the second argument $layer was referenced originally though.

skilip’s picture

dman’s picture

The 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

Mascher’s picture

I see different team about Errors imageapi with php 5.3 - http://drupal.org/node/540486
It's no some problem?

nohup’s picture

+1 for #12
fixes on php 5.3.3-1ubuntu9.1

ryan_courtnage’s picture

subscribe

brycesenz’s picture

Status: Needs work » Needs review

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

Maciej Lukianski’s picture

I can confirm that the patch works for me as well. Using php5.3 on Ubuntu 10.4

BeatnikDude’s picture

Me: Ubuntu 10.04: PHP 5.3.2: Drupal 6.20: ImageCache 6.x-2.0-beta10
#12 did the trick. Lets commit!

eric.chenchao’s picture

subscribe

timb’s picture

I am using the patch in #12 on multiple installs. Would love to see this committed

dman’s picture

Status: Needs review » Fixed

fine...

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

Status: Fixed » Closed (fixed)
Issue tags: -PHP 5.3, -PHP 5.3 compatibility

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