Closed (works as designed)
Project:
Drupal core
Version:
7.x-dev
Component:
comment.module
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
17 Nov 2010 at 06:31 UTC
Updated:
23 Nov 2010 at 22:19 UTC
Jump to comment: Most recent file
Comments
Comment #1
joachim commentedSounds like a regression from D6 to me. Changing to bug report.
Comment #2
mattyoung commentedAdd an issue tag so we don't go to RC1 and forget about this
Comment #3
mattyoung commentedwould something like this work?
Comment #4
joachim commentedDon't have time to test sorry, but mind the whitespace, and maybe also add a comment like 'Invoke hook_comment_validate blah blah'.
Comment #5
joachim commentedOh and something in comment.api.php too.
Comment #6
bfroehle commentedcomment_validate() has been removed:
#335034: refactor comment validate/save logic: Use comment_form_validate() for all form-related validation of comments.
Did I miss something? I suggest closing the (non)-issue.
Comment #7
mattyoung commented>Use comment_form_validate() for all form-related validation of comments
comment_form_validate()is the validation function for the "comment_form" in comment.module. There is no "hook" to give other modules a chance validate.Also the patch in #335034: refactor comment validate/save logic doesn't seem to match what's in comment.module currently. For example, there is no
function comment_invoke_comment()in comment.module right now but it's in the patch.I don't understand this at all. As is, I think we need to have a
hook_comment_validate().Here is a new patch. This one makes
hook_comment_validat()work like D6'shook_comment($op == 'validate')Comment #8
bfroehle commentedLook to see how the mollom module does it. Use hook_form_alter and add your validation to
$form['#validate'].Comment #9
mattyoung commentedMollom is not a good example because it needs to insert its validation into lots of forms so it's appropriate for it to use form_alter() to do its things uniformly there. What we have here is specifically letting modules validate comment on submit/preview.
hook_comment($op == 'validate')exists in D6, there is no reason to remove it . If removing that make sense, then why not also removehook_node_validate()and go usehook_form_alter()instead?Comment #10
mattyoung commentedHello, can we get this in?
Comment #11
marcingy commentedPatch has white spaces and a typo. Plus why are we doing this
rather than
Comment #12
mattyoung commentedChanged and fixed typo. But I don't know what the white spaces are.
Comment #13
joachim commentedWhitespace means empty spaces at the end of lines of code.
There's a space at the end of the middle line here.
Sentence case here.
This function doesn't actually exist, does it? I don't think it's a good idea to put fictional functions into the API examples.
Sentence case again.
Indentation wrong here.
I'm sure this seems like nitpicking to you -- it used to me too ;) -- but this level of attention to detail means that the whole of the code in Drupal core is consistent, which in turn makes it much clearer to read and work with.
Powered by Dreditor.
Comment #14
bfroehle commentedIf you really want to pursue this, I think you'd have to also use
LANGUAGE_NONEinstead of'und'for consistency and clarity.However, please see #512492: Remove hook_comment_validate(). I don't think this patch fits with the current direction Drupal is heading.
Comment #15
damien tournoud commentedThis is definitely by design. The hook was purposely removed by #512492: Remove hook_comment_validate().
Comment #16
mattyoung commentedBy design? So to validate comment, you have to write two functions. One of them is boiler plate mumbo jumbo. This doesn't make sense to me. I don't think having a dedicated comment_validate hook is confusing. To the contrary, it make thing clearer and easier for the reader of the code to understand. This also makes comment hooks consistent with node hooks: there is a hook_node_validate().