Skip to:
Content

bbPress.org

Opened 21 months ago

Last modified 20 months ago

#3195 new defect

bbp_get_topic_replies_link doesn't respect pagination for extra links

Reported by: Clorith Owned by: johnjamesjacoby
Milestone: 2.7 Priority: high
Severity: normal Version:
Component: General Keywords: needs-patch
Cc:

Description

When creating the extra links (for viewing hidden replies) the call to bbp_get_topic_replies_link() uses the primary permalink of the topic, and appends a ?view=all to it.

This then drops any pagination from the string, so if you were on page 3, you would get sent back to page 1.

One reason for this might be that page 1 has hidden entries as well though, so this is likely a discussion point.

Change History (3)

#1 @johnjamesjacoby
21 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 2.6
  • Owner set to johnjamesjacoby
  • Priority changed from normal to high

This is not intentional. We probably aren’t juggling arguments to correct for additional trusted ones like pagination.

#2 @johnjamesjacoby
20 months ago

In 6787:

Links: Add $cap to bbp_get_view_all filter. See #3195.

#3 @johnjamesjacoby
20 months ago

  • Milestone changed from 2.6 to 2.7

One reason for this might be that page 1 has hidden entries as well though, so this is likely a discussion point.

It's actually less about page 1 having hidden entries, and all about not pre-calculating the minimum and maximum number of pages for each available view. We don't really know how to calculate the boundary change intelligently.

For example, let's say that when viewing all, there are 25 pages, but when not viewing all, there are only 20. If you're on page 25 and viewing all, page 25 would 404 when not viewing all. How do we know there are only 20 pages for the not-viewing-all status?

I _think_ I know how we'd address this, but it might be more costly than it's ultimately worth. We'd just need to calculate pagination boundaries for all available views (of which there are only 2 now) and make sure to never link outside of them. In addition, we'd probably want to identify the reply position of the first available reply in the current view, and use that to maintain the exact page when going back and forth.

Because of all this, I'm going to move this out of 2.6 and into 2.7 to think more about whether we even want to do all of that querying & calculation, if there's an easier way, or if there's something more cool we can do instead.

If anyone has more thoughts, I'd love to hear them.

Note: See TracTickets for help on using tickets.