Make WordPress Core

Opened 15 years ago

Closed 10 years ago

Last modified 8 years ago

#13258 closed enhancement (duplicate)

wp_dropdown_categories() uses $term->id instead of $term->name for taxonomies that are not categories

Reported by: mfields's profile mfields Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Taxonomy Keywords: has-patch needs-refresh
Focuses: Cc:

Description

I was excited to discover that wp_dropdown_categories() had been extended to support custom taxonomies but when I tried to implement it, the navigation failed because it always uses the $term->ID in the value attribute of the option tag for each term.

<option class="level-0" value="13">A Category</option>

This makes sense because WordPress category requests uses the term id: /?cat=13

But in the case of tags, we get this code:

<option class="level-0" value="3">My Tag</option>

which pulls a 404 when you request: /?tag=3

Attachments (3)

wp-dropdown-categories-slug-value.patch (1.2 KB) - added by stephenh1988 12 years ago.
Adds an optional 'value' argument to specify what should be used as for the value of the option (slug or ID). If unspecified reverts to ID for categories and slug otherwise.
category-template.php.patch (949 bytes) - added by zaharamh 12 years ago.
Adds an optional 'value' argument to specify what should be used as for the value of the option (slug or ID). If unspecified reverts to ID for categories and slug otherwise.
13258.patch (1.8 KB) - added by jfarthing84 10 years ago.

Download all attachments as: .zip

Change History (25)

#1 follow-up: @nacin
15 years ago

wp_dropdown_categories() only returns HTML. I imagine you're referring to the default widget in particular?

#2 in reply to: ↑ 1 @mfields
15 years ago

Replying to nacin:

wp_dropdown_categories() only returns HTML. I imagine you're referring to the default widget in particular?

I actually just finished up a plugin http://wordpress.mfields.org/2010/screenshot-of-the-new-taxonomy-widget-plugin/ that is a fork of the categories widget. It would have been simple to create except for the bug in wp_dropdown_categories(). To fix the bug, I had to rewrite functionality of wp_dropdown_categories(), walk_category_dropdown_tree() and the Walker_CategoryDropdown class.

The reported bug is in the html outputed by the Walker_CategoryDropdown class mainly this line:

$output .= "\t<option class=\"level-$depth\" value=\"".$category->term_id."\"";

Which works great for categories by does not work at all for other taxonomies.

/?cat=14

/?tag=my-tag-name

/?taxonomy=my-taxonomy-term-name

Categories are the only post taxonomy queried by id number, all others use slug. I reported this as a bug because I was looking through core and saw that wp_dropdown_categories() now allows the user to pass taxonomy as a key in the $args array.

This would be awesome to have fixed in core. Hopefully I have explained the issues well enough. You can see the changes that I needed to make to get my plugin functioning correctly here http://wordpress.pastebin.com/scckuBhp.

Thanks for your attention!

#3 @mfields
15 years ago

  • Cc mfields added

#4 @nacin
15 years ago

  • Keywords 2nd-opinion added; wp_dropdown_categories tag taxonomy category removed
  • Milestone changed from 3.0 to 3.1
  • Type changed from defect (bug) to enhancement

I want to move this to 3.1. I'm not really sure the best way to do restructure this. Ultimately, the categories widget still only takes categories. Ideally, we'd allow either a walker override, or the ability to use the slug instead of the ID.

The slug instead of the ID should probably be default for non-category taxonomies, which makes me want to revert taxonomy support for 3.0 from wp_dropdown_categories, or at least force a check in the walker to use the slug instead of the ID if it's a category. Then again, there are plenty of use cases (the JS functionality of the widgets not one of them) for the ID.

#5 @kevinB
14 years ago

  • Cc kevinB added

#6 @nerrad
14 years ago

This bug also applies if a person wants to add a custom filter for taxonomies to edit.php . WP is expecting custom-taxes to have a slug as the query_var rather than the term_id and this means that the existing wp_dropdown_categories won't work for the custom filter. Like mfields, I have to create a custom dropdown function for the custom filter.

#7 @t31os_
14 years ago

  • Cc wp-t31os@… added

#8 @nacin
14 years ago

  • Milestone changed from Awaiting Triage to 3.1

Viper007Bond is going to try to cook up a patch for this.

#9 @markjaquith
14 years ago

  • Milestone changed from 3.1 to Future Release

