Skip to:
Content

Opened 4 years ago

Closed 2 years ago

#2702 closed defect (fixed)

Topics and Replies Pagination issue when pages are > 9 using default permalinks

Reported by: igmoweb Owned by: johnjamesjacoby
Milestone: 2.6 Priority: normal
Severity: normal Version: 2.5.4
Component: Component - Replies Keywords: has-patch needs-testing
Cc: jjjay@…

Description

Hi.

When permalinks are deactivated and pagination is generated in bbp_has_replies() function. In line number 273, file includes/replies/template.php we can see the following code:

$bbp->reply_query->pagination_links = str_replace( '&paged=1', '', $bbp->reply_query->pagination_links );

When pages are > 9, they are replaced also by nothing.

Here's the result for one of my links:
http://www.nlhvnlv.dev/?topic=sus-preguntas&paged=9 (page 9, that's fine)
http://www.nlhvnlv.dev/?topic=sus-preguntas0 (page 10)
http://www.nlhvnlv.dev/?topic=sus-preguntas0 (page 11)

Sorry, my forum is private so you won't have access.

I think this should be high priority. It should be easy to fix though as we could leave the paged arg in the first page link and yet is ugly it still work.

Or... we could use a regexp instead.

Attachments (6)

2702.diff (855 bytes) - added by tharsheblows 4 years ago.
don't replace paged=1 in eg paged=14
2702.2.diff (1.5 KB) - added by netweb 4 years ago.
2702.3.diff (1.5 KB) - added by netweb 4 years ago.
2702.tests.diff (3.5 KB) - added by tharsheblows 4 years ago.
unit tests for replies and topics pagination
2702.4.diff (5.4 KB) - added by tharsheblows 4 years ago.
replies and topic pagination patches and unit tests
2702.5.diff (2.0 KB) - added by thebrandonallen 3 years ago.

Download all attachments as: .zip

Change History (20)

#1 @netweb
4 years ago

  • Component changed from General to Replies
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 2.6
  • Priority changed from high to normal
  • Severity changed from major to normal

Thanks, makes sense in that we should not be doing that, will get this fixed for the next release.

Related: #2468 #2556

#2 @igmoweb
4 years ago

Thanks netweb!

I don't have much time right now to provide a patch but I'll see if I can :)

#3 @tharsheblows
4 years ago

  • Cc jjjay@… added

Um, I'm just deleting the post where I said "I'm obviously missing something" because I was... :) I will try to submit a patch for this one, too.


Last edited 4 years ago by tharsheblows (previous) (diff)

@tharsheblows
4 years ago

don't replace paged=1 in eg paged=14

#4 @tharsheblows
4 years ago

  • Keywords has-patch added; needs-patch removed

@netweb
4 years ago

#5 @netweb
4 years ago

  • Keywords needs-testing added
  • Summary changed from Replies Pagination issue when pages are > 9 to Topics and Replies Pagination issue when pages are > 9 using default permalinks

In 2702.2.diff, added code for topics based of 2702.diff.

I'm just refreshing my test site with some demo content and will test shortly.

#6 @tharsheblows
4 years ago

I clearly have some sort of block about remembering to check topics for the issues that affect replies, sorry. This one got a little twisted around in my head at the time, so if it doesn't work I can take another look if you'd rather not.

#7 @johnjamesjacoby
4 years ago

Patch isn't working for me, and the regular expression doesn't look right to me.

Any other eyes on this?

@netweb
4 years ago

This ticket was mentioned in Slack in #bbpress by netweb. View the logs.


4 years ago

#10 @johnjamesjacoby
4 years ago

Thanks; will test again in my AM.

The other nit is that we can't use anonymous functions until PHP5.3, so we'll need an actual callback function instead.

@tharsheblows
4 years ago

unit tests for replies and topics pagination

#11 @tharsheblows
4 years ago

Unit tests for topics and replies pagination. I will change the anonymous functions too, just keep forgetting.

#12 @tharsheblows
4 years ago

This has the patches with the named function and unit tests.

@tharsheblows
4 years ago

replies and topic pagination patches and unit tests

#13 @thebrandonallen
3 years ago

Updated patch to use preg_replace instead of preg_replace_callback. Also, added search pagination links.

#14 @johnjamesjacoby
2 years ago

  • Owner set to johnjamesjacoby
  • Resolution set to fixed
  • Status changed from new to closed

In 6228:

Pagination: Use preg_replace to find & remove page=1 query arguments.

Fixes #2702. Props thebrandonallen, tharsheblows.

Note: See TracTickets for help on using tickets.