WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 3 years ago

Last modified 3 years ago

#21837 closed defect (bug) (fixed)

Site search for a domain by text fails in subdomain installs of multisite with www in primary domain

Reported by: frisco Owned by: jeremyfelt
Milestone: 4.6 Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords: has-patch
Focuses: multisite Cc:

Description

To reproduce:

1) Create a subdomain network install where the main site's domain contains www
2) On the Network Admin->Sites search for an existing site based on the domain name, such as some* to find somedomain.domain.com.
3) Search returns "No sites found." even when the site exists and was searched for using a valid technique.

I've tested the problem on Apache and Nginx servers, with Twenty Eleven as the theme on the main site, and network activated plugins. Existing sites can never be found by domain name.

Subdomain installs in a network are allowed where the main site is www.domain.com. DOMAIN_CURRENT_SITE in wp-config.php points to www.domain.com. But blogs.domain stores the domain of a multi-site site named somedomain as somedomain.domain.com (without the www).

As a result, any site search for a domain based on text (the name of the domain) fails because of how the query is built on lines 70-73 of wp-admin\inludes\class-wp-ms-sites-list-table.php. A search for some* produces a query that searches for some%.www.domain.com instead of some%.domain.com; the result is that nothing is ever found.

Searches based strictly on site ID work fine.

Possible fixes: 1) leave as is but warn that subdomain network installs where the primary domain includes www will have a broken site search (not ideal) and encourage no www on primary site of a subdomain network install or 2) check if the primary domain contains www and strip it out prior to building the query (ideal).

Attachments (4)

21837.diff (1.6 KB) - added by georgestephanis 4 years ago.
21837.with-unit-tests.diff (3.5 KB) - added by georgestephanis 4 years ago.
21837.with-unit-tests.fixed.diff (3.6 KB) - added by georgestephanis 4 years ago.
21837.2.diff (2.1 KB) - added by johnjamesjacoby 4 years ago.
Kinda terrible, but closer to what I expect to happen

Download all attachments as: .zip

Change History (21)

#2 @jeremyfelt
6 years ago

  • Component changed from Multisite to Networks and Sites
  • Focuses multisite added

#3 @jeremyfelt
5 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from 3.4.2 to 3.0

Thanks for the ticket, @frisco. Sorry to leave you hanging so long.

A message was added via #11945 as a warning. The following displays when configuring multisite on a www domain:

We recommend you change your siteurl to wordpress.dev before enabling the network
feature. It will still be possible to visit your site using the www prefix with an
address like www.wordpress.dev but any links will not have the www prefix.

That said. :)

Site search is pretty wobbly right now. I'm not finding another ticket at the moment that covers site search in general, and it's something that could use a bit of work. I don't think it would be too tough to expand the current logic to do a better job. It would probably be best to ignore parsing the request against $current_site->domain and focus instead on whatever is being searched for.

#4 @frisco
5 years ago

@jeremyfelt - Thanks for the follow up. Couple of things:

1) If I understand correctly, the message added doesn't seem accurate with the phrase "any links will not...." When I posted my original issue, without the www, links on the main site did have the www; if that was changed, it would seem odd. It seems the intended explanation is that subdomains and any links on them (not links on the main site) will not have the www prefix. Domain mapping throws another wrinkle into that message, because www can appear in a mapped subdomain and its links. I think the message can be improved to say specifically that with www on the main site, site search on multisite won't work.

2) It's been a while since I looked at the relevant code, but I recall thinking that it would be straightforward to fix the search logic so it took into account www or no www. I'm happy to take a stab at that, but if multisite is getting bigger overhauls that might change this code, maybe that's not a productive approach. If the overhauls are otherwise going to leave this alone, then it ought to be fixed.

This isn't a burning issue for me personally, because once I realized the problem, I switched my networks to drop www on the main site, but I'm happy to try to provide a fix to help others.

#5 @jeremyfelt
5 years ago

1 - I'm going to have to poke around at the www linking a bit. It's entirely possible that things are backwards a bit there.

2 - Absolutely! I would be happy to test any improvements made there. From looking at the code last night, I think it's only a few changes from being drastically improved.

#6 @frisco
5 years ago

@jeremyfelt - Ok, I'll try to put something together and post back. Might take me at least a week + with everything else going on, but I'll get to it.

This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.


4 years ago

#9 @georgestephanis
4 years ago

In get_blog_details() if you’re searching by domain/path — https://core.trac.wordpress.org/browser/tags/4.2.2/src/wp-includes/ms-blogs.php#L115 — if searching for a domain with a www. prefix, it looks like it will find the result either with or without. But if searching for the domain without the www. prefix, it won’t catch an entry that has www. prefixed on the domain.

It doesn’t look like the www. gets stripped in update_blog_details() anywhere either.

Not sure if this is precisely the same issue, but I'll try to write a patch shortly, but in case I forget, documenting stuff here for future generations.

#10 @johnjamesjacoby
4 years ago

Just ran into this, but with a different configuration other than www..

  • Primary site of network: demo.example.com
  • Secondary sites of network: foo.example.com, bar.example.com, etc...

When attempting to search for sites, it looks for search%.demo.example.com.

I believe @jeremyfelt's first assessment to avoid concatenating $current_site->domain is the bare minimum. I'll go 1 step further and say that my expectation as a network admin is to see results from both the domain and path regardless of installation type, and let me narrow the results with more precise search strings if necessary.

@johnjamesjacoby
4 years ago

Kinda terrible, but closer to what I expect to happen

This ticket was mentioned in Slack in #core-multisite by ocean90. View the logs.


4 years ago

#12 @jeremyfelt
3 years ago

  • Milestone changed from Future Release to 4.6

Thanks for the latest patch, @johnjamesjacoby. :)

I have a feeling #35791 will help solve a few tickets like this, so I'm leaving a note for later.

Related: #24833, #33185.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

#14 @jeremyfelt
3 years ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to jeremyfelt
  • Status changed from new to assigned

The latest patch on #36675, which implements WP_Site_Query (see #35791) resolves this ticket in the process. With that patch applied, the entire domain column is searched rather than a portion of it.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


3 years ago

#16 @jeremyfelt
3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 37736:

Multisite: Use WP_Site_Query to power WP_MS_Sites_List_Table.

WP_Site_Query provides for a cleaner prepare_items() method. It significantly improves the search experience in the sites list table:

  • In a subdomain configuration, domain and path are searched for a provided terms.
  • In a subdirectory configuration, path is searched for a provided term.
  • The full domain is searched in a subdomain configuration rather than the portion not matching the network's domain.
  • Terms are searched as %term% by default. Adding * in the middle of a term will search %te%rm%.

Props flixos90, Fab1en.
Fixes #33185, #24833, #21837, #36675.

#17 @DrewAPicture
3 years ago

In 37739:

Docs: Add a missing summary and @since version to the DocBlock for WP_MS_Sites_List_Table::prepare_items().

Also adds a changelog entry for 4.6.0 noting the changeover to get_sites().

Fixes #36675. See #21837, #24833 and #33185.

Note: See TracTickets for help on using tickets.