API page: http://api.drupal.org/api/drupal/modules--node--node.api.php/function/ho...

Describe the problem you have found:

The signature for hook_node_submit is not consistent with expected behavior. According to the comment in function node_form_submit_build_node:

  // @todo Legacy support for modules that extend the node form with form-level
  //   submit handlers that adjust $form_state['values'] prior to those values
  //   being used to update the entity. Module authors are encouraged to instead
  //   adjust the node directly within a hook_node_submit() implementation. For
  //   Drupal 8, evaluate whether the pattern of triggering form-level submit
  //   handlers during button-level submit processing is worth supporting
  //   properly, and if so, add a Form API function for doing so.

Note that "Module authors are encouraged to instead adjust the node directly within hook_node_submit...". However, according to the documentation the signature for hook_node_submit takes $node as a copy and not a reference. Functionally, module implementors can simply supply the hook with node as a reference (e.g. hook_node_submit(&$node...) and indeed modify the node as necessary.

Reco: Add the '&' in front of the $node parameter in the signature in the documentation.

Comments

jhodgdon’s picture

Title: Documentation problem with hook_node_submit » hook_node_submit doc is not quite correct

Thanks for reporting this!

mr.baileys’s picture

Version: 7.x-dev » 8.x-dev
Status: Active » Needs review
Issue tags: +Needs backport to D7
StatusFileSize
new834 bytes

No, I think the current signature is correct: $node is an object, and PHP passes the $node object via its object identifier (similar to but not the same as a reference) to the hook implementation, allowing the object to be changed. Using &$node would actually mean passing a reference to the object identifier rather than the object itself. The same happens all over core, see hook_entity_presave for a simple example.

See http://mjtsai.com/blog/2004/07/15/php-5-object-references/ for additional information.

Patch attached adds a tiny clarification to the docs to stress that $node is an object.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Correct! I must have been nearly asleep when I first triaged this issue. :)

This clarification seems fine to me. Please commit to 8.x and 7.x.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x and 8.x. Thanks!

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