WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#9211 closed defect (bug) (fixed)

Recent comments widget with "private" and draft entries

Reported by: menelicte Owned by:
Milestone: 2.9 Priority: lowest
Severity: trivial Version: 2.7
Component: Widgets Keywords: has-patch tested
Focuses: Cc:

Description

Issue:

the wp 2.7.1 standard widget for the latest comments lists comments that are posted to a private entry, even if the user has no private entry reading capability.
In this case, if the link is clicked, wordpress does not display the article because the user hasn't privileges.

Context:

a public multi-author site with commented private pages and posts

Solution:

If you're interested I've done a bit of changes (2 vars added, query modified) in wp_widget_recent_comments() in the 'wp-includes/widgets.php' file, to check user capabilities and to show only the list of viewable comments.
The modified code is this:

function wp_widget_recent_comments($args) {
	global $wpdb, $comments, $comment;
	extract($args, EXTR_SKIP);
	$options = get_option('widget_recent_comments');
	$title = empty($options['title']) ? __('Recent Comments') : apply_filters('widget_title', $options['title']);
	if ( !$number = (int) $options['number'] )
		$number = 5;
	else if ( $number < 1 )
		$number = 1;
	else if ( $number > 15 )
		$number = 15;
	$can_read_priv_posts=current_user_can('read_private_posts')?"OR p.post_type='post'":"";       //row inserted
	$can_read_priv_pages=current_user_can('read_private_pages')?"OR p.post_type='page'":"";       //row inserted

	if ( !$comments = wp_cache_get( 'recent_comments', 'widget' ) ) {
		$comments = $wpdb->get_results("SELECT * FROM $wpdb->comments c LEFT JOIN $wpdb->posts p ON c.comment_post_id = p.ID WHERE c.comment_approved = '1' AND (p.post_status<>'private' $can_read_priv_posts $can_read_priv_pages ) ORDER BY c.comment_date_gmt DESC LIMIT $number");       //row modified
		wp_cache_add( 'recent_comments', $comments, 'widget' );
	}
?>

		<?php echo $before_widget; ?>
			<?php echo $before_title . $title . $after_title; ?>
			<ul id="recentcomments"><?php
			if ( $comments ) : foreach ( (array) $comments as $comment) :
			echo  '<li class="recentcomments">' . sprintf(__('%1$s on %2$s'), get_comment_author_link(), '<a href="' . clean_url( get_comment_link($comment->comment_ID) ) . '">' . get_the_title($comment->comment_post_ID) . '</a>') . '</li>';
			endforeach; endif;?></ul>
		<?php echo $after_widget; ?>
<?php
}

Attachments (5)

widgets.diff (1.0 KB) - added by menelicte 5 years ago.
widgets.2.diff (1.1 KB) - added by mrmist 5 years ago.
Alternative widgets patch
widgets.3.diff (1.1 KB) - added by mrmist 5 years ago.
adds the attachement post type
9211.diff (1.5 KB) - added by Denis-de-Bernardy 5 years ago.
9211.2.diff (778 bytes) - added by Denis-de-Bernardy 5 years ago.
strip out comments on non-public posts

Download all attachments as: .zip

Change History (33)

comment:1 follow-up: DD325 years ago

Can you make that in a Diff form at all?

See also: #9200 (comment widget links draft articles)

comment:2 in reply to: ↑ 1 menelicte5 years ago

Replying to DD32:

Can you make that in a Diff form at all?

I don't know how.
I'll do that, if someone will tell me how

comment:3 DD325 years ago

I meant to link to it.. but forgot, http://codex.wordpress.org/Using_Subversion (However, it only mentions Command line..), Westi's post he linked to is good: http://blog.ftwr.co.uk/archives/2005/11/03/windows-wordpress-toolbox/

Looking at the quoted code a bit closer however:

  • Joins are expensive, It could be more efficient to do a get_post() and a check that the post_status is set to 'publish' or 'inherit'(attachments)
  • Pretty sure WP has a function hidden somewhere to generate the WHERE case for a users private drafts & all that.

I'd be in favour of the first method however, Due to the join benefiting a small amount of people, increasing query time for something that is used on very active blogs should be avoided IMO.. The posts table is queried during the loop for the post title regardless, so a get_post() in the loop wouldn't affect the speed.

I might whip up a patch based on that at some point if that idea seems acceptable to others (Given the closure of the other mentioned ticket)

menelicte5 years ago

comment:4 menelicte5 years ago

Well, now I attached the diff file for the "join" method.

You should be right, but in the first method I see some difficulty in achieving a number of list items equal to the limit specified:
if the query limit is 5 but there are 3 private comments in the Top5, the resulting list will have only 2 items.

comment:5 DD325 years ago

if the query limit is 5 but there are 3 private comments in the Top5, the resulting list will have only 2 items.

If that was the case, I'd just select x+y comments, An extra 5 or so.. The extra memory consumption of the extra comments isn't much, and in the case those extra comments weren't useful, you just display a smaller number. WordPress does this in a few other areas of the code, Includes a buffer, but if its not a hugely important area makes do with less than requested.

Thanks for the diff though, Makes it much more clear whats been added/suggested :)

comment:6 follow-up: mrmist5 years ago

-1 to the current patch.

Honestly for the recent comments widget the existing select * from comments was bad enough, but select * from comments with a left join to posts would be horrific.

Either the SELECT list needs to be made specific, or this needs to be approached in a different way that won't cripple sites.

comment:7 mrmist5 years ago

See also #9200 fairly related.

mrmist5 years ago

Alternative widgets patch

comment:8 mrmist5 years ago

  • Keywords has-patch tested added
  • Milestone changed from Future Release to 2.8

Ok so it looks like the $comments is expecting to be filled with the whole contents of wp_comments, even when it's just populating the dashboard widget.

So given that I've tidied up the query as best I can think to cover off #9200 and this one. It works for me (no more draft comments and private only if you can view private stuff) though I have not exhaustively tested it against any possible regressions.

I still don't like it all that much ;)

comment:9 mrmist5 years ago

  • Summary changed from Recent comments widget with "private" entries to Recent comments widget with "private" and draft entries

mrmist5 years ago

adds the attachement post type

comment:10 mrtorrent5 years ago

Patch #3 works fine for me and seems reasonable, although it does not deal with "pending" or "draft" entries. I'm not sure that it should, anyway.

comment:11 mrmist5 years ago

It should deal with draft entries. Are you logged in? You'll probably still see them if logged in.

comment:12 mrmist5 years ago

Actually re-looking at the code you shouldn't be seeing draft status comments at all. Are you sure about that?

comment:13 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added; widget comments has-patch tested removed

None of these patches are valid imo.

As things currently stand, the comments get cached. If the admin is the first to view the widget, his set of comments (including private ones) get cached.

The cache should only be filled and served if the user has the most generic permissions. (The same kind of bug affects recent posts, btw.)

comment:14 mrmist5 years ago

Good point, I forgot about the caching potential.

Unfortunately I do not understand how the WordPress cache system works, so I can't update my patch to compensate.

comment:15 in reply to: ↑ 6 Denis-de-Bernardy5 years ago

like this:

wp_cache_set($identifier, $bucket);

wp_cache_delete($identifier, $bucket);

in this case, it would require a $widget_id as an identifier, and widgets as the bucket.

comment:16 Denis-de-Bernardy5 years ago

you'd want to skip the caching process entirely (or create a separate argument in the md5(serialize()) that generates a cache id) when the user can edit private posts or pages.

comment:17 Denis-de-Bernardy5 years ago

  • Milestone changed from 2.8 to 2.9

patch needs to be refreshed, moving to 2.9

Denis-de-Bernardy5 years ago

comment:18 Denis-de-Bernardy5 years ago

  • Keywords has-patch needs-testing added; needs-patch removed
  • Milestone changed from 2.9 to 2.8

attached patch needs some testing

comment:20 Denis-de-Bernardy5 years ago

See also: #9144 (it's the same ticket, but for the dashboard)

comment:21 Denis-de-Bernardy5 years ago

  • Keywords tested commit added; needs-testing removed

comment:22 Denis-de-Bernardy5 years ago

  • Owner set to Denis-de-Bernardy
  • Status changed from new to accepted

comment:23 Denis-de-Bernardy5 years ago

  • Keywords early added
  • Milestone changed from 2.8 to 2.9

comment:24 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added; has-patch tested commit early removed

Quick aside... I have a plugin that displays recent comments, and I've opted to ignore this stuff altogether. I only display comments on comments on non-private posts. It makes caching easier, and it's more efficient from an SQL standpoint.

Denis-de-Bernardy5 years ago

strip out comments on non-public posts

comment:25 Denis-de-Bernardy5 years ago

  • Keywords has-patch tested added; needs-patch removed

comment:26 Denis-de-Bernardy5 years ago

  • Owner Denis-de-Bernardy deleted
  • Status changed from accepted to assigned

comment:27 markjaquith4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In [12333] Do not display comments on non-public posts in comments widget. props Denis-de-Bernardy. fixes #9211

comment:28 voyagerfan57614 years ago

  • Cc WordPress@… added
Note: See TracTickets for help on using tickets.