As discussed thoroughly in http://drupal.org/node/705512, php5.3 does not allow passing array() into a function expecting a reference.

Behavior is:
upload a file and when the next upload field is produced there is a Warning:

"warning: Parameter 2 to [module]_form_alter() expected to be a reference, value given in [.]/common.inc on line 2883."

I'm not near a system to create a patch (sorry).

My solution was to copy the CCK fix:

upload.module line 629:
--drupal_alter('form', $form, array(), 'upload_js');
++ $form_state = array();
++ $data = $form;
++ $data['__drupal_alter_by_ref'] = array(&$form_state);
++ drupal_alter('form', $data, 'upload_js');

works great for me.

CommentFileSizeAuthor
#14 925580-php5-3-drupal-alter-14.patch657 bytesmarcp
#3 upload-drupal_alter_parameter_by_reference_2.patch781 bytesrsevero
#1 upload-drupal_alter_parameter_by_reference.patch618 bytesrsevero

Comments

rsevero’s picture

rsevero commented
Status: Active » Needs review
StatusFileSize
new upload-drupal_alter_parameter_by_reference.patch618 bytes

Fix for upload module similar to the one from #705512: drupal_alter('form'... called from content_add_more_js needs to pass $form_state as a reference.

Status: Needs review » Needs work

The last submitted patch, upload-drupal_alter_parameter_by_reference.patch, failed testing.

rsevero’s picture

rsevero commented
Status: Needs work » Needs review
StatusFileSize
new upload-drupal_alter_parameter_by_reference_2.patch781 bytes

The same patch for latest CVS core version.

Status: Needs review » Needs work

The last submitted patch, upload-drupal_alter_parameter_by_reference_2.patch, failed testing.

rsevero’s picture

rsevero commented
Version: 6.19 » 6.x-dev
Status: Needs work » Needs review

Trying to change version to see if this way the patch is at least applied during test.

rsevero’s picture

rsevero commented

#3: upload-drupal_alter_parameter_by_reference_2.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, upload-drupal_alter_parameter_by_reference_2.patch, failed testing.

mcjim’s picture

mcjim commented
Status: Needs work » Needs review

#3: upload-drupal_alter_parameter_by_reference_2.patch queued for re-testing.

Not sure why it's failing: applies cleanly and works.

Status: Needs review » Needs work

The last submitted patch, upload-drupal_alter_parameter_by_reference_2.patch, failed testing.

eric.chenchao’s picture

eric.chenchao commented
Status: Needs work » Needs review

#3: upload-drupal_alter_parameter_by_reference_2.patch queued for re-testing.

eric.chenchao’s picture

eric.chenchao commented

Yes, so strange the patch looks good and can solved issues like:

# warning: Parameter 2 to *****_form_alter() expected to be a reference, value given in C:\Arquivos de Programas\wamp\www\drupal-6.15\includes\common.inc on line 2829.

Status: Needs review » Needs work

The last submitted patch, upload-drupal_alter_parameter_by_reference_2.patch, failed testing.

Anonymous’s picture

Anonymous (not verified) commented

I simply modified the file upload.module like tbenice suggested. OK, not a good idea to modify the core, but it works well for me. Thanks!

marcp’s picture

marcp commented
Status: Needs work » Needs review
StatusFileSize
new 925580-php5-3-drupal-alter-14.patch657 bytes

New patch attached. This one sends in an empty form_state array, which is what (I think) the intent was of the original code.

marcp’s picture

marcp commented
Title: PHP5.3 and drupal_alter » upload throws warnings on PHP 5.3
Issue tags: +PHP 5.3

Updated the title and tagged with PHP 5.3.

marcp’s picture

marcp commented

Just to get the latest line number (from 6.20) in here where folks will see the problem upon uploading a file:

* warning: Parameter 2 to date_form_alter() expected to be a reference, value given in /var/www/html/includes/common.inc on line 2892.

dave kopecek’s picture

dave kopecek
he/him
commented

#14 Worked for me. Fixed ad / ad_image module use on php 5.3 which relies on upload.

jweowu’s picture

jweowu commented
Status: Needs review » Reviewed & tested by the community

This looks good.

I can't quite see the point of $data = &$form; as opposed to using $form directly, but it does no harm, and regardless this is clearly the appropriate solution to the problem, as the __drupal_alter_by_ref key is the official API for passing extra arguments by reference when calling drupal_alter().

Danzki’s picture

Danzki commented

this patch worked for me :)

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented

Worked for me too. Can we get this committed?

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented
Title: upload throws warnings on PHP 5.3 » Upload throws warnings on PHP 5.3
Jewish_Doctor’s picture

Jewish_Doctor commented

How do you apply this patch? Obviously I'm a noob but I can't find info on this kind of file off Google to "do it yourself".

jweowu’s picture

jweowu commented

Jewish_Doctor: Start reading here: http://drupal.org/patch

yngens’s picture

yngens commented

Same issue. Subscribing.

danSamara’s picture

danSamara commented

Patch worked for me. Commit it, please )

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

There's no need to do these strange tricks. It should be enough to do $empty_form_state = array(); and pass that instead of array() directly no?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs work » Reviewed & tested by the community

The patch is exactly what http://api.drupal.org/api/drupal/includes--form.inc/function/drupal_prep... does when it already has an $form_state variable defined:

  $data = &$form;
  $data['__drupal_alter_by_ref'] = array(&$form_state);
  drupal_alter('form_' . $form_id, $data);
catch’s picture

catch
he/him
Primary language English
commented

Dave Reid pointed out that we do need to do this, and it's copied from drupal_prepare_form(), I'd apparently not seen that pattern before.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

This is correct. Ideally, we'd add a @see drupal_prepare_form(), but I don't really care about the inline docs in older releases.

praseodym’s picture

praseodym commented

This patch silently breaks uploading for me (i.e. no errors, but nothing gets uploaded either).

monotaga’s picture

monotaga commented

The patch from #14 is working so far for me.

As much as I love hacking core (and fatally wounding young felines in the process), any chance we can get this committed?

rares’s picture

rares commented

Yes, let's get this committed. Unfortunately, a lot of contrib modules have used the drupal_alter('form', $form, array(), $form_id); syntax (see http://drupal.org/node/710892). However, hacking drupal_alter() to support this syntax is probably too much.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Committed this, thanks for testing.

lyd’s picture

lyd commented

I am still using #14 solution successfully.
From #33 I understand this has been committed. Does that mean that new downloads of core include this change? Or, what am I supposed to do to have the change included when creating a new OA site?

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

@lyd: the next bugfix Drupal release (either 6.23 or 6.24) will include the fix. In the meantime it is in the development version of Drupal 6 already.

jmoughon’s picture

jmoughon commented

I applied the patch and the files are being uploaded. However they are not displaying in the "File Attachments" list. The uploader runs and resets and the file gets put in the files folder.

philosurfer’s picture

philosurfer commented

Patch @ #14 worked perfect on Atrium 1.0 install.

Status: Fixed » Closed (fixed)

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

technikh’s picture

technikh commented

The patch in #14 didn't work for me. any suggestions?
Open Atrium 1.0
Drupal 6.22

I get these warning messages..

warning: Parameter 2 to notifications_team_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to purl_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to spaces_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to atrium_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to atrium_book_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to casetracker_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to date_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to date_timezone_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to og_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to fivestar_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to ideation_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to wysiwyg_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to og_access_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
warning: Parameter 2 to notifications_content_form_alter() expected to be a reference, value given in /www/openatrium/docs/includes/common.inc on line 2892.
WebmistressM’s picture

WebmistressM commented

I have the same issue. Using Drupal 6

Ludwig’s picture

Ludwig commented

I have the same issue as TechNikh @ #39
Any clues would be nice:-)

EDIT: I applied the patch to upload.module for Open Atrium and it works for me:-)

technikh’s picture

technikh commented

@Ludwig #41
I am applying the patch in #14 manually to upload.module file in /www/openatrium/docs/modules/upload
I still see the warning messages. do you know where I am doing wrong?

technikh’s picture

technikh commented

editing includes/common.inc as per http://drupal.org/node/710892#comment-3884210 , fixed the warning in my case

memcinto’s picture

memcinto commented

Applying the patch in #14 fixed my problem. I also patched common.inc as mentioned in #43 to fix yet another PHP 5.3 error message, and that also worked.

memcinto’s picture

memcinto commented

Patch in #14 worked for me - at least for this error. (I also had to apply a patch to common.inc, as described http://drupal.org/node/710892#comment-3884210, to fix another problem.)

kamranzafar’s picture

kamranzafar commented

Thanks memcinto, I also had to apply a patch to common.inc, from http://drupal.org/node/710892#comment-3884210. It worked for me.