Opened 3 years ago

Last modified 3 months ago

#15256 reopened defect (bug)

?cat=## permalinks do not redirect when category is a parent

Reported by: froman118 Owned by:
Priority: high Milestone: Future Release
Component: Canonical Version: 3.0.1
Severity: normal Keywords: has-patch needs-testing needs-unit-tests early
Cc: peter@…

Description

One of my plugin users reported this while using dropdowns to display his categories: http://wordpress.org/support/topic/plugin-my-category-order-multiple-widget-support-broken

I was able to duplicate it on my own site in 3.0.1. Any category that has children displayed in the dropdown will redirect to /?cat=## instead of the the friendly /category-name permalink when selected. Select any other category or one of the children and they redirect correctly.

Any thoughts?

Attachments (3)

15256.patch (737 bytes) - added by SergeyBiryukov 21 months ago.
15256.diff (834 bytes) - added by dd32 21 months ago.
15256.2.diff (1.9 KB) - added by dd32 20 months ago.

Download all attachments as: .zip

Change History (20)

Just tested again and saw that it happens even if the child categories aren't displayed in dropdown.

comment:2   dd322 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

The dropdown list is doing exactly what it's designed to do. Give a cat input with the category ID.

In this case, Canonical should redirect it to the permalink, If that's not happening, please test on the latest wordpress 3.1 RC and re-open with feedback.

It indeed works.

  • Keywords changed from template, redirect, category to template redirect, category
  • Resolution wontfix deleted
  • Status changed from closed to reopened

As andrewspittle pointed out to me, cat=X doesn't redirect, there's no canonical there, only on category_name.

comment:5 follow-up: ↓ 8   dd322 years ago

  • Component changed from Permalinks to Canonical
  • Keywords changed from template redirect, category to template redirect category
  • Milestone set to Future Release
  • Summary changed from wp_dropdown_categories() redirects parent categories to /?cat=## to ?cat=## permalinks do not redirect when category is a parent

As andrewspittle pointed out to me, cat=X doesn't redirect, there's no canonical there, only on category_name.

Alright, So I've duplicated it. ?cat=123 redirects, as long as it's a child category or is a top level category with no children. ?cat=456 doesn't redirect if it has childen, this is because the canonical code doesn't redirect on multiple category queries (which a parent category query is)

A quick hack "hack" would be to restore a block like this before the current taxonomy redirection code:

		} elseif ( is_category() && !empty($_GET['cat']) ) {
			if ( $redirect_url = get_term_link( (int)$_GET['cat'], 'category') )
				$redirect['query'] = remove_query_arg('cat', $redirect['query']);

however, that has the downside that it'll probably break on other taxonomy queries, or multi-query_var queries, which is what the current code was designed to fix.

  • Keywords needs-patch added; template redirect category removed
  • Milestone changed from Future Release to 3.3
  • Priority changed from normal to high

Just pushing this into 3.3 and raising the priority, This should've never occurred, and needs to be fixed.

Hi folks

Ran into this on the latest release (version 3.2.1)

The bug is actually not just cosmetic, and in my case it causes serious breakage.

The breakage occurs if, like in my case, posts have a custom permalink structure. In my case I use "/places/%postname%"

When that's the case, and there are more than N posts in the category index requiring pagination, the breakage is as follows:

  1. User chooses one of the affected categories, goes to URL /?cat=5 and does not get redirected to slug
  2. User scrolls past N entries, uses pagination to click on page 2
  3. User goes to URL: /page/2?cat=5
  4. Which is a broken/invalid URL. Instead of seeing the listing for page 2, they see the homepage

It'd be great if this can be fixed in the next release. All my current options suck (disabling pagination / making the entries-per-page a large number / removing child categories / disabling pretty permalinks and using the default /?p=X ).

comment:8 in reply to: ↑ 5   SergeyBiryukov21 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

Replying to dd32:

A quick hack "hack" would be to restore a block like this before the current taxonomy redirection code:

		} elseif ( is_category() && !empty($_GET['cat']) ) {
			if ( $redirect_url = get_term_link( (int)$_GET['cat'], 'category') )
				$redirect['query'] = remove_query_arg('cat', $redirect['query']);

however, that has the downside that it'll probably break on other taxonomy queries, or multi-query_var queries, which is what the current code was designed to fix.

15256.patch restores this block along with the condition from [10396], to prevent redirect on ?cat=1,2. Doesn't seem to break anything so far, but needs more testing.

Or is there a better way to fix this?

Replying to minaguib:

  1. User goes to URL: /page/2?cat=5
  2. Which is a broken/invalid URL. Instead of seeing the listing for page 2, they see the homepage

Can't reproduce this. URL like that still shows page 2 of category 5. Didn't check 3.2.1 though, only 3.3-trunk.

dd3221 months ago

comment:9 follow-up: ↓ 11   dd3221 months ago

Or is there a better way to fix this?

I've been thinking about it a bit, and in testing, it's hit me that only categories (not custom taxonomies, etc) include children in the Tax Query array, which is where the issue is originating from.

Upon looking into WP_Tax_Query I've found that children are, by default, included - rendering query.php loading them being redundant. The attached patch removes category children from the array, allowing WP_Tax_Query to fall back to it's default of loading the children itself. Logically, this results in no logic change to the Query process, but does however, fix the Canonicalism process (as now WP_Query now states that a single category was requested - which is true)).

However, 15256.patch is also viable. I'd like someone elses opinion on it before going ahead with either though.

15256.diff seems to make much more sense as it corrects the root problem rather than a symptom.

comment:11 in reply to: ↑ 9   SergeyBiryukov21 months ago

Replying to dd32:

The attached patch removes category children from the array, allowing WP_Tax_Query to fall back to it's default of loading the children itself.

That would result in #16152 again.

That would result in #16152 again.

Hmm Good catch.. I hadn't blamed it right. Ultimately, cat != category__in

dd3220 months ago

attachment 15256.2.diff added

Yet another shot at the root cause. Took the line of parsing 'cat' to it's own tax query rather than piggybacking on category__in. Doesn't appear to regress anything that I can see, If a query is using both 'cat' and 'category__in' that'll result in 2 queries in WP_Tax_Query (unless it rolls same/similar queries into one)

  • Keywords needs-unit-tests added
  • Keywords early added
  • Milestone changed from 3.3 to Future Release

This needs to be fixed early in a cycle.

  • Cc peter@… added

As of WordPress 3.5.1 this is still an issue.

Any plans on fixing this?

#23604 was marked as a duplicate.

Note: See TracTickets for help on using tickets.