Use of get_comment() function shows Notice

[WisdmLabs]

The get_comment_ID() ​https://codex.wordpress.org/Function_Reference/get_comment_ID function throws following Notice while calling -

Trying to get property of non-object in <WP DIR>/wp-includes/comment-template.php on line 646

I see that this function internally makes call to the get_comment() ​https://codex.wordpress.org/Function_Reference/get_comment function which supposed to be a parameterized function.

When get_comment() (without parameter) is used, it returns nothing (not even any warning, error or notice) but when it is used somewhere else in a variable which is then used as an object to retrieve something from it, it shows the Notice. Obviously, since there is no value in it, it would be showing the Notice.

e.g. $comment->comment_ID at
https://core.trac.wordpress.org/browser/tags/4.4.1/src/wp-includes/comment-template.php#L646

$comment->comment_author_email at
https://core.trac.wordpress.org/browser/tags/4.4.1/src/wp-includes/comment-template.php#L183

[boonebgorges]

Hi @WisdmLabs - Thanks for the ticket.

When get_comment() (without parameter) is used, it returns nothing (not even any warning, error or notice)

This is not entirely correct. When used without parameter, and the $comment global is set, it will return the $comment object. Functions like get_comment_author_email_link() - which you linked to above - are designed to be used inside of comment template loops, where this global will be set.

In what context are you calling get_comment() without a parameter? Are you using get_comment_author_email_link() et al outside of a comment loop? If so, what would you *expect* to happen? It seems to me that the notice is a helpful reminder that you're doing something wrong.

[WisdmLabs]

Hi @boonebgorges,

Thanks for your reply. Here is how the get_comment() function is being called in my case -

I am using wp_list_comments() which has a callback function. This callback function has global $comment set (which works fine) and the comment_reply_link() function to display the Reply link. Whenever this Reply link is clicked, it shows the following Notice -

Trying to get property of non-object in <WP DIR>/wp-includes/comment-template.php on line 646

When I debugged this, I found that it uses the get_comment() function which is causing the notice and that notice is being displayed after clicking on Reply link in the comment box. I have added the comment-reply.js which loads the comment reply box via JavaScript and PHP call is not made and hence that notice is not visible which is fine for me. But was just wondering if this is some issue in the core where some validation is not handled yet.

[boonebgorges]

Replying to WisdmLabs:

Hi @boonebgorges,

Thanks for your reply. Here is how the get_comment() function is being called in my case -

I am using wp_list_comments() which has a callback function. This callback function has global $comment set (which works fine) and the comment_reply_link() function to display the Reply link. Whenever this Reply link is clicked, it shows the following Notice -

Trying to get property of non-object in <WP DIR>/wp-includes/comment-template.php on line 646

When I debugged this, I found that it uses the get_comment() function which is causing the notice and that notice is being displayed after clicking on Reply link in the comment box. I have added the comment-reply.js which loads the comment reply box via JavaScript and PHP call is not made and hence that notice is not visible which is fine for me. But was just wondering if this is some issue in the core where some validation is not handled yet.

Can you provide a full stack trace leading up to the error? You said (as I expected) that you're having no problems when the global $comment is set, but that you have problems after clicking a link; but I'm not sure where you're going or what's being fired after the link is clicked.

[d4z_c0nf]

Hello guys,
I was about to open a new ticket for the same issue. As @boonebgorges says get_comment() and similar are intended to be used inside the "comment loop", so that's not the problem.
For my understanding the problem is here:
https://core.trac.wordpress.org/browser/tags/4.4.2/src/wp-includes/comment-template.php#L1859

Called by:
https://core.trac.wordpress.org/browser/tags/4.4.2/src/wp-includes/comment-template.php#L2219

When does this happen?
When you reply to a comment but you haven't ran (or you're outside te 'comment loop') the comment query ( global $comment is not set => get_comment_ID() breaks).
For example when you show(call) the comment form before the list of comments (wp_list_comments()) and you reply to a comment without using js.
I mean, that's when you get the notice, but overall this line:

$author = ( $linktoparent ) ? '<a href="#comment-' . get_comment_ID() . '">' . get_comment_author( $comment ) . '</a>' : get_comment_author( $comment );

is wrong even if you call the comment form after the list of comments (always without using js), 'cause in that case global {{{comment}} is set, but points to the latest comment, not to the one you're replying to.

The fix is simple. In the line above replace get_comment_ID() with the already known $replytoid.

I'm also not sure about this:
https://core.trac.wordpress.org/browser/tags/4.4.2/src/wp-includes/comment-template.php#L1848

as that variable is not used in that function if not in the "reply" block, where it's retrieved again but passing the id of the comment to reply to (.. but maybe I'm missing something).

Here's a simple way to reproduce the issue with a standard theme (I know .. I'm lazy :D):

• use Twentysixteen theme
• in its comments template (comments.php) move the comment comment form call (comment_form(...)) from the current position to before wp_list_comments(...) call
• go in a page/post with comments
• be sure you're not using the comment-reply.js (e.g disabling js with google-chrome inspect element *)
• click on a reply button

