Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Network wide search may switch away from the site the search is performed on #2121

Closed
gonzomir opened this issue Mar 8, 2021 · 6 comments
Closed

Network wide search may switch away from the site the search is performed on #2121

gonzomir opened this issue Mar 8, 2021 · 6 comments
Assignees
@tott @felipeelia
Labels
confirmed bug high priority
Milestone
3.6.2

Comments

@gonzomir
Copy link
Contributor

gonzomir commented Mar 8, 2021

Describe the bug
When retrieving the posts in a network-wide search, a switch_to_blog() is called as part of the_post hook, but restore_current_blog() is not called immediately after that, it's called in the next the_post hook if the next post is from a different site. This works OK in the loop, but some plugins (Yoast for example) may trigger the hook outside the loop. In that case WordPress will be left switched to the blog the first result came from, and if it's not the blog the search was performed on, unexpected thing happen.

I didn't go that deep to investigate what exactly Yoast is doing, but the hook is triggered when it generates it's output in the <head />.

Steps to Reproduce
I'm sorry, but I can't share a public URL where the bug can be observed.

  1. Create multisite WordPress installation with at least two blogs.
  2. Install and activate Yoast and ElasticPress.
  3. Do a search on the main blog that will return a post from the second site as a first result.
  4. The site name in the header will be of the second blog, not the main one.

Expected behavior
The expected behaviour would be the site name not to change because of the returned results.

Environment information

  • WordPress version: 5.6
  • Plugins and version: Yoast 15.6.2
  • Theme and version: custom theme, N/A

Additional context

My workaround was the following:

add_action( 'the_post', __NAMESPACE__ . '\\maybe_restore_current_blog', 11, 1 );
function maybe_restore_current_blog( $post ) {
	if ( in_the_loop() ) {
		return;
	}
	if ( is_multisite() && ! empty( $post->site_id ) ) {
		restore_current_blog();
	}
}

Maybe at the end of QueryIntegration::maybe_switch_to_blog() a similar check should be made:

if ( ! in_the_loop() ) {
	restore_current_blog();
}
@gonzomir gonzomir added the type:bug Something isn't working. label Mar 8, 2021
@felipeelia
Copy link
Member

Hi @gonzomir,

By default, ElasticPress won't search on other sites unless the sites parameter is added to the WP_Query arguments. Do you mind sharing how you are doing that?

FWIW, I've tried adding it with the following snippet and things seem to work as expected:

add_action(
	'pre_get_posts',
	function ( $query ) {
		if ( ! is_admin() && $query->is_main_query() && $query->is_search() ) {
			$query->set( 'sites', 'all' );
		}
	}
);

@gonzomir
Copy link
Contributor Author

Hi, @felipeelia,

In my case the sites param is an array of exact blog IDs because there is a case where a site is indexed but not searched from the global search. But that's not really important, your code should render the same results. I'll try to set up a minimal example to confirm my findings and give you precise recipe to debug on your side.

@mckdemps mckdemps added reporter feedback bug Something isn't working and removed type:bug Something isn't working. labels Mar 12, 2021
@gonzomir
Copy link
Contributor Author

I prepared a minimal install that confirms the issue. WordPress multisite with just ElasticPress and Yoast, and Twenty Twenty-One theme and one subsite added. There are two essential additions:

  • Adding define( 'EP_IS_NETWORK', true ); in wp-config.php
  • Adding the above pre_get_posts action to a mu-plugin.

To get a search result from the subsite appear on top of the results I just added a new post with some more "hello"s in the content and did a search for "hello". Here are screenshots that explain what happens:

This is the main site homepage (note the URL and the site name):
Screenshot from 2021-03-13 12-14-27

Here's the subsite:
Screenshot from 2021-03-13 12-14-53

And here's a search on the main site with the first result comming from the subsite. Note the URL of the main site and site name from the subsite:
Screenshot from 2021-03-13 12-15-13

@felipeelia
Copy link
Member

Thanks @gonzomir, I can reproduce it now.

It seems the root cause of it is a call to wp_reset_query();, by the way. The solution you mentioned works, I just wonder if it would have a side effect on people relying on Post\QueryIntegration::get_switched() method or something like that.

Do you want to craft a Pull Request with that and we can run some tests with it? Thanks in advance!

@felipeelia felipeelia added confirmed bug and removed bug Something isn't working labels Mar 19, 2021
@gonzomir
Copy link
Contributor Author

Yes, I will, no problem.

@gonzomir gonzomir mentioned this issue Mar 20, 2021
Closed
6 tasks
@felipeelia felipeelia mentioned this issue Mar 26, 2021
Closed
@mckdemps mckdemps added this to the 3.6.2 milestone Jul 27, 2021
@felipeelia felipeelia mentioned this issue Aug 1, 2021
Merged
6 tasks
@felipeelia
Copy link
Member

Closing this one as #2283 was merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tott tott

@felipeelia felipeelia

Labels
confirmed bug high priority
Projects
None yet
Milestone
3.6.2
Development

No branches or pull requests
4 participants
@tott @felipeelia @gonzomir @mckdemps