WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 20 months ago

Last modified 18 months ago

#8823 closed defect (bug) (worksforme)

Canonical redirects when it should not

Reported by: andy Owned by: markjaquith
Milestone: Priority: normal
Severity: normal Version: 2.7
Component: Canonical Keywords:
Focuses: Cc:

Description

With redirect_canonical disabled, this works:
http://celebrity-babies.com/?year=2008&cat=36894

With it enabled, it redirects here:
http://celebrity-babies.com/category/main/breastfeeding/

This kind of query cannot be expressed as a permalink. redirect_canonical should not redirect to a permalink unless the permalink perfectly matches the query in the URL.

Change History (15)

comment:1 filosofo5 years ago

See #5989

As I point out there, there is a way for it resolve to a permalink, one that already works in WP:

http://celebrity-babies.com/category/main/breastfeeding/2008/

comment:2 filosofo5 years ago

I mean, when you have the right permalink structure enabled, such as

/%category%/%year%/%monthnum%/%day%/%postname%/

it works already, and it should work the same way for requests such as you mention.

comment:3 andy5 years ago

No matter what your permalink structure, currently redirect_canonical will convert some queries into the wrong permalink. It must not do this.

comment:4 akhost5 years ago

  • Cc akhost added

comment:5 akhost5 years ago

  • Cc akhost@… added; akhost removed

Looks like I have to CC my email, not my username, sorry.

comment:6 follow-up: jeremyclarke5 years ago

  • Cc jeremyclarke added

filosofo: That just doesn't work for me either and I have a permalink structure like the one you named. There is no accesible way to get the date in with a category name.

Just to be clear, the goal is to show only posts from that category and that time period, not to get to a specific post as your example implies. Changing the post permalink structure seems to have no effect for me, and if I add stuff like that in the 'category base' option it just creates broken links with the replacements (%year%) still inside.

It's not a simple problem but its an important one to face, as cat+month is pretty obvious as a need.

@andy and @akhost: My solution for this was to disable redirect_canonical selectively. In my plugin code I check the query variables, and if 'm' (month) is present then I disable redirect canonical, otherwise I leave it alone. This lets the rest of the SEO benefits work without stopping the month archives from working.

// --------------------------------------------
// Filter redirect_canonical to disable it for ?m=
// --------------------------------------------
//
// End redirection any time there is a $m value in the url
// Passing FALSE to redirect_canonical filter ends redirection
// Needed for ?cat=x&m=y style urls to work. Otherwise you are 
// forwarded to category without month. 

// DISABLED: this turns off canonical redirectoin completely, which is overkill. see above.
// remove_filter('template_redirect', 'redirect_canonical'); 

function gv_redirect_canonical($redirect_url) {
	global $wp_query, $wp;
	if ((int) $_GET['m']) return FALSE;
	return $redirect_url;
} //gv_redirect_canonical()

add_filter('redirect_canonical', 'gv_redirect_canonical');

comment:7 in reply to: ↑ 6 filosofo5 years ago

Replying to jeremyclarke:

filosofo: That just doesn't work for me either and I have a permalink structure like the one you named. There is no accesible way to get the date in with a category name.

Well, it works for me with that permalink structure. :) But my point is not that it's always working correctly. Clearly it's not---that's why I have a related ticket already. My point is to respond to andy's statement that "This kind of query cannot be expressed as a permalink." I'm saying that there is a logical permalink that expresses that query, and WordPress should redirect to that permalink (which also should work all the time).

Just to be clear, the goal is to show only posts from that category and that time period, not to get to a specific post as your example implies.

I agree with that goal; I didn't mean to imply otherwise.

comment:8 akhost5 years ago

Could you please clarify-- when saying it should redirect and work all of the time, does that mean you will be adding some sort of supported permalink structure for this query in the future? Thanks!

comment:9 jeremyclarke5 years ago

@akhost: I'm talking to him in IRC right now. He means that regardless of whether you CAN make a link that works right now (without using the %category% element in your post urls, which is very bad in many ways and isn't a viable solution) it SHOULD work, so yes, he means that it needs to be worked out.

I think his main intention was to say that we should NOT depend on raw urls like ?cat=1&m=200808, but instead figure out nice permalinks that can solve the problem instead (though the ugly versions should still be converted if they are used).

Does that makes sense? Our choice solution would be to (once the redirects are worked out) also have a new function for creating pretty permalinks with multiple query vars, something like:

get_archive_url('cat=1&m=200808');

Who knows when that might be done though, so for now I recommend my workaround until it happens.

comment:10 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added
  • Milestone changed from 2.7.2 to Future Release

comment:11 Denis-de-Bernardy5 years ago

  • Milestone changed from Future Release to 2.9

comment:12 janeforshort4 years ago

  • Milestone changed from 2.9 to Future Release

Still needs patch, punting b/c we're in beta.

comment:13 dd323 years ago

Current status:

URL: http://localhost/wordpress/?year=2010&cat=1 redirects to http://localhost/wordpress/2010/?cat=1 which redirects to http://localhost/wordpress/category/uncategorized/

comment:14 wonderboymusic20 months ago

  • Keywords needs-patch removed
  • Resolution set to worksforme
  • Status changed from new to closed

comment:15 ocean9018 months ago

  • Milestone Future Release deleted
Note: See TracTickets for help on using tickets.