Skip to:
Content

bbPress.org

Opened 22 months ago

Last modified 2 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.7 Priority: normal
Severity: normal Version: 2.0
Component: General Keywords: needs-patch reporter-feedback dev-reviewed
Cc:

Description

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 WordPress.org/support '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 (4)

#1 @johnjamesjacoby
22 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())

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?

Last edited 22 months ago by johnjamesjacoby (previous) (diff)

#2 @dd32
22 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)

#3 @johnjamesjacoby
2 months ago

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.

(Also thinking out loud...)

I did some research, and found that some other forum softwares (including Reddit & GitHub fwiw) do not even bother to try and hide non-public replies to topics/threads – either it's there, or it's fully-fully deleted.

I don't really prefer that personally, but it circumvents the issue almost completely.

What they do is anonymize the non-public content with some kind of [deleted] or [moderated] labeling for non-capable participants or spectators, but if you're a moderator or keymaster equivalent you can see what the original content was.

The forums over at the Ubiquiti Community do this also when they split a topic or move a reply out into its own. They leave a ghost-post in the topic where the original reply was.


Back to this issue, and more specifically emails – what if we send different emails based on user role?

It seems completely logical to me that a moderator/keymaster who is subscribed to something would deserve a different kind of alert than a participant/spectator; the former being action, and the latter being consumption?

This way, instead of making changes to pagination, we leave it for later and improve the email experience instead?

#4 @johnjamesjacoby
2 months ago

  • Milestone changed from 2.6.10 to 2.7

Moving to 2.7 to clear the 2.6.10 minor milestone.

Note: See TracTickets for help on using tickets.