Legacy get_category_link function call in get_term_link function should probably use $term->ID or be removed

[ev3rywh3re]

To replicate this bug in a loop somehow use something like:

$cats = get_the_term_list( $post->ID, 'category', '', '|', '' );

In this case get_the_term_list puts together a term object but then when it calls get_term_link the get_term_link function tries to pass the term object to get_category_link which is really asking for a category ID.

The culprit is wp-includes/taxonomy.php lines 2122-2123

	if ( $taxonomy == 'category' )
		return get_category_link($term);

For testing I removed that line and it worked when using get_the_term_list from the example. I also modified $term to pass $term->ID instead and that worked as well. There's no patch because it has multiple solutions and I don't think I have enough test cases to make a proper judgment call for a solution.

I know category functions need to be modified to use the taxonomy functions more elegantly. I also know this is considered an internal function and get_the_category_list should be used instead, but I mistakenly thought get_the_term_list was pretty cool looking.

[ev3rywh3re]

Great, I may have reported too soon. It may be a Dreamhost PHP CGI issue. Other systems I maintain don't seem to cause an error. Switching that domain to Fast-CGI fixed it. The error does not reproduce on the mod_php version 4.x or 5.x servers I maintain.

[filosofo]

Replying to ev3rywh3re:

Great, I may have reported too soon. It may be a Dreamhost PHP CGI issue. Other systems I maintain don't seem to cause an error. Switching that domain to Fast-CGI fixed it. The error does not reproduce on the mod_php version 4.x or 5.x servers I maintain.

Actually, what you're probably seeing is that the bug shows up only if "pretty" permalinks are not enabled.

I've attached a patch with what I think is the best solution: make get_term_link() accept only a term id as its first argument, instead of the multiple options it currently (sorta) does: term object, term id, or term slug.

This is only place in core WP where get_term_link() is called with something that might not be a term ID, and that can easily be corrected (see patch). Also, both get_category_link() and get_tag_link() expect term IDs, so it's consistent behavior to require the ID.

[Denis-de-Bernardy]

there would be a few calls to get_term_link() that would need to be changed, no? namely in get_the_taxonomies()

[filosofo]

Replying to Denis-de-Bernardy:

there would be a few calls to get_term_link() that would need to be changed, no? namely in get_the_taxonomies()

Don't know why I missed that one, but I've refreshed the patch.

[Denis-de-Bernardy]

I dunno how many plugins this'll break if any were relying on get_term_link($term) to work, but patch works.

An alternative approach would be to make get_tag_link and get_category_link work with terms too.

[filosofo]

Replying to Denis-de-Bernardy:

I dunno how many plugins this'll break if any were relying on get_term_link($term) to work, but patch works.

Just for fun I grepped through the plugins repository. Of over 7000 plugins, 3 call get_term_link() and only 1 of them, Custom Taxonomies, passes something other than a term id to the first argument. If we can commit this, I'll be happy to send the author an email to let him know.

[Denis-de-Bernardy]

I'm +1 to the patch, then.

[Denis-de-Bernardy]

broken patch

[filosofo]

Refreshed the patch.

[Denis-de-Bernardy]

+1

[filosofo]

Refreshed patch again against [11257]

[ryan]

Whoops, didn't see this one.

[Denis-de-Bernardy]

broken again. :-(

[Denis-de-Bernardy]

nevermind. svn updated and it worked.

[ryan]

(In [11263]) Accept only IDs for get_term_link(). Prop filosofo. fixes #9323

[ryan]

(In [11324]) Revert [11263]. Busted several themes. see #9323

[ryan]

This broke several themes and plugins, especially ones that used get_term_link() as part of their custom taxonomy support.

[ryan]

I think [11257] fixed the core problem.