Make WordPress Core

Opened 13 years ago

Last modified 22 months ago

#18734 new defect (bug)

Subcategory archive does work with any name as parent category in URL

Reported by: ocean90's profile ocean90 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.0.1
Component: Canonical Keywords: has-patch dev-feedback needs-testing needs-refresh
Focuses: Cc:

Description

Parent category is parentcategory and his sub category is subcategory.

The URL will be domain.com/category/parentcategory/subcategory.

The problem is, that you will get the same page if you use any words as parentcategory.

Examples:

  • domain.com/category/xxx/subcategory
  • domain.com/category/subcategory
  • domain.com/category/foo/bar/subcategory

IMO redirect_canonical should do his work here (and sometimes it does).

In 3.1 it does redirect.
In 3.1.4 it doesn't redirect; after r17549.
In 3.2.1 it doesn't redirect.
Duck_ found that it does redirect before r18079.
In current trunk it doesn't redirect.

Attachments (4)

18734-tests.patch (1.2 KB) - added by boonebgorges 9 years ago.
Removing from trunk as per #30284
18734.diff (1.5 KB) - added by kasparsd 8 years ago.
Compare the full category paths
18734-proper-replace.diff (1.5 KB) - added by kasparsd 8 years ago.
Include the category permastructure in the replacement strings.
18734-tests.diff (3.2 KB) - added by kasparsd 8 years ago.
And here are the tests.

Download all attachments as: .zip

Change History (22)

#1 @duck_
13 years ago

  • Keywords needs-unit-tests added

We need tests for all the bugs the have been fixed and unfixed over time.

Related: #17174, #16627, #12659

#2 @duck_
12 years ago

  • Version changed from 3.1 to 3.0.1

Some more canonical tests: [UT444]

History in trunk

[13091] for original /category/child/ to /category/parent/child/ redirection.

Broken by [15462] as "The easiest one is to assume that if we had come to the right category page without any get variables, we don't need the logic for redirecting to the canonical category page. This is valid statement, because that logic relies only on removing get arguments." (#14201) is incorrect. parse_url($tax_url) ... $redirect['path'] = $tax_url['path']; is redirecting to the correct term link.

Fixed again in [15705], but re-introducing #14201. The scope of the !empty($redirect['query'] conditional was changed.

Re-broken by [18079] which moved stuff back inside !empty($redirect['query'] (also fixing #14201 again).

tl;dr: canonical isn't kicking in unless the URL has a query. This was done as a stop-gap fix for #14201.

#3 @ocean90
12 years ago

  • Keywords needs-patch added; dev-feedback removed
  • Milestone changed from Awaiting Review to Future Release

#4 @ocean90
12 years ago

Duck_, any idea what we can do here?

#5 @juliobox
11 years ago

  • Cc juliobosk@… added

Waiting for a fix, i created this hack:

add_action( 'wp', 'baw_non_duplicate_content' );
function baw_non_duplicate_content()
{
	global $wp_query;
	if( isset( $wp_query->query_vars['category_name'], $wp_query->query['category_name'] )
		&& $wp_query->query_vars['category_name'] != $wp_query->query['category_name'] ):

		$correct_url = str_replace( $wp_query->query['category_name'], $wp_query->query_vars['category_name'], $wp->request );
		wp_redirect( home_url( $correct_url ), 301 );
		die();
	endif;
}

#6 @SergeyBiryukov
10 years ago

#25737 was marked as a duplicate.

#7 follow-up: @wonderboymusic
10 years ago

This hasn't received any action in a while ... should these redirects be occurring? are we intentionally not touching this?

#8 in reply to: ↑ 7 @SergeyBiryukov
10 years ago

Replying to wonderboymusic:

should these redirects be occurring?

This crops up on support forums once in a while, so I would say yes, as long as it doesn't introduce any regressions. Looks like this should be handled together with #14201.

@boonebgorges
9 years ago

Removing from trunk as per #30284

#9 @ocean90
9 years ago

#31022 was marked as a duplicate.

#10 @dd32
9 years ago

#31022 was marked as a duplicate.

@kasparsd
8 years ago

Compare the full category paths

#11 @kasparsd
8 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

Attached is a patch that will

  • ensure that the current category has a parent category,
  • ensure that the category permastructure actually contains the category slug %category%

Only then it will

  • generate a list of all parent categories,
  • check if the category path matches the $wp->query_vars['category_name']
  • replace the incorrect category path $wp->query_vars['category_name'] with the correct one

Notice that this approach will respect any pagination, feeds and custom endpoints that might be present in the URL.

@kasparsd
8 years ago

Include the category permastructure in the replacement strings.

@kasparsd
8 years ago

And here are the tests.

This ticket was mentioned in Slack in #core by kaspars. View the logs.


8 years ago

#13 @kasparsd
8 years ago

  • Keywords needs-unit-tests removed

#14 @BinaryKitten
8 years ago

  • Keywords needs-testing added

#15 @ocean90
4 years ago

  • Keywords needs-refresh added
  • Milestone set to Future Release

#16 @SergeyBiryukov
2 years ago

#41211 was marked as a duplicate.

#17 @SergeyBiryukov
2 years ago

#55273 was marked as a duplicate.

#18 @dd32
22 months ago

#55735 was marked as a duplicate.

Note: See TracTickets for help on using tickets.