WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 8 months ago

#16600 reopened defect (bug)

AdminMenu rendering code chokes on uppercase

Reported by: jltallon Owned by:
Milestone: Awaiting Review Priority: normal
Severity: minor Version: 3.1
Component: Administration Keywords:
Focuses: Cc:

Description

When registering custom taxonomies for a custom post type, if the post type name contains uppercase letters, the menu items for the custom taxonomies are not shown.

It looks like the post type name is lowercased in some parts of the core code (but no all) and so string comparisons fail.
Just isolated this behaviour, so I'm still not sure how much of the core this affects.

Moreover, when I saved posts with the intended mixed-case name, they have been saved to the DB with lowercased post_type

It it is indeed the intended behaviour, a note should be added to the Codex:

"Custom Post Type names must be lowercase"

The attached sample (real) code reproduces the problem by setting "post_type_tag" to "DomainName" instead of "domain_name". Numbers work properly, however.

Tested with latest SVN (RC4 + r17467)

Attachments (4)

cpt_test.php (2.1 KB) - added by jltallon 3 years ago.
Test case for "C-P-T case sensitivity" - 'DomainName' vs 'domainname'
cpt_test.2.php (2.2 KB) - added by greuben 3 years ago.
patch.16600a.diff (3.6 KB) - added by jltallon 3 years ago.
Implement 'sanitize_objectname' and use it for register_post_type and register_taxonomy
16600.diff (472 bytes) - added by greuben 3 years ago.

Download all attachments as: .zip

Change History (19)

jltallon3 years ago

Test case for "C-P-T case sensitivity" - 'DomainName' vs 'domainname'

greuben3 years ago

comment:1 greuben3 years ago

  • Keywords dev-feedback removed
  • Resolution set to worksforme
  • Status changed from new to closed

You need to register the taxonomies before the custom post type and also you must use 'taxonomies' in register_post_type arguments.

see the attached file.

comment:2 jltallon3 years ago

  • Resolution worksforme deleted
  • Severity changed from major to minor
  • Status changed from closed to reopened

Thanks.

The Codex does not clearly specify that it has to be done in this order, so this would be a documentation bug.
Moreover, the fact that doing it in the order I was doing it works, but only with all-lowercase names is at the very least surprising. WP should be case-preserving and case-respecting or be documented as "case
folding". The problem being with varying behaviour and/or lack of documentation, not with the internals as such.

comment:3 greuben3 years ago

  • Keywords needs-patch added

Okay my bad...the order doesn't matter. The taxonomies arg have done the trick.

This is a valid bug. In register_post_type the $post_type is sanitized but in register_taxonomy we are not sanitizing the $object_type array

comment:4 jltallon3 years ago

wp-includes/post.php, line 822:

$post_type = sanitize_user($post_type, true);

( whereas taxonomy.php, line 313 reads "$args [ ' object_type ' ] = (array) $object_type;" )

The problem being that sanitize_user, in strict mode, changes the intended "custom post type name".
Moreover, it looks like a C-P-T (or taxonomy, for that matter) name shouldn't contain the 'at' sign, right?

Hence, I propose to introduce a new sanitize_object_name function (not filter) for this matter.

(patch follows)

comment:5 jltallon3 years ago

Alternative b) just use "sanitize key" for register_taxonomy (just like it is done in post.php, line 916)

Unless there is a compelling reason to do so, I think WP should be case-preserving and case-sensitive for user-defined Post-Types and Taxonomies.

jltallon3 years ago

Implement 'sanitize_objectname' and use it for register_post_type and register_taxonomy

greuben3 years ago

comment:6 greuben3 years ago

  • Keywords dev-feedback added

Instead of creating a new function we can just array_map to sanitize_key.

comment:7 duck_3 years ago

See #14910 and #15982 as to why we cannot just replace with sanitize_key.

comment:8 nacin3 years ago

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

Let's start with a note added to the Codex page.

comment:9 jltallon3 years ago

Yes, the documentation piece is needed anyway.

Noted that sanitize_key is a bit too expensive. My "sanitize_objectname" could be made simpler.
Moreover, I did not use sanitize_key (in fact, I replaced its usage for register_object_type) since it wraps names to lowercase, and I wanted it to be case-preserving.

Call the function whatever you like, but (IMHO) we should allow [A-Za-z0-9_] (i.e. "identifier" as in most programming languages) for custom post types and custom taxonomies.

comment:10 ocean903 years ago

Duplicate: #16649

comment:11 mikeschinkel3 years ago

  • Cc mikeschinkel@… added

I'd love to see lowercase as a requirement for taxonomy and post type names. They are, after all, internal names and not named for display to the user. having them be lowercase means it's easier to ensure our code can sanitize them. FWIW.

comment:12 scribu3 years ago

+1 on case folding, for both post types and taxonomies.

comment:13 follow-up: c3mdigital9 months ago

  • Keywords close added; needs-codex removed
  • Resolution set to invalid
  • Status changed from reopened to closed

Fixed in #16649

comment:14 in reply to: ↑ 13 helen9 months ago

Replying to c3mdigital:

Fixed in #16649

#16649 was closed as a dupe of this one...

comment:15 ocean908 months ago

  • Keywords close removed
  • Resolution invalid deleted
  • Status changed from closed to reopened
Note: See TracTickets for help on using tickets.