Skip to:
Content

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#5339 closed defect (bug) (fixed)

comments_template() runs during theme compatibility

Reported by: r-a-y Owned by: johnjamesjacoby
Milestone: 1.9.2 Priority: normal
Severity: normal Version:
Component: Core Keywords: has-patch
Cc:

Description

r7735 changed the reset post arguments so 'is_page' is set to true.

If a theme's page.php template uses comments_template(), that function will now run since 'is_page' is set to true:
https://core.trac.wordpress.org/browser/tags/3.8/src/wp-includes/comment-template.php#L1020

Previously, this did not run at all since 'is_archive' was set.

What happens is a comments loop is shown when a user is logged out. (Props to imath for testing.) This obviously isn't great.

I've come up with two ways to address the issue.

1) toggle_is_page.patch toggles the 'is_page' and 'is_single' flags at the very end of object buffering. This forces comments_template() to bail out of rendering the rest of that function, which is great because we don't want any additional DB queries or requiring to load the comments template. I tried some other things like toggling the $withcomments global to false, but that did not work.

2) current_comment.patch alters bp_theme_compat_reset_post() so $wp_query->current_comment is greater than $wp_query->comment_count in . This forces have_comments() to always be false. However, this will still load up the entire comments_template() function, which will lead to the DB queries and loading of the comments template I mention in point 1.


I've tested this Twenty Twelve, but you can test with any theme calling comments_template() in their post loop in page.php.

I'm an advocate of point 1, but it isn't the most elegant piece of code ever. Thoughts?

Attachments (2)

5339.toggle_is_page.patch (2.0 KB) - added by r-a-y 4 years ago.
5339.current_comment.patch (581 bytes) - added by r-a-y 4 years ago.

Download all attachments as: .zip

Change History (6)

#1 @boonebgorges
4 years ago

toggle_is_page is, as you not, not very beautiful. But it seems like the most general solution to this problem. There could be other functionality out there that's checking is_page() in the same way as comments_template(), and the toggle will catch all of them.

#2 @imath
4 years ago

Hi r-a-y, Boone,

i agree, i think "current_comment" patch is not great as some code and queries are run in WordPress comments_template() function so "toggle_is_page" patch is better as it avoids this.

I've tested "toggle_is_page" patch, and i confirm it's fixing the issue, thanks r-a-y :)

#3 @johnjamesjacoby
4 years ago

  • Owner set to johnjamesjacoby
  • Resolution set to fixed
  • Status changed from new to closed

In 7793:

Word around for comments_template() running when in theme compatibility mode. Props r-a-y.Fixes #5339. (1.9 branch)

#4 @johnjamesjacoby
4 years ago

In 7794:

Word around for comments_template() running when in theme compatibility mode. Props r-a-y. Fixes #5339. (trunk)

Note: See TracTickets for help on using tickets.