WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#10552 closed enhancement (fixed)

Add get_search_link()

Reported by: sirzooro Owned by: dd32
Milestone: 3.0 Priority: normal
Severity: normal Version: 2.8.3
Component: Permalinks Keywords: needs-patch needs-testing
Focuses: Cc:

Description

WP_Rewrite class provides get_search_permastruct() method, but it is not used. It will be good to add get_search_link() function for completness. I am not sure where it should go (which file), so I post it here:

function get_search_link( $query = null ) {
	global $wp_rewrite;
	$searchlink = $wp_rewrite->get_search_permastruct();

	if ( is_null( $query ) )
		$query = get_search_query();

	$query = rawurlencode( $query );

	if ( empty( $searchlink ) ) {
		$file = trailingslashit( get_option( 'home' ) );
		$searchlink = $file . '?s=' . $query;
	} else {
		$searchlink = str_replace( '%search%', $query, $searchlink );
		$searchlink = trailingslashit( get_option( 'home' ) ) . user_trailingslashit( $searchlink, 'category' );
	}
	return apply_filters( 'search_link', $searchlink, $query );
}

Attachments (2)

link-template.diff (1.0 KB) - added by jshreve 6 years ago.
link-template.2.diff (1.1 KB) - added by jshreve 6 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 @ryan6 years ago

  • Milestone changed from 2.9 to 3.0

comment:2 follow-up: @sirzooro6 years ago

Could you reconsider adding this function in 2.9? I had to add the same code to two of my plugins (Meta SEO Pack and Recent Searches Widget), and would like to use built-in function if possible.

comment:3 @sirzooro6 years ago

  • Milestone changed from 3.0 to 2.9

comment:4 in reply to: ↑ 2 @westi6 years ago

  • Milestone changed from 2.9 to 3.0

Replying to sirzooro:

Could you reconsider adding this function in 2.9? I had to add the same code to two of my plugins (Meta SEO Pack and Recent Searches Widget), and would like to use built-in function if possible.

Now that 2.9 has gone to beta we are in feature freeze and want to concentrate on bugs in the new features and regressions against 2.8.6

Unfortunately that means this will have to wait until 3.0

comment:5 @sirzooro6 years ago

OK, I will wait 3.0.

BTW, I think it will be good to perform review of all enhancements before entering feature freeze - as I checked, at this moment 77 enhancements are milestoned for 3.0, and 48 of them has patches.

comment:6 @jshreve6 years ago

  • Cc justin.shreve@… added

I don't want this patch to be forgotten about. It fits in with some of the other search functionality being worked on (#10667) and would be an important addition in my opinion.

I rewrote this a bit to use some of the new 3.0 functions (home_url mainly) and to use get_search_query. I also put it in patch form (instead of a pasted function like above) so it can be tested some more.

comment:7 @dd326 years ago

  • Keywords needs-patch added; has-patch removed

needs-patch: $permalink = home_url("?page_id=$search"); I somewhat dont think you meant page_id :)

Seems like its a useful addition to me other than that.

@jshreve6 years ago

comment:8 @jshreve6 years ago

@dd32 thanks - was testing it with permalinks ON and missed that. updated & everything else should be ok

comment:9 @dd326 years ago

  • Owner changed from ryan to dd32
  • Status changed from new to reviewing

I'm also not sure if esc_attr() is the right escaping function to use here, If indeed it should be escaped at all. urlencodes are required, but the calling function should escape the link appropriately.

comment:10 @jshreve6 years ago

I based it off of how get_search_feed_link & get_search_comments_feed_link were working. (I assuming there was a reason for doing this). They should already be clean but it looks like it's being used in a few places so if it's ok then those functions could be changed too.

comment:11 @sirzooro6 years ago

There is a problem when someone will include slash in search - you will get url like site.com/search/foo%2Fbar/ . But when you will try to open it in browser, you will get 404. And there is an interesting part - for some reason this url is not handled by WP, but error is returned by Apache itself (at least on my dev machine with Windows).

When you do not encode slahses (url like site.com/search/foo/bar/), you will get 404 served by WP.

comment:12 @jshreve6 years ago

sirzooro: I meant te esc attrs that some of the other functions have.

comment:13 @dd326 years ago

When you do not encode slahses (url like site.com/search/foo/bar/), you will get 404 served by WP.

This is due to Apache discarding the invalid URL.

/search/test%2Fpost/ is NOT a valid url, however ?s=test%2Fpost IS a valid url. %2F may appear in a URL query string, but not its path.

Valid URL's would be in the form of:

http://localhost/wordpress-commit/search/test/post/
OR
http://localhost/wordpress-commit/search/test%25Fpost/

The problem with the 2nd one however, Is that WordPress doesnt double-urldecode the link.

It seems Apache has a 'AllowEncodedSlashes' option, which transforms %2F to / before its run by the plugins.

one way around this is:

function get_search_link( $query = '' ) {
	global $wp_rewrite;

	if ( empty($query) )
		$search = get_search_query();
	else
		$search = stripslashes($query);

	$permalink = $wp_rewrite->get_search_permastruct();

	if ( empty( $permalink ) ) {
		$permalink = home_url('?s=' . urlencode($search) );
	} else {
		$search = urlencode($search);
		$search = str_replace('%2F', '/', $search); // %2F(/) is not valid within a URL, Pre-encode it so its double-encoded.
		$permalink = str_replace( '%search%', $search, $permalink );
		$permalink = home_url( user_trailingslashit( $permalink, 'search' ) );
	}

	return apply_filters( 'search_link', $permalink, $search );
}

@jshreve6 years ago

comment:14 @jshreve6 years ago

line-template.2.diff is your above changes.

Tested against r13402. Works like it should.

@dd32: Do you know if there a reason we aren't use the permastructure for the search feeds and search comment feeds? I'll probably write up a small patch to attach here to stay consistent.

comment:15 @dd326 years ago

: Do you know if there a reason we aren't use the permastructure for the search feeds and search comment feeds?

Honestly, No.
Part of the reason could be that JavaScript is required on the client side to generate that type of URL.

But aside from that, Most people know of the ?s= style for searches...

comment:16 @dd326 years ago

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

(In [13483]) Add get_search_link() with Permastruct (/search/) support. Props jshreve. Update get_search_link(), get_search_feed_link() & get_search_comments_feed_link() to use it, as well as supporting Permastructs for those feeds. Fixes #10552

Note: See TracTickets for help on using tickets.