Opened 8 years ago
Last modified 8 years ago
#2985 new defect (bug)
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)
#2
@
8 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.
@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