WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 3 months ago

#11330 new defect (bug)

Empty search takes you to homepage instead of empty search page

Reported by: jacobfogg Owned by:
Milestone: Future Release Priority: low
Severity: minor Version: 2.8.5
Component: Query Keywords: needs-patch
Focuses: Cc:

Description

  1. 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".
  2. Visit your blog (http://localhost/ in my case) and you will see your static page.
  3. Navigate to your blog (http://localhost/blog/ in my case).
  4. 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.
  5. 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 (3)

empty-search-string.diff (1.2 KB) - added by wonderboymusic 19 months ago.
empty-search-string.2.diff (1.3 KB) - added by wonderboymusic 19 months ago.
11330.diff (1.4 KB) - added by wonderboymusic 9 months ago.

Download all attachments as: .zip

Change History (31)

comment:1 Denis-de-Bernardy4 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

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.

comment:2 strider724 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.

comment:3 scribu4 years ago

This is related to #10710.

comment:4 jacobfogg4 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.

comment:5 jacobfogg4 years ago

  • Cc jacobfogg added

comment:6 thomask4 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);
    }
}

comment:7 follow-up: strop2 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');
Last edited 2 years ago by strop (previous) (diff)

comment:8 strop2 years ago

  • Cc oskar@… added

comment:9 in reply to: ↑ 7 scribu2 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.

comment:10 follow-up: scribu2 years ago

Like every patch involving WP_Query, extreme care will have to be applied in order to not cause other unexpected behavior.

comment:11 in reply to: ↑ 10 strop2 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.

comment:12 foxinni20 months 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');

comment:13 SergeyBiryukov20 months 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.

comment:14 wonderboymusic19 months 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

comment:15 SergeyBiryukov19 months 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

Last edited 19 months ago by SergeyBiryukov (previous) (diff)

comment:16 wonderboymusic19 months ago

ah yes I see - I attached a new patch, is_search = not empty OR main_query and 's' query_var exists

comment:18 scribu19 months ago

This seems like a bolted-on solution. Why should we treat just the 's' parameter differently?

comment:19 wonderboymusic19 months 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.

comment:20 scribu19 months 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.

comment:21 dcowgill15 months ago

  • Cc dcowgill@… added

comment:22 MikeHansenMe14 months ago

  • Cc mdhansen@… added

comment:23 dotnetCarpenter10 months ago

  • Cc dotnetCarpenter added

wonderboymusic9 months ago

comment:24 wonderboymusic9 months ago

  • Milestone changed from Future Release to 3.7

Refreshed, works

comment:25 nacin7 months 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.

comment:26 nacin6 months ago

  • Milestone changed from 3.7 to 3.8

comment:27 follow-up: nacin4 months 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.

comment:28 in reply to: ↑ 27 GhostToast3 months 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?

Note: See TracTickets for help on using tickets.