Object's are passed by reference by default, so there's no need to state that the first parameter to webform_node_view() is a reference.

In fact, doing so displays the following warning:

Warning: Parameter 1 to webform_node_view() expected to be a reference, value given in module_invoke_all() (line 708 of www/includes/module.inc).

CommentFileSizeAuthor
#4 hook-parameters-765308-4.patch983 bytesjohnalbin
#1 hook-node-view-765308-1.patch424 bytesjohnalbin

Comments

johnalbin’s picture

johnalbin
they/他
Primary language English
Location Taipei, Taiwan
commented
Status: Active » Needs review
StatusFileSize
new hook-node-view-765308-1.patch424 bytes

Here's the patch.

quicksketch’s picture

quicksketch
he/him
commented

While true, it sounds like something is being passed into webform_node_view() that cannot be passed by reference at all (such as NULL) which is causing that error. Manually specifying the ampersand may not be necessary, but it shouldn't be causing such a warning.

johnalbin’s picture

johnalbin
they/他
Primary language English
Location Taipei, Taiwan
commented
Title: first paramter to webform_node_view() is object, not reference » hook_node_view and hook_node_load should not use & in their parameters
Priority: Normal » Critical
Status: Needs review » Needs work

This turns out part of a bigger problem. On a clean CVS install with PHP 5.3.1, when I try to add a component to a webform-enabled node type, I get the following failure:

Warning: Parameter 1 to webform_node_load() expected to be a reference, value given in DrupalDefaultEntityController->attachLoad() (line 255 of includes/entity.inc).

Notice: Undefined property: stdClass::$webform in webform_component_edit_form() (line 409 of sites/all/modules/webform/includes/webform.components.inc).

Notice: Undefined property: stdClass::$webform in webform_component_edit_form() (line 438 of sites/all/modules/webform/includes/webform.components.inc).

Warning: Invalid argument supplied for foreach() in webform_component_edit_form() (line 438 of sites/all/modules/webform/includes/webform.components.inc).

Notice: Undefined property: stdClass::$webform in webform_component_edit_form_validate() (line 548 of sites/all/modules/webform/includes/webform.components.inc).

Warning: Invalid argument supplied for foreach() in webform_component_edit_form_validate() (line 548 of sites/all/modules/webform/includes/webform.components.inc).

Notice: Undefined index: pid in webform_component_insert() (line 658 of sites/all/modules/webform/includes/webform.components.inc).

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'pid' cannot be null: INSERT INTO {webform_component} (nid, cid, pid, form_key, name, type, value, extra, mandatory, weight) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9); Array ( [:db_insert_placeholder_0] => 120 [:db_insert_placeholder_1] => 1 [:db_insert_placeholder_2] => [:db_insert_placeholder_3] => test [:db_insert_placeholder_4] => test [:db_insert_placeholder_5] => textfield [:db_insert_placeholder_6] => [:db_insert_placeholder_7] => a:3:{s:8:"disabled";i:0;s:6:"unique";i:0;s:20:"conditional_operator";s:1:"=";} [:db_insert_placeholder_8] => 0 [:db_insert_placeholder_9] => 0 ) in webform_component_insert() (line 667 of sites/all/modules/webform/includes/webform.components.inc).

And when just viewing a node of that type I get:

Warning: Parameter 1 to webform_node_load() expected to be a reference, value given in DrupalDefaultEntityController->attachLoad() (line 255 of includes/entity.inc).
Warning: Parameter 1 to webform_node_view() expected to be a reference, value given in module_invoke_all() (line 708 of includes/module.inc).

Ah, crap. I just discovered its a bug in PHP 5.3.1. http://bugs.php.net/bug.php?id=50394 That explains the "expected to be a reference, value given" part of the warning.

I still think webform shouldn't be using & in the params for those two hooks, but I can't really test a fix on this broken PHP 5.3.1 system. (which, btw, is the default PHP on the Mac OS X 10.6.3.)

johnalbin’s picture

johnalbin
they/他
Primary language English
Location Taipei, Taiwan
commented
Priority: Critical » Normal
StatusFileSize
new hook-parameters-765308-4.patch983 bytes

Whoops. The PHP bug is critical. Can't tell the severity of the webform bug.

Here's a patch.

johnalbin’s picture

johnalbin
they/他
Primary language English
Location Taipei, Taiwan
commented
Status: Needs work » Needs review

Needs review!

quicksketch’s picture

quicksketch
he/him
commented

I still can't telly why this is necessary or reproduce any problems with the current approach. Could you provide any way to reproduce an error?

Stevel’s picture

Stevel commented

I've reproduced this bug with 7.x-3.0-beta5, and drupal 7 beta 5. The PHP-version is 5.3.2, the default version in the latest Ubuntu LTS version. I got the same as in #3. The patch provided in #4 solves the problems as far as I can tell.

I also agree that apart from this php error, the two mentioned hooks shouldn't use the & as per http://api.drupal.org/api/function/hook_node_load/7 and http://api.drupal.org/api/function/hook_node_view/7

Stevel’s picture

Stevel commented
Status: Needs review » Reviewed & tested by the community
quicksketch’s picture

quicksketch
he/him
commented
Category: bug » task
Status: Reviewed & tested by the community » Fixed

Ah well, even though I'd argue these changes are 100% unnecessary and indicative of a larger problem with either the implementation around Webform or (in this case apparently) directly with PHP. The & shouldn't make any difference at all.

But as is these lines are changed anyway between the D6 and D7 versions so I can't really make a claim for consistency between branches. Committed to D7 branch.

Status: Fixed » Closed (fixed)

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