in comment.module line 1184 the api documentation says "validate" can "even modify the comment"
but the $edit is passed by value and not by reference .
the declaration should be comment_validate(&$edit)
so that comment values could be changed in the validation stage before being saved

CommentFileSizeAuthor
#11 hook_comment_doc-728300.patch938 bytesbarbi
#1 comment.module.hook_comment_reference.patch588 bytesjmcclelland

Comments

jmcclelland’s picture

jmcclelland commented
StatusFileSize
new comment.module.hook_comment_reference.patch588 bytes

I've found the same problem.

The attached patch fixes it.

jamie

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Active » Postponed (maintainer needs more info)

Where is this documentation that says it should be passed by reference?

yakoub’s picture

yakoub commented

http://api.drupal.org/api/drupal/developer--hooks--core.php/function/hoo...

"validate": The user has just finished editing the comment and is trying to preview or submit it. This hook can be used to check or even modify the comment

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Postponed (maintainer needs more info) » Closed (works as designed)

I don't see a problem here.

If you define
mymodule_comment(&$edit, $a2)
then you will receive $edit by reference.

I'm closing this... feel free to reopen and explain the problem you are seeing, if there is a problem.

yakoub’s picture

yakoub commented

here is the code from line 1184

function comment_validate($edit) {
  global $user;

  // Invoke other validation handlers
  comment_invoke_comment($edit, 'validate');

comment_validate receives a copy of $edit , and calls hook_comment($edit,'validate');
if i declare mymodule_comment(&$edit..); , i receive a reference to a copy that comment_validate received
and not the original $edit array that will be saved latter

yakoub’s picture

yakoub commented
Status: Closed (works as designed) » Needs review

i am not sure how to "reopen" , hope i got it right

Status: Needs review » Needs work

The last submitted patch, comment.module.hook_comment_reference.patch, failed testing.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Title: validate parameter » hook_comment doc says on validate you can modify the comment - this is wrong
Component: comment.module » documentation
Status: Needs work » Active
Issue tags: +Novice

Ah, ok. Good catch. This does need a documentation fix (not a code fix -- we don't change the behavior of the code in Drupal 6 except for major problems, and this isn't one).

The doc for hook_comment() currently says:
"validate": The user has just finished editing the comment and is trying to preview or submit it. This hook can be used to check or even modify the comment. Errors should be set with form_set_error().

The part about "or even modify" needs to be removed.

jack.r.abbit’s picture

jack.r.abbit commented

ARGH! Doc is still not changed. I wish I'd found this ticket before banging my head against the problem for a bit. I need to modify the comment after submit and was sooooo happy to have the docs tell me it was simple. The doc lies. :)

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Someone needs to create a patch and then it can be fixed in the doc...

barbi’s picture

barbi commented
Assigned: Unassigned » barbi
Status: Active » Needs review
StatusFileSize
new hook_comment_doc-728300.patch938 bytes

Status: Needs review » Needs work

The last submitted patch, hook_comment_doc-728300.patch, failed testing.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Project: Drupal core » Documentation
Version: 6.x-dev »
Component: documentation » API documentation files
Status: Needs work » Reviewed & tested by the community

The patch is fine. It failed to apply because the file it is patching is currently part of the Documentation project in D6, not the Drupal Core project. Needs to be committed to the Doc project.

jn2’s picture

jn2 commented
Assigned: barbi » Unassigned
Status: Reviewed & tested by the community » Fixed

This is fixed, patch committed to the D6 docs. (It may take a little while to show up.)

Status: Fixed » Closed (fixed)
Issue tags: -Novice

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

Issue tags: +Novice

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.