p.s.
will attach a patch agains the latest trunk

*
another way to reproduce this without disabling js or dequeuing the comment-reply.js is accessing the page/post with comments + clicking on a reply button from the Customizer, it will reload the preview due to a bug (I think) of the preview js dealing with links to anchors .. but this is another story :)

[boonebgorges]

After a bit of banging my head on the desk, I figured out what's going on here.

[33963] was part of an effort to stop using global $comment to fetch the current comment, in favor of get_comment(). In the case of comment_form_title(), this change broke what was previous a funny "feature', where the line $comment = get_comment($replytoid); would assign that comment to the $comment global. I would have to look at the history, but I assume that this was intentional, as it allows comment template tags to be used in the reply form.

As such, I believe that restoring the previous behavior will fix all of the issues described above. See 35624.diff​. Can I get verification from others?

[d4z_c0nf]

Thanks @boonebgorges I see what you mean.
So this way, when a $replytoid exists, you overwrite (well you set it) the global $comment.
Isn't this a little weird? I mean a function which should just print a string also changes a global variable?

I still think that since you already have the comment id ($replytoid) there's no sense to retrieve it again with get_comment_ID(). But most likely there's something I don't know ;)

Thanks for looking into it. :)

[boonebgorges]

Replying to d4z_c0nf:

Thanks @boonebgorges I see what you mean.
So this way, when a $replytoid exists, you overwrite (well you set it) the global $comment.
Isn't this a little weird? I mean a function which should just return a string also changes a global variable?

I still think that since you already have the comment id ($replytoid) there's no sense to retrieve it again with get_comment_ID(). But most likely there's something I don't know ;)

Thanks for looking into it. :)

I agree that this is not elegant software design (or, at least, it's not very transparent). But I assume that it was originally a conscious choice to overload the global here, so that template tags would be usable when rendering the rest of the form.

Your suggested fix for comment_form_title() would fix comment_form_title(), but wouldn't address any of the other uses of comment-template.php functions within the context of a comment reply form, such as the one (get_comment_author_email_link()) that prompted this ticket to be opened in the first place. By restoring the previous behavior - ie setting the global - we solve all of the problems at once.

[d4z_c0nf]

Replying to boonebgorges:

Replying to d4z_c0nf:

Thanks @boonebgorges I see what you mean.
So this way, when a $replytoid exists, you overwrite (well you set it) the global $comment.
Isn't this a little weird? I mean a function which should just return a string also changes a global variable?

I still think that since you already have the comment id ($replytoid) there's no sense to retrieve it again with get_comment_ID(). But most likely there's something I don't know ;)

Thanks for looking into it. :)

I agree that this is not elegant software design (or, at least, it's not very transparent). But I assume that it was originally a conscious choice to overload the global here, so that template tags would be usable when rendering the rest of the form.

Your suggested fix for comment_form_title() would fix comment_form_title(), but wouldn't address any of the other uses of comment-template.php functions within the context of a comment reply form, such as the one (get_comment_author_email_link()) that prompted this ticket to be opened in the first place. By restoring the previous behavior - ie setting the global - we solve all of the problems at once.

Yeah you're absolutely right about the template tags thing.
My .. "I still think.." was just referring to that other point (in addition) => not using get_comment_ID() when you already have the ID. But I suppose it could make sense too because of the filter hook.

Thanks for your time :)

[d4z_c0nf]

Just to be more accurate:
@WisdmLabs didn't say that get_comment_author_email_link() prompted this ticket to be opened in the first place :)
He identified get_comment() as culprit (as the ticket title reads), and referenced two functions which calls it.
Infact he didn't say he was using any template tag within the context of a comment reply form, he said he used a wp_list_comments() callback, which, per se, cannot generate that issue (as it's in the comments loop) :D
He also restricted the issue to a specific case : replying to a comment without the js handling.
So, for my understanding, the issue was/is, in his and my case, only the one inside comment_reply_title().

Also if he had used one of those template tags within the context of the comment form he'd have faced the same issue, not only while replying, but just loading the comment form (always when before the comments loop), 'cause when not replying the global $comment is unset. To avoid that he should have conditioned the use of such template tags to the state of the the global $comment, and if so he wouldn't have faced the 'Notice' while replying too.

For the record, that check will still be needed (when not replying) with @boonebgorges patch.

[boonebgorges]

In 36512:

Set the $comment global in comment_form_title().

In [33963], comment_form_title() was refactored so that it no longer made
reference to the $comment global. This broke some functionality within the
comment form, as certain template would no longer be able to access the
"current" comment.

Props d4z_c0nf, WisdmLabs, boonebgorges.
Fixes #35624.

[WisdmLabs]

Hi @boonebgorges,

I just checked the fix [36512]. It works. Thanks!!!

Thanks @d4z_c0nf, for bringing out the steps to replicate the issue in a detailed format.

[DrewAPicture]

In 36527:

Docs: Add an internal note to the DocBlock for comment_form_title() explaining restoration of the $comment global.

See [36512]. See #35624.