bbp_has_replies/topics broken on lastest WP Trunk

[thebrandonallen]

​[WP28623] breaks bbp_has_replies/topics. Since we set 's' in the $defaults array, the new change in trunk always thinks we're doing a search. So, when we're not actually doing a search, no results are returned.

Unsure about who's in the wrong here. It could be considered a backward incompatible change, but maybe it's just exposing the we're _doing_it_wrong. Figured I'd start here, and we can push upstream if necessary.

[netweb]

Thanks, came across this the other day with bbpress.org/buddypress.org and hadn't started looking at what upstream changeset it was that broke things.

Related: ​#WP11330 ​[WP28623] ​[WP28612]

[thebrandonallen]

Attached patch fixes the issue. Turns out, the code that was causing the problem has never actually been used. It was introduced with the initial version of the bbPress plugin, but it has never been used. I'm providing two versions of a patch.

The first patch keeps the original code more or less intact, but works around the change to WP. The only reason this would be necessary is if there were a plugins making use of the ts and rs parameters. Since bbPress never even made use of these, I doubt that there are any plugins using them. The second patch removes the unused code, which, simultaneously, fixes the change to WP.

[thebrandonallen]

Work around with back-compat

[thebrandonallen]

Remove unused code

[thebrandonallen]

The original issue has been fixed as of ​[WP28804], however, the "fix" in 2607.02.patch is still valid. As it stands, the 'ts' (for topics) and 'rs' (for replies) array keys will never be anything but empty. This means we're setting a variable for every call to bbp_has_topics and bbp_has_replies, but has not purpose. Leaving open for @netweb or @johnjamesjacoby to decide the fate of this ticket ;)

[johnjamesjacoby]

In 5467:

Only add the s argument to queries if rs or ts are set in their respective topics & replies queries.

See #2607, ​#WP11330, ​[WP28623] ​[WP28612], ​[WP28804].

[johnjamesjacoby]

In 5468:

Only add the s argument to global forum search queries if search-terms exist.

See #2607, ​#WP11330, ​[WP28623] ​[WP28612], ​[WP28804].

[johnjamesjacoby]

These should be fixed now, along with a third query for the global search.

Going to mark as closed. Please reopen as needed.

[tollmanz]

One little issue slipped into the r5467. L147 uses $default_topic_search which is no longer defined. This can lead to PHP notices in certain conditions.

[thebrandonallen]

This was fixed in [5477].

[coreymckrill4ttf]

The changesets related to this bug, [5467], [5468], and [5477] do not seem to be in the latest distributed version of bbPress (2.5.8 at the moment). Is this an oversight, or were they pulled out for some reason?

[thebrandonallen]

The changesets you mentioned were all for trunk, and were never applied to the 2.5 branch. Are you having an issue related to this?

[coreymckrill4ttf]

Ah, gotcha. Yeah, this bug breaks our whole forum. I am currently manually applying the changes every time I update to the latest 2.5.x.

[thebrandonallen]

You shouldn't have any issues. The initial report was based off of WP trunk, at the time, and it was fixed before an official WP release came out. I haven't seen anyone else report this. Could you give more details, so we can see if this is something we need to fix in the 2.5 branch?

[coreymckrill4ttf]

Thanks, @thebrandonallen

Basically, always including the 's' parameter in the query args for topics and replies causes is_search() to always return true, even when it's just showing a topic/reply thread, and therefore should be false.

Our theme uses is_search to conditionally do some other funky stuff to the query args, which then breaks the forum. So, as it turns out, it's really a problem with our theme, though it seems like getting a false positive on is_search() is still a significant bug...

[Cybr]

I'm afraid this bug is back. Tested in version 2.5.14.

Use case: ​https://wordpress.org/support/topic/conflict-with-bbpress-13/#post-9721655

[johnjamesjacoby]

This is fixed in trunk (for 2.6) and wasn't ported to the 2.5 branch.

[johnjamesjacoby]

Just moving this back into 2.6 to mark as fixed.