No patch. Punting.

#10 @thomask
13 years ago

  • Keywords needs-patch added

Please patch this very old bug, 2 years old you postponed to 3.1, but it is not even in 3.4 - tons of people got problem with this (http://wordpress.org/tags/wp_dropdown_categories)

and the solution is IMO very simple - just use

$output .= "\t<option class=\"level-$depth\" value=\"".$category->term_slug."\"";

instead of

$output .= "\t<option class=\"level-$depth\" value=\"".$category->term_id."\"";

on wp-includes/category-template.php row 953 for all other taxonomies then $category (or even for category and change the default select name to category instead of cat), or add another option to wp_dropdown_categories e.g. 'values' with default 'term_id' and then do something like

$output .= "\t<option class=\"level-$depth\" value=\"".$category->{$values}."\"";

@stephenh1988
12 years ago

Adds an optional 'value' argument to specify what should be used as for the value of the option (slug or ID). If unspecified reverts to ID for categories and slug otherwise.

#11 @stephenh1988
12 years ago

  • Cc stephen@… added
  • Keywords has-patch added; needs-patch removed

In the meantime, its possible to replicate this by passing a custom walker which extends the Walker_CategoryDropdown class and replaces the start_el method. See this gist: https://gist.github.com/2902509

#12 @vanjwilson
12 years ago

I just ran into this issue in a different context.

I use wp_dropdown_categories in a meta box to let the user pick a "primary" term from a custom taxonomy for a related custom post. Then, I filter get_posts in a shortcode to insert a list custom posts with that meta value. Only the dropdown still has the id values, so my shortcode query would by dependent on the order the custom terms were entered in the database.

I will have to use the custom walker (which is in use in the wild: http://stackoverflow.com/questions/14078572/how-to-change-value-for-options-and-select-in-wp-dropdown-categories), but why hasn't stephen1988's patch made it into core yet?

#13 @vanjwilson
12 years ago

  • Cc vanjwilson added

@zaharamh
12 years ago

Adds an optional 'value' argument to specify what should be used as for the value of the option (slug or ID). If unspecified reverts to ID for categories and slug otherwise.

#14 @zaharamh
12 years ago

  • Cc zaharamh added
  • Keywords needs-testing added

Stephen1988's patch seems to create several issues:

  • breaks the "selected" argument
  • adds a "show_last_update" argument that returns an error since no such argument exists

I fixed those two things and also tested the patch, but I am not sure it is in working condition yet. Issues I find

  • it will break if you don't set the "name" argument with the taxonomy's slug (but maybe that's a different issue altogether)
  • it will break if you set value to "term_id" when using anything other than category (is the possibility of having an option really needed, or wouldn't it be better to just have it default to id for categories and slug for other taxonomies?)
Last edited 12 years ago by zaharamh (previous) (diff)

#15 @pbaylies
11 years ago

  • Cc pbaylies added

#17 @jfarthing84
11 years ago

This definitely needs traction after 3 years!

#18 @jfarthing84
11 years ago

  • Cc jeff@… added

#19 @lchski
11 years ago

  • Cc lucas@… added

#20 @wonderboymusic
11 years ago

  • Keywords needs-refresh added; 2nd-opinion needs-testing removed

We need another patch here that follows WP coding standards as pertains to whitespace and produces code that doesn't come with "It will break" disclaimers.

@jfarthing84
10 years ago

#21 @boonebgorges
10 years ago

  • Milestone Future Release deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Whoops, this was fixed with the introduction of the 'value_field' parameter in [31006] #30306. You can use 'value_field' to specific id, slug, or whatever other term field you want to serve as the "value" attribute of the dropdown options.

As for the precise issue that led to the opening of this ticket: The job of the category/term dropdown is to generate a dropdown. It does that perfectly fine. It so happens that, in the case of categories, the value of the dropdown can be used to concatenate a certain kind of URL (?cat=13), while the precise same thing can't be done in the case of other taxonomies. But this is not a bug. It just means that devs should be responsible for sanitizing data they get from form submits before generating URLs or any other user-facing data.

#22 @wewlad
8 years ago

Since this ticket is closed - you might want to do something about the code in the block starting at https://github.com/woothemes/woocommerce/blob/master/includes/widgets/class-wc-widget-product-categories.php#L210

Note: See TracTickets for help on using tickets.