Experiencing two errors when using php 5.3:

warning: Parameter 1 to theme_wysiwyg_profile_overview() expected to be a reference, value given in /Library/WebServer/Documents/drupal6/includes/theme.inc on line 617.

warning: Parameter 1 to theme_wysiwyg_admin_button_table() expected to be a reference, value given in /Library/WebServer/Documents/drupal6/includes/theme.inc on line 617.

Removing the & on line 325 and 460 of wyswiwyg.admin.inc fixes the warning.

Cheers

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mobo1’s picture

Hello,

in line 460 is no " &"

line 460 is looking so:

460 }

thx for help

brainajw’s picture

Priority: Normal » Critical

I'm having the same issue.

Error:warning: Parameter 1 to theme_wysiwyg_profile_overview() expected to be a reference, value given in C:\wamp\www\mysite\includes\theme.inc on line 617.

Code from 319 -333
/**
* Submit callback for Wysiwyg profile form.
*
* @see wysiwyg_profile_form()
*/
function wysiwyg_profile_form_submit($form, &$form_state) {
$values = $form_state['values'];
if (isset($values['buttons'])) {
// Store only enabled buttons for each plugin.
foreach ($values['buttons'] as $plugin => $buttons) {
$values['buttons'][$plugin] = array_filter($values['buttons'][$plugin]);
}
// Store only enabled plugins.
$values['buttons'] = array_filter($values['buttons']);
}

Code from 452 - 465

$formats = filter_formats();
$profiles = wysiwyg_profile_load_all();
$form['formats']['#tree'] = TRUE;
foreach ($formats as $id => $format) {
$form['formats'][$id]['name'] = array(
'#value' => check_plain($format->name),
);
$form['formats'][$id]['editor'] = array(
'#type' => 'select',
'#default_value' => isset($profiles[$id]) ? $profiles[$id]->editor : '',
'#options' => $options,
'#id' => "edit-editor-$id",
'#disabled' => isset($profiles[$id]) ? (bool) $profiles[$id]->editor : FALSE,
);

Any ideas?

TwoD’s picture

Priority: Critical » Normal
Status: Active » Needs review

Not critical. The module still works, the profile overview is just compressed and the button/plugin list just has one large column.

I now have access to a PHP 5.3 server and the simple changes in this patch fixes it. It also compatible with 5.2 but I have no idea about PHP 4 yet.

TwoD’s picture

FileSize
917 bytes
Dave Reid’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

TwoD’s picture

@Dave Reid, does that include when using PHP 4?

Dave Reid’s picture

Yes it will work just fine.

sun’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for reporting, reviewing, and testing! Committed to all branches.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

maximpodorov’s picture

I think one more change is required:

-function wysiwyg_form_alter(&$form, &$form_state) {
+function wysiwyg_form_alter(&$form, $form_state) {

Cousken’s picture

Hi, i had this issue and applying the patch seems to have solved it. Unless there are some buttons or some such supposed to be visible on the configuration page. I just have a list of the installed modules, never seen WYSIWYG in action before.

sun’s picture

@maximpodorov: No, $form_state is always passed by reference.

maximpodorov’s picture

According to
http://api.drupal.org/api/function/hook_form_alter/6
$form_state is passed by value (for Drupal 6).

DrumLib’s picture

How do you apply this patch? I'm testing XAMPP and Drupal on a USB key installation.

TwoD’s picture

@DrumLib, the patch has already been committed to CVS, no need to apply it. Just download the -dev snapshot. This will become part of release 2.1.

maximpodorov’s picture

Excuse me for the annoyance, but not using #9 patch leads to
warning: Parameter 2 to wysiwyg_form_alter() expected to be a reference, value given in ...
message.

TwoD’s picture

Curious, I'm not seeing that warning on the PHP 5.3 site I'm testing on.

maximpodorov’s picture

I saw it when pressed "Add more item" button for cck image field at node edit form with wysiwyg editor.

Berdir’s picture

@17

In that case, the one that calls the function is doing something wrong. See http://drupal.org/node/360605#comment-2335898 for a way to figure out who is causing this. (You need to paste that code inside wysiwyg_form_alter(), of course.

makatozi’s picture

The Patch in #4 worked well. XAMPP 1.7.3 Thanks!

maximpodorov’s picture

Actually, is't CCK who initiate addressing to wysiwyg_form_alter in this case. The code in cck/includes/content.node_form.inc (content_add_more_js function):
...
drupal_alter('form', $form_element, array(), 'content_add_more_js');
...

There're some discussions on this topic:
http://drupal.org/node/705512
http://drupal.org/node/649106

If I understood correctly, it's safe to remove ampersand from $form_state parameter. BTW, hook_form_alter implementations in the core Drupal modules follow the API documentation:

book_form_alter(&$form, $form_state, $form_id)
color_form_alter(&$form, $form_state, $form_id)
comment_form_alter(&$form, $form_state, $form_id)
forum_form_alter(&$form, $form_state, $form_id)
locale_form_alter(&$form, $form_state, $form_id)
menu_form_alter(&$form, $form_state, $form_id)
node_form_alter(&$form, $form_state, $form_id)
openid_form_alter(&$form, $form_state, $form_id)
path_form_alter(&$form, $form_state, $form_id)
taxonomy_form_alter(&$form, $form_state, $form_id)
translation_form_alter(&$form, $form_state, $form_id)
update_form_alter(&$form, $form_state, $form_id)
upload_form_alter(&$form, $form_state, $form_id)

sun’s picture

Thanks for figuring that out!

#705512: drupal_alter('form'... called from content_add_more_js needs to pass $form_state as a reference seems to be the cause. And contains a patch that looks like it just needs confirmation + testing + RTBC.

sun’s picture

Issue tags: +PHP 5.3

Tagging.

maximpodorov’s picture

Excuse me again, but is there any reason to declare wysiwyg_form_alter function as getting $form_state by reference, if it's not modified by the function? I think functions in Drupal core modules mentioned in #20 may be thought as reference.

sun’s picture

As of now, it is technically not needed by Wysiwyg, so as a quick/dirty fix, you could remove the ampersand. However, $form_state is always passed by reference, so changing it here would be wrong. Instead, we need to push that patch for CCK.

Also note that http://drupal.org/project/issues/search?issue_tags=PHP+5.3 provides you a nice list of all PHP 5.3 issues on drupal.org.

b3liev3’s picture

@sun is right.

please read this http://drupal.org/node/705512

Status: Fixed » Closed (fixed)

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

roball’s picture

Version: 6.x-2.x-dev » 6.x-2.4
Category: feature » support
Status: Closed (fixed) » Active

I am still getting

Parameter 2 to wysiwyg_form_alter() expected to be a reference, value given in /usr/share/drupal6/includes/common.inc on line 2892.

PHP errors, immediately after having logged in. Any ideas which module could cause this?

roball’s picture

Status: Active » Closed (fixed)

Found the originating module: One page profile. Once I have disabled it, the error went away. Already found the issue reported there.

florian.cathala’s picture

Status: Closed (fixed) » Needs review
FileSize
476 bytes

Patching CCK does not fix the error reported by roball (#27).

Here is the patch as attached file.

florian.cathala’s picture

Here is the fixed patch.

TwoD’s picture

Status: Needs review » Closed (fixed)

That patch is wrong, see #24. Please fix the module which does not pass $form_state by reference instead.
Patching CCK (no longer needed btw) will not always fix the issue because it might not be the offending module, as roball noted in #28.

See the documentation for drupal_alter() on passing alterable arguments through it.

memcinto’s picture

How did you trace it back to the originating module? That's what I'm having trouble with.