Skip to:
Content

bbPress.org

Opened 3 years ago

Last modified 2 years ago

#2985 new defect

bbp_get_topic_pagination_count() has a short-circuiting return statement

Reported by: mlwilkerson Owned by:
Milestone: 2.7 Priority: normal
Severity: normal Version: trunk
Component: Component - Replies Keywords: needs-patch needs-testing
Cc:

Description

While debugging a problem pertaining to replies not showing up on a topic in a specific case, I found what appears to be a plain coding error. (BTW: Fixing this doesn't solve my original problem, but it does solve it partially). In my scenario, I have bbpress and BuddyPress working together, and I'm loading a single topic—but it doesn't seem that BuddyPress has anything to do with this issue.

The problem is an apparently stray (void) return statement in the pagination logic for replies.

This is the offending code snippet, from includes/replies/template.php. Notice the third line with the void return, followed by more logic. This breaks the logic and the expected return value of the method.

		// We are threading replies
		if ( bbp_thread_replies() && bbp_is_single_topic() ) {
			return;
			$walker  = new BBP_Walker_Reply;
			$threads = (int) $walker->get_number_of_root_elements( $bbp->reply_query->posts );
			// Adjust for topic
			$threads--;
			$retstr  = sprintf( _n( 'Viewing %1$s reply thread', 'Viewing %1$s reply threads', $threads, 'bbpress' ), bbp_number_format( $threads ) );
		// We are not including the lead topic
		} elseif ( bbp_show_lead_topic() ) {

The impact of this stray return is that the resulting div with class bbp-pagination is empty, like this:

<div class="bbp-pagination">
	<div class="bbp-pagination-count">

		
	</div>

	<div class="bbp-pagination-links">

		
	</div>
</div>

If I comment out that apparently stray return statement, this div is populated with the expected content, like this:

<div class="bbp-pagination">
	<div class="bbp-pagination-count">

		Viewing 1 reply thread
	</div>

	<div class="bbp-pagination-links">

		
	</div>
</div>

I discovered this in version 2.5.10 of bbpress. I see that it's still there in the latest version of this code at https://github.com/wp-plugins/bbpress/blob/master/includes/replies/template.php

Change History (2)

#1 @thebrandonallen
3 years ago

  • Keywords 2nd-opinion added

@mlwilkerson

Thanks for the report!

This happened in [4973]. It's hard to tell from the commit if this was actually a mistake, or if was a reason for it at the time. I'm with you, it looks like a mistake, but let's see if John can fill us in.

cc @johnjamesjacoby

#2 @johnjamesjacoby
2 years ago

  • Keywords needs-patch needs-testing added; 2nd-opinion removed
  • Milestone changed from Awaiting Review to 2.7

Yeah; that return is there by design, because our pagination functions don't take the hierarchy into account yet.

WordPress *does* provide us with much of this logic. When we are ready to fully support this, we can likely take a lot of inspiration from upstream.

Note: See TracTickets for help on using tickets.