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.
Comment | File | Size | Author |
---|---|---|---|
#14 | 925580-php5-3-drupal-alter-14.patch | 657 bytes | marcp |
#3 | upload-drupal_alter_parameter_by_reference_2.patch | 781 bytes | rsevero |
#1 | upload-drupal_alter_parameter_by_reference.patch | 618 bytes | rsevero |
Comments
Comment #1
rsevero CreditAttribution: rsevero commentedFix 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.
Comment #3
rsevero CreditAttribution: rsevero commentedThe same patch for latest CVS core version.
Comment #5
rsevero CreditAttribution: rsevero commentedTrying to change version to see if this way the patch is at least applied during test.
Comment #6
rsevero CreditAttribution: rsevero commented#3: upload-drupal_alter_parameter_by_reference_2.patch queued for re-testing.
Comment #8
mcjim CreditAttribution: mcjim commented#3: upload-drupal_alter_parameter_by_reference_2.patch queued for re-testing.
Not sure why it's failing: applies cleanly and works.
Comment #10
eric.chenchao CreditAttribution: eric.chenchao commented#3: upload-drupal_alter_parameter_by_reference_2.patch queued for re-testing.
Comment #11
eric.chenchao CreditAttribution: eric.chenchao commentedYes, 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.
Comment #13
Tomark CreditAttribution: Tomark commentedI 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!
Comment #14
marcp CreditAttribution: marcp commentedNew patch attached. This one sends in an empty form_state array, which is what (I think) the intent was of the original code.
Comment #15
marcp CreditAttribution: marcp commentedUpdated the title and tagged with PHP 5.3.
Comment #16
marcp CreditAttribution: marcp commentedJust 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.
Comment #17
Dave Kopecek#14 Worked for me. Fixed ad / ad_image module use on php 5.3 which relies on upload.
Comment #18
jweowu CreditAttribution: jweowu commentedThis 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 callingdrupal_alter()
.Comment #19
Danzki CreditAttribution: Danzki commentedthis patch worked for me :)
Comment #20
colanWorked for me too. Can we get this committed?
Comment #21
colanComment #22
Jewish_Doctor CreditAttribution: Jewish_Doctor commentedHow 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".
Comment #23
jweowu CreditAttribution: jweowu commentedJewish_Doctor: Start reading here: http://drupal.org/patch
Comment #24
yngens CreditAttribution: yngens commentedSame issue. Subscribing.
Comment #25
danSamara CreditAttribution: danSamara commentedPatch worked for me. Commit it, please )
Comment #26
catchThere'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?
Comment #27
Dave ReidThe 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:
Comment #28
catchDave 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.
Comment #29
sunThis is correct. Ideally, we'd add a @see drupal_prepare_form(), but I don't really care about the inline docs in older releases.
Comment #30
praseodym CreditAttribution: praseodym commentedThis patch silently breaks uploading for me (i.e. no errors, but nothing gets uploaded either).
Comment #31
monotaga CreditAttribution: monotaga commentedThe 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?
Comment #32
rares CreditAttribution: rares commentedYes, 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.Comment #33
Gábor HojtsyCommitted this, thanks for testing.
Comment #34
lyd CreditAttribution: lyd commentedI 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?
Comment #35
Gábor Hojtsy@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.
Comment #36
jmoughon CreditAttribution: jmoughon commentedI 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.
Comment #37
philosurfer CreditAttribution: philosurfer commentedPatch @ #14 worked perfect on Atrium 1.0 install.
Comment #39
TechNikh CreditAttribution: TechNikh commentedThe patch in #14 didn't work for me. any suggestions?
Open Atrium 1.0
Drupal 6.22
I get these warning messages..
Comment #40
WebmistressM CreditAttribution: WebmistressM commentedI have the same issue. Using Drupal 6
Comment #41
Ludwig CreditAttribution: Ludwig commentedI 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:-)
Comment #42
TechNikh CreditAttribution: 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?
Comment #43
TechNikh CreditAttribution: TechNikh commentedediting includes/common.inc as per http://drupal.org/node/710892#comment-3884210 , fixed the warning in my case
Comment #44
memcinto CreditAttribution: memcinto commentedApplying 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.
Comment #45
memcinto CreditAttribution: memcinto commentedPatch 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.)
Comment #46
kamranzafar CreditAttribution: kamranzafar commentedThanks memcinto, I also had to apply a patch to common.inc, from http://drupal.org/node/710892#comment-3884210. It worked for me.