/page/2/ being adding to reply links incorrectly

[sterndata]

Replies on the first page of a topic and getting "/page/2/" added with the links, breaking them. For example, see ​https://wordpress.org/support/topic/smileys-not-working-on-mobile/?view=all

This reply is still on the first page:

https://wordpress.org/support/topic/smileys-not-working-on-mobile/page/2/?view=all#post-15900871

but doesn't work unless /page/2/ is removed.

It may be significant that this is the 10th reply in the chain. Subsequent replies also have /page/2/

[dd32]

I suspect this is a bug from bbPress or WordPress trunk, threads show 15 replies per page, but I guess the pagination code is assuming 10-per-page, I'm not sure where it's inheriting that number from though.

[sterndata]

Does this matter, under SETTINGS->READING?

[dd32]

Does this matter, under SETTINGS->READING?

I did check that setting, but as it's set to 12 I disregarded it as being potentially related.. since this is kicking in at post 10 and there didn't appear to be any hidden replies prior to that.

[sterndata]

Just a note that this is affecting real users. They get an email from the forums with a link to a post and when they click it, they don't see it because the pagination is wrong. Can we bump up the priority a bit on this?

[dd32]

Took a quick look into this.. Per-forum-page is set to 15, and the page a reply is on is generated through basically bbp_get_reply_position() / bbp_get_replies_per_page().

The problem is that bbp_get_reply_position() is returning the incorrect result for the thread, and other threads.

On ​https://wordpress.org/support/topic/smileys-not-working-on-mobile/?view=all that should be returning 1 through 15, but instead, the position is being returned as... (with > 15 being page=2)

1, 3, 4, 6, 7, 9, 10, 12, 13, 15, 17, 18, 18, 22, 23 (archived)

It's not immediately obvious to me as to why there are missing reply position numbers in there..

[dd32]

As somewhat of a bandaid stop-gap fix, we could redirect /page/xx/ to the first-page to avoid the situation where accessing a too-high-page pagination url returns the thread starter without any replies.

edit: replaced reference to last-page with first page, as last-page is probably inaccurate for affected threads.

[sterndata]

There is one archived post in that topic, but it's near the bottom of the first page of replies.

[SergeyBiryukov]

Related: #2110

[bcworkz]

It's not immediately obvious to me as to why there are missing reply position numbers in there..

I'm pretty sure the missing replies were spam replies that have been deleted. Apparently the reply count does not get updated when spam is removed, causing the pagination to behave as though the spam is still there.

[dd32]

Replying to bcworkz:

It's not immediately obvious to me as to why there are missing reply position numbers in there..

I'm pretty sure the missing replies were spam replies that have been deleted. Apparently the reply count does not get updated when spam is removed, causing the pagination to behave as though the spam is still there.

That would certainly account for it!

Looking at bbPress, I only see bbp_update_reply_position() being called for merged/split topics, and from within the reply-position repair tool. (edit: the menu_order repair tool, I'm not sure if that's the same thing)

Perhaps what we should be looking at here is having bbp_update_reply_position() run for topics after any reply is archived/spammed/deleted.

This feels like it's an upstream bbPress bug, but one that we can probably patch over on w.org while we figure out a way to reproduce it in a test case for upstream fixing.

[sterndata]

I'd like to bump this on the priority list. I get at least 3 emails a day with bad pagination links. It makes it harder to moderate stuff. BUT, more importantly, it affects "real" users, too, and as it's a publicly facing bug, we should get it fixed ASAP.

[tellyworth]

From a brief dive into the code, it looks like the Recalculate position of each reply in each topic repair job in Tools / Forums / Repair Forums will run bbp_update_reply_position(). Though I'm not sure it'll run it on the topics where pagination is broken.

I haven't run it because it's listed as High Overhead. @dd32 would you say it's safe to run once? (Once a week?)

[dd32]

I haven't run it because it's listed as High Overhead. dd32 would you say it's safe to run once? (Once a week?)

I would not say it's safe to run at all on WordPress.org/support.

From my comment above:

Perhaps what we should be looking at here is having bbp_update_reply_position() run for topics after any reply is archived/spammed/deleted.

I think that's safe though, I'm not sure if it'll fix the issue or not though.

[tellyworth]

Call bbp_update_reply_position() after removing replies from a thread

[tellyworth]

I think attachment:6443.patch​ gets us part of the way there. There might be some caching issues to deal with, and I haven't tested it thoroughly. There are probably other places it should hook into as well (unspammed, unarchived?).

[dd32]

Unfortunately bbp_update_reply_position() doesn't completely do the trick here it seems.

It turns out this is a deeper problem than I thought.. In addition to the problem where a reply simply no longer exists (ie. fully deleted), when replies are spam or archived, they still occupy a reply ID, and so the problem is that the URL pagination generation is unaware of some replies not being visible to non-moderators.

​https://wordpress.org/support/topic/smileys-not-working-on-mobile/ is now an example of that. The 15th reply is archived, and so when it's not being viewed by a moderator with ?view=all the 15th reply (last reply on the first page) has a /page/2/ url.

Either

1. ​bbp_get_reply_url() needs to handle the reply id being incorrect OR
2. ​bbp_get_reply_position_raw() needs to change to not give reply IDs to hidden replies.

The problem with these is that with 1 it means iterating over all replies prior to the reply in the thread to determine how many are hidden (..or missing?), and lower the reply_id by that many, and with 2 is that when a moderator is viewing it, the reply_ids need to be in the correct order to render the replies in the correct order. 1 would also mean that the links would be incorrect for moderators (but that's okay I guess).

As much as it seems like a performance hit, 1 is the obvious choice by filtering bbp_get_reply_url, I guess we can get away without much of an impact by only checking when the URL is >= /page/2 as replies on the first page aren't affected. The replies should be being displayed anyway so should also be cached in memory already.

[dd32]

I've filed this bug upstream with bbPress: ​https://bbpress.trac.wordpress.org/ticket/3503

[sterndata]

#6725 was marked as a duplicate.