Skip to:

Opened 17 months ago

Last modified 17 months ago

#3503 new defect (bug)

bbp_get_reply_url() generates incorrect paginated urls for topics with hidden replies

Reported by: dd32's profile dd32 Owned by:
Milestone: 2.6.10 Priority: normal
Severity: normal Version: 2.0
Component: General Keywords: needs-patch reporter-feedback dev-reviewed


bbp_get_reply_url() is used within emails to link to a paged topic URL for a reply within a topic.

However, this function can generate the incorrect pagination URL for replies when a topic has hidden replies - because they're spammed, or in the case of 'archived'.

Take this example, where there's 2 replies per page displayed:

1. Thread post
2. Reply 1
3. Reply 2 <-- Spammed.
4. Reply 3

When a non-moderator views the above thread, they'll see a single-page of replies. Reply 1 and Reply 3. When a moderator views it, they'll see two-pages of replies with all three replies.

However, in both cases, bbp_get_reply_url( $reply_3 ) will return topic/page/2 and for the non-moderator following that link, will land on a page with no replies, as the "correct" link was topic/.

This leads to a lot of confusion.
I see several options:

  • Have bbp_get_reply_url() actually count the replies visible, rather than using the cached reply_position value.
  • Have bbp_get_reply_url() never return a pretty-url, and instead use a url such as topic/?reply=123 and redirect to the appropriate paged url upon request
  • Have bbp_get_reply_position_raw() not give a reply_position to hidden replies.

All of those have one thing in common: The URL to individual replies will constantly change, as the pagination location of the reply is never truely known.

Another option is that when displaying threads, hidden replies should be taken into account, so in the above example, with 2 replies per page, everyone would only see 1 reply on Page 1, and 1 reply on Page 2. Arguably a worse-experience.

This was reported as #meta6443

Change History (2)

#1 @johnjamesjacoby
17 months ago

  • Keywords needs-patch reporter-feedback dev-reviewed added
  • Milestone changed from Awaiting Review to 2.6.10
  • Version set to 2.0

Thanks for reporting this here @dd32 🙏

In my mind, I envision a combination of your first and third suggestions. It results in bbPress more closely working the way Comments in WordPress do (see: get_page_of_comment()).

  • Have bbp_get_reply_position_raw() not give a reply-position to non-public replies
    • Perhaps this means still explicitly setting it to 0, or something else?
    • I'm unsure of any ramifications this will have on bbPress plugins – I.E. "Private Replies" is used by plugin authors who use bbPress to support their projects
    • Perhaps an opportunity to intentionally support & implement private replies, topic notes, etc...
  • Cache reply position (menu_order) using "public" replies instead of "all"
    • Swap bbp_get_all_child_ids() to bbp_get_public_child_ids() in bbp_get_reply_position_raw()
  • Have bbp_get_reply_url() skip menu_order and count visible replies that uniquely match the scope of the visitor
    • Add some'include_unapproved' equivalent query argument that would include logged-in user IDs and/or logged-out user emails (see: build_comment_query_vars_from_block() and WP_Comment_Query::get_comment_ids())
Version 0, edited 17 months ago by johnjamesjacoby (next)

#2 @dd32
17 months ago

I think, it would be interesting, to always paginate based on public replies, but then also include non-public ones without influencing the pagination links. That might mean a moderator sees 25 spam replies when replies-per-page is set to 10, but at least the next-public page would always be right, and the boundaries would never slide around?

This seems like the best solution to me, as otherwise moderators views would be wildly different to the public view.

However, it doesn't fully resolve the issue. Because if a moderator archives/spams 5 replies on page 1, suddenly the replies that were on page 2 get pulled back to page 1 and previous links are now invalid, likewise, when 5 replies are unspammed the following replies shift to page 2. Not as bad an experience though.
I don't really see a better option for how to handle that though.

(That points out the other issue, if you access /page/99 you'll get an empty page, which should probably redirect back to /page/2)

Displaying 25 replies on a 10-paged page-1 for the moderators would be hard pagination-code wise, you'd have to switch the query to post_parent = ?? AND post_status IN(??) AND menu_order BETWEEN $page_offset AND $next_page_offset ORDER BY date DESC instead of using LIMIT.. or double-query for any hidden replies that should be visible but aren't - That might be the best option in retrospect, single-query for regular viewers, double-query for moderators to find any hidden replies that should've been included..

(Just thinking out loud)

Note: See TracTickets for help on using tickets.