The change in comment_notify.module 1.1.4.3 causes PHP errors when submitting the comment. The problem is that the $comment passed to _comment_notify_mailalert2() is actually an array, not an object. It comes from the patch to comment.module, "comment_invoke_comment($edit, 'publish')" where $edit is an array.

CommentFileSizeAuthor
#7 php_object_array_error_191313.patch1.12 KBgreggles
#1 comment_notify.module.patch577 bytesmichal.cihar

Comments

michal.cihar’s picture

michal.cihar commented
StatusFileSize
new comment_notify.module.patch577 bytes

Attaching patch which fixes this issue for me.

curtbeckmann’s picture

curtbeckmann commented

I think I encountered this same issue, but not 100% sure. Still, I thought it fair to add the full error message that I received:

* warning: Invalid argument supplied for foreach() in /home/.jong/curtbeckmann/worldenhancing.org/forums/drupal-5.1/modules/node/node.module on line 521.
* warning: implode() [function.implode]: Bad arguments. in /home/.jong/curtbeckmann/worldenhancing.org/forums/drupal-5.1/modules/node/node.module on line 525.
* user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1 query: SELECT n.nid, n.vid, n.type, n.status, n.created, n.changed, n.comment, n.promote, n.sticky, r.timestamp AS revision_timestamp, r.title, r.body, r.teaser, r.log, r.format, u.uid, u.name, u.picture, u.data FROM node n INNER JOIN users u ON u.uid = n.uid INNER JOIN node_revisions r ON r.vid = n.vid WHERE in /home/.jong/curtbeckmann/worldenhancing.org/forums/drupal-5.1/includes/database.mysql.inc on line 172.

For experienced folks like michal.cihar, this may not be necessary.
I shall go attempt Michal's patch (if there is enough documentation on patch installation :-))

curtbeckmann’s picture

curtbeckmann commented

I attempted the patch (actually, did it manually using VI after saving the original file), and seems to fix my problem, so I guess it was the same problem :-)

(I used "reply" this time, so I hope my note is indented...sorry about entering my last comment the wrong way)

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Active » Fixed

It appears this was fixed in the latest commit http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/comment_not...

junyor’s picture

junyor commented
Status: Fixed » Active

@greggles: That's the commit that *introduced* this issue!

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Ha, right. I was thinking the reverse. Sorry!

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Active » Needs review
StatusFileSize
new php_object_array_error_191313.patch1.12 KB

Well, actually I think this depends on some other factors, perhpas, like whether you are using comment moderation or not. I use comment moderation and when my $comment variable gets there it is an object.

When I apply the patch as proposed in #1 (actually, slightly modified to use '' around the array keys) and then go to admin/content/comment/list/approval I get the error

Fatal error: Cannot use object of type stdClass as array in /home/.beanor/knaddiso/knaddison.com/sites/all/modules/comment_notify/comment_notify.module on line 343

So....if someone can test this out then I will commit it (cristoph gave me commit access so I can help out with things like this)

junyor’s picture

junyor commented
Status: Needs review » Needs work

I haven't tested, but I had a look at the code. I don't think this is the correct solution. The $comment variable comes from hook_comment(). The documentation for comment_invoke_comment() says the $comment variable is "A comment object". I've checked around in comment.module and this doesn't really seem to be consistent. In other words, I think the correct fix is to get comment_invoke_comment() to always send an object rather than having every module have to convert to object just in case. Fixing this in core will probably be a bit tricky, though, since the casting can't be done in comment_invoke_comment() since the $comment variable is passed by reference. It'll have to be done in each function that calls comment_invoke_comment(), as far as I can tell.

I'm not sure it even makes sense for the $comment variable to be passed by reference. comment_invoke_comment() doesn't pass the variable to the functions it calls by reference, so it's a dead end. The casting probably could be done in comment_invoke_comment() if the pass by reference is removed.

That said, the patch in #7 does solve the PHP error, so it's an acceptable work-around for now.

junyor’s picture

junyor commented
Status: Needs work » Reviewed & tested by the community
greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Cool - thanks for the review. In php5 all objects are always passed by reference which often causes unintended problems. I guess that given that it could be passed by reference all the way down we should be doing a drupal_clone first and then the cast.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Reviewed & tested by the community » Fixed

I applied this to DRUPAL-5. I realized we don't need to worry about cloning because: if it's passed as an array, it's not by reference and that's no problem. If it is passed as an object then it's already an object and the cast doesn't change anything. Sweet.

gabiroba’s picture

gabiroba commented

Hi, I fell it is a bit confusing. I have the problem described in the first post and I'm not a PHP coder.
So can anyone explain me what I should do?

Thanks in advance.
Gabriela

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

I suggest you try the 5.x-1.x-dev release of this module. Let us know if that fixes it for you (or not).

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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