#11330 closed defect (bug) (fixed)
Empty search takes you to homepage instead of empty search page
Reported by: | jacobfogg | Owned by: | wonderboymusic |
---|---|---|---|
Milestone: | 4.0 | Priority: | low |
Severity: | minor | Version: | 2.8.5 |
Component: | Query | Keywords: | |
Focuses: | Cc: |
Description
- In the admin tool, if you go to "Settings->Reading" then choose "A static page (select below)" and choose a page for your "Front page" and a different page for your "Posts page".
- Visit your blog (http://localhost/ in my case) and you will see your static page.
- Navigate to your blog (http://localhost/blog/ in my case).
- Enter a search string (e.g. "test") and click "Search". This will take you to the search results page (http://localhost/?s=test in my case). Notice that it is located at the base of your site.
- Return to your blog, leave the search textbox blank and click "Search". This _should_ take you to the search results page but takes you to the _Homepage_ instead (http://localhost/?s= in may case).
IMHO, a blank search string _should_ take you to the search page and state "No posts found. Try a different search?" as it does when you enter an invalid search. At the very least, it should take you to the blog page.
The reason it takes you to the homepage is due to how the get_search_form() function in the /wp-includes/general-template.php file creates the Action url for the search form:
$form = '<form role="search" method="get" id="searchform" action="' . get_option('home') . '" >'
I propose a change in the code that will do the following:
if(get_option('show_on_front') == 'page'){//if the blog is not the front page $searchAction = get_page_link(get_option('page_for_posts'));//get the link to the blog }else{//if it is the front page $searchAction = get_option('home')."/";//get the link to the home } $form = '<form role="search" method="get" id="searchform" action="' . $searchAction . '" >
That way, at the very least, you know the search is landing where it belongs.
This, however, does not _fix_ the issue where a blank search does not land you on the search page. My guess is, wherever it is determined what page is displayed, there is a check for something like:
if($_REQUEST['s']!=''){ //if the request var 's' is not blank, this is a search }
If that is indeed the case, a simple change of the code to something like this should do the trick:
if(isset($_REQUEST['s'])){ //if the request var 's' isset, this is a search }
Attachments (8)
Change History (67)
#1
@
15 years ago
- Component changed from General to Template
- Keywords close added
- Milestone changed from Unassigned to Future Release
- Priority changed from normal to low
- Severity changed from normal to trivial
- Type changed from defect (bug) to enhancement
#2
@
15 years ago
- Keywords needs-patch added; close removed
- Severity changed from trivial to minor
Well as enhancements go, it makes a lot of sense. Hitting the Search button should take you to a Search result page, even if there is no result.
#4
@
15 years ago
This may seem trivial if WP is using as _just_ a blog, however you are using the site as a web page with a blog, a search performed on a blog page should NOT send you back to the home page.
#6
@
15 years ago
- Type changed from enhancement to defect (bug)
please when this trivial needs patch will be solved?
Imho it is bug, search should definitely put you on search page even when you search nothing. So we would be easily able to create separate search page (in many countries it is a must-have accesibility rule when creating webs e.g. for public administration)
P.S.: if i would consider this as an enhancement, it would be to have an option to give it some sexy url, but this could be solved by something like
add_action('template_redirect', 'nice_search'); function nice_search() { if (is_search()) { $search_uri = get_bloginfo('url') .'/search/'. urlencode(get_query_var('s')) . ((get_query_var('paged')) ? '/page/'. get_query_var('paged') .'/' : '/'); if (!empty($_GET['s']) || !empty($_GET['paged'])) wp_redirect($search_uri); } }
#7
follow-up:
↓ 9
@
13 years ago
- Component changed from Template to Query
- Summary changed from Empty search takes you to hompage, even when homepage is not your blog to Empty search takes you to hompage instead of empty search page
- Version changed from 2.8.5 to 3.3
Other solutions are cropping up that indicate that this "should" really be updated.
What's the logic in not counting an empty search as search?
function fix_empty_search ($query){ global $wp_query; if (isset($_GET['s']) && empty($_GET['s'])){ $wp_query->is_search=true; } return $query; } add_action('pre_get_posts','fix_empty_search');
#9
in reply to:
↑ 7
@
13 years ago
- Version changed from 3.3 to 2.8.5
Please don't "bump" the version.
Replying to strop:
What's the logic in not counting an empty search as search?
The logic was that WP_Query can't distinguish between when the search parameter is empty vs. when it's not set at all.
#10
follow-up:
↓ 11
@
13 years ago
Like every patch involving WP_Query, extreme care will have to be applied in order to not cause other unexpected behavior.
#11
in reply to:
↑ 10
@
13 years ago
Replying to scribu:
Like every patch involving WP_Query, extreme care will have to be applied in order to not cause other unexpected behavior.
Of course, but it shouldn't stop an obvious bug from being fixed.
Sorry about the version bump.
#12
@
12 years ago
This issue has come up in my development cycle too.
I'm opting for strop's solution but I've modified it to work for me, especially the need to put is_home
to false
.
function search_query_filter($query) { // If 's' request variable is set but empty if (isset($_GET['s']) && empty($_GET['s']) && $query->is_main_query()){ $query->is_search = true; $query->is_home = false; } return $query; } add_filter('pre_get_posts', 'search_query_filter');
#13
@
12 years ago
- Summary changed from Empty search takes you to hompage instead of empty search page to Empty search takes you to homepage instead of empty search page
Closed #10156 as a duplicate.
#14
@
12 years ago
- Keywords has-patch added; needs-patch removed
Patch does the following:
1) WP_Query::is_search
is true if 's' query var exists (isset)
2) The search query becomes AND 0
if the term is empty so the SQL query will tank right away
3) Ditches the comment join generation if the search term is empty
Result: empty search terms go to the search page
#15
@
12 years ago
Replying to wonderboymusic:
1) WP_Query::is_search is true if 's' query var exists (isset)
This doesn't work as intended. Query vars (including 's'
) are always set in fill_query_vars()
:
http://core.trac.wordpress.org/browser/tags/3.4.2/wp-includes/query.php#L1356
With the patch applied, home page is displayed in search.php
template and shows "Nothing Found".
Related: #18485
#16
@
12 years ago
ah yes I see - I attached a new patch, is_search = not empty OR main_query and 's' query_var exists
#17
@
12 years ago
empty-search-string.2.diff seems to work.
#18
@
12 years ago
This seems like a bolted-on solution. Why should we treat just the 's' parameter differently?
#19
@
12 years ago
That's my problem with 800 line methods and 200 line loops (elsewhere), makes it hard to do surgery. I am open to alternative solutions.
#20
@
12 years ago
The length of the method has nothing to do with it, though it will probably grow, since refactoring for it's own sake is frowned upon, since we don't have unit tests to ensure that everything still works afterwards.
Anyway, the issue is that we ignore all empty query vars, except for 's', but I guess that's ok, since it's the only one that does a search, rather than an exact match. Well, we also have 'sentence', actually.
#25
@
11 years ago
Not so sure. Isn't 's' always going to be a filled query var? Seems like $_GET might have been desired here? Or, better probably, looking in $wp->query_vars.
#27
follow-up:
↓ 28
@
11 years ago
- Keywords needs-patch added; has-patch removed
- Milestone changed from 3.8 to Future Release
"s" is always going to be a filled query variable. This patch doesn't work.
#28
in reply to:
↑ 27
@
11 years ago
Replying to nacin:
"s" is always going to be a filled query variable. This patch doesn't work.
When I reproduce this anomaly, get_queried_object()
appears to return absolutely nothing. Could this be turned into a possible solution? When else is that the case, 404?
#29
@
11 years ago
- Keywords has-patch commit added; needs-patch removed
- Milestone changed from Future Release to 4.0
11330.2.diff checks $this->query
for existence of unadulterated's'
#30
@
10 years ago
- Owner set to wonderboymusic
- Resolution set to fixed
- Status changed from new to closed
In 28612:
#31
@
10 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
- Why limiting this to the main query? I'd expect
WP_Query( 's=' )
to work the same way, which would be consistent with #28099. array_key_exists()
can be replaced withisset()
, and removing$this->is_main_query()
makes the$qv['s']
check redundant (a non-empty$qv['s']
means that$this->query['s']
is set).$search = 'AND 0'
looks confusing where it is, it could be moved to a more appropriate place.
11330.3.diff addresses these points.
#33
@
10 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
This broke filters in the admin:
- Go to All Posts screen.
- Select any category in "View all categories" dropdown and click "Filter".
- You'll get "No posts found", because the URL contains an empty
s
parameter. - If you choose a month in "All dates" dropdown instead, you'll also get an SQL error:
WordPress database error: [You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '0 AND develop_posts.post_type = 'post' AND (develop_posts.post_status = 'publish' at line 1]
SELECT SQL_CALC_FOUND_ROWS develop_posts.ID FROM develop_posts WHERE 1=1 AND YEAR(develop_posts.post_date)=2014 AND MONTH(develop_posts.post_date)=06AND 0 AND develop_posts.post_type = 'post' AND (develop_posts.post_status = 'publish' OR develop_posts.post_status = 'future' OR develop_posts.post_status = 'draft' OR develop_posts.post_status = 'pending' OR develop_posts.post_status = 'private') ORDER BY develop_posts.post_date DESC LIMIT 0, 20
11330.4.diff fixes the SQL error.
I'm not quite sure what to do with the general filter breakage. Looks like this fix can lead to unexpected results elsewhere, so perhaps it should be reverted.
#34
@
10 years ago
I mean, we could add a ! is_admin()
check, but it still can affect custom filter forms on front-end.
#38
follow-up:
↓ 41
@
10 years ago
It sounds like this has the potential to break in the same way for faceted search forms on the front end of a site.
#39
in reply to:
↑ 35
@
10 years ago
Replying to wonderboymusic:
Prevent admin breakage for
s
inWP_Query
after [28623].
To clarify, $this->is_main_query()
is true for list tables, so this was technically broken in [28612], not in [28623].
Replying to johnbillion:
It sounds like this has the potential to break in the same way for faceted search forms on the front end of a site.
Yep, that's what I meant.
11330.5.diff uses $this->is_admin
and moves the check to a more appropriate place if we want to keep it.
#41
in reply to:
↑ 38
@
10 years ago
Replying to johnbillion:
It sounds like this has the potential to break in the same way for faceted search forms on the front end of a site.
We should probably drop the ' AND 0'
part (11330.6.diff).
The main point of this ticket (loading search template for an empty search instead of front page) is resolved.
#43
follow-up:
↓ 44
@
10 years ago
r28623 broke infinite scroll in Jetpack for the same reason #28099 did. See this comment for some insight.
#44
in reply to:
↑ 43
@
10 years ago
Replying to kovshenin:
r28623 broke infinite scroll in Jetpack for the same reason #28099 did. See this comment for some insight.
Would 11330.6.diff fix it, or should we revert [28612]/[28623]/[28666]/[28668] completely?
#46
@
10 years ago
Tested with .6.diff, seems to have resolved the issue.
Jetpack should be passing the values from ->query, not ->query_vars
As I mentioned in my comment on the other ticket, I believe it's fair to assume that the resulting query has been built with query_vars (which is publicly accessible), and by passing the same array to a new WP_Query, I would expect to get the same results. Obviously Jetpack and other plugins can use the query array instead, which is also publicly accessible, but they didn't.
#48
@
10 years ago
- Resolution set to fixed
- Status changed from reopened to closed
To summarize the changesets here, we've changed this:
if ( ! empty( $qv['s'] ) ) { $this->is_search = true; }
to this:
if ( isset( $this->query['s'] ) ) { $this->is_search = true; }
The main point of this ticket (loading search template for an empty search instead of front page) is resolved.
#49
@
10 years ago
I've reviewed this through [28804]. I concur with the decisions here. Looks good.
#50
@
10 years ago
- Keywords has-patch commit removed
Reporting some infinite scroll breakage (again), sorry! Since IS builds off of query_vars and not query, the scroll request sets an empty search, which is expected given this change. The results are still correct, however the context is wrong.
IS uses a theme's content.php file to produce output, and many such files will display the_content()
or the_excerpt()
based on is_search()
, like Twenty Twelve, so when scrolling Twenty Twelve is loading excerpts, rather than the post content because with an empty s the context is always search.
Changing IS to build the query from query and not query_vars seems to fix the issue, but reporting it anyway, just in case you guys wanna revert this ;)
#51
@
10 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening due to kovshenin's two-month-old comment.
This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.
10 years ago
#53
@
10 years ago
I actually checked out IS today on one of my sites. It was broken for a while, now it works. IS should be built by passing query
not query_vars
.
They patched for 4.0:
https://github.com/Automattic/jetpack/commit/baa5b7a8ffab784dbbb9f7e136dcbf387a6ffe38
Since they are passing to WP_Query
directly, those args are actually getting double parsed:
https://github.com/Automattic/jetpack/blob/master/modules/infinite-scroll/infinity.php#L900
Nobody makes a WP_Query
with every arg passed. If you only pass the args that were originally passed (represented by WP_Query->query
, you'll end up with all of those args anyways.
This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.
10 years ago
#55
@
10 years ago
We can probably leave it as is for now, haven't seen any other reports in alpha/beta forums.
itching to close this as wontfix.
you're describing the expected behavior, which works fine, so this technically is an enhancement. and then, it seems extremely specialized.