Opened 3 years ago

Closed 19 months ago

Last modified 11 months ago

#14111 closed enhancement (fixed)

wp_unique_post_slug() doesn't have a filter

Reported by: Viper007Bond Owned by: westi
Priority: lowest Milestone: 3.3
Component: Permalinks Version:
Severity: trivial Keywords: has-patch needs-testing westi-likes
Cc: trac@…, mikeschinkel@…, Vynce

Description

I'm smarter than WordPress, I want to set my own slug. :)

Attachments (2)

ticket.14111.diff (387 bytes) - added by ptahdunbar 2 years ago.
ticket.14111.2.diff (613 bytes) - added by ptahdunbar 2 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 follow-up: ↓ 2   Viper007Bond3 years ago

I'm currently using a wp_insert_post_data filter to re-set the slug to what I want it to be (a number) and it works fine, but get_sample_permalink() runs the slug through wp_unique_post_slug() which makes the URL displayed below the title when editing the page wrong (it gains it's "-2" back).

comment:2 in reply to: ↑ 1   Viper007Bond3 years ago

  • Priority changed from normal to lowest
  • Severity changed from normal to trivial

Replying to Viper007Bond:

get_sample_permalink() runs the slug through wp_unique_post_slug() which makes the URL displayed below the title when editing the page wrong (it gains it's "-2" back).

The editable_slug filter solves that by the way, but it'd still be better/easier if wp_unique_post_slug() just had it's own filter. :)

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

Future pending patch. Which makes sense to me.

  • Cc trac@… added
  • Keywords has-patch reporter-feedback added; needs-patch removed

ticket.14111.diff adds a new filter "wp_unique_post_slug_{$post_type}" at the end of wp_unique_post_slug() passing $post_ID, $post_status, $post_type, $post_parent as parameters.

Taking another look at this, if you want control over the slug you probably want to skip the queries as your probably going to run your own queries. ticket.14111.2.diff skips the queries.

if you could clarify your approach, that would be great.

comment:5 follow-up: ↓ 6   Viper007Bond2 years ago

This is the code in question that currently hacks around the issue: http://wordpress.org/extend/plugins/allow-numeric-stubs/

comment:6 in reply to: ↑ 5   ptahdunbar2 years ago

  • Keywords needs-testing 2nd-opinion added; reporter-feedback removed

Replying to Viper007Bond:

This is the code in question that currently hacks around the issue: http://wordpress.org/extend/plugins/allow-numeric-stubs/

I would say that the second patch would be best then. I'm going to tweak your plugin to test it out using the new filter.

  • Cc mikeschinkel@… added

Replying to Viper007Bond:

I'm smarter than WordPress, I want to set my own slug. :)

+1. Have had lots of issue recently that this filter would solve.

Bump.

A filter here would be immensely helpful. At this point, I'm just wanting to use the underscore character instead of a dash to delimit spaces and having to jump through several hoops for such a simple change.

Off-list I have informed imadha of the sanitize_title filter. :)

  • Keywords westi-likes added; 2nd-opinion removed
  • Owner set to westi
  • Status changed from new to accepted

We should have a filter at the end of the function to compliment the ones in the function so that if you want to do something to the resultant slug or replace the behaviour easily you can.

Turns out imadha wants to change -2 to _2 so I was wrong.

Regardless, you can never have enough filters! :)

  • Milestone changed from Future Release to 3.3
  • Resolution set to fixed
  • Status changed from accepted to closed

In [18546]:

Add a general filter to wp_unique_post_slug to allow for full customisation of the uniqueness functionality. Fixes #14111.

comment:14 follow-up: ↓ 16   Vynce21 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Summary: i would recommend instead inserting this at line 2835:
$alt_post_name = substr( $slug, 0, 200 – ( strlen( $suffix ) + 1 ) ) . “-$suffix”;

Reasoning: the way [18546] was implemented the uniqueness does not appear to take
into account the filter. for instance, assume I have a filter that
converts that – to a _.

  1. first, i create post ‘counting’
  2. then i try to create ‘counting’ and that’s not unique, so I go into the loop and get ‘counting-2′, which is unique, so i exit the loop with counting-2, which i filter to ‘counting_2′.
  3. then i try to create ‘counting’ and that’s not unique, so i go into the loop and get ‘counting-2′, which is unique, so i exit the loop with counting-2, which i filter to ‘counting_2′ — which isn’t unique.
  4. chaos

If, on the other hand, the filter were on line 2835, then I believe
it would actually unique the value that was going to be used, instead
of something else.

.

Vynce

.

Version 0, edited 21 months ago by Vynce (next)
  • Cc Vynce added

on further reflection, the idiom used seems a bit fragile:

if ($some_condition || $other_condition) { 
    do { 
        fix_condition(); 
    } while ($some_condition)
} 

What happens -- even using the default, current uniquing -- if there's a feed that happens to be named 'common-slug-2'? wouldn't it be more straightforward and safer to change that to

	$suffix = 2;
	while ( $post_name_check || in_array( $slug, $feeds ) || apply_filters( 'wp_unique_post_slug_is_bad_attachment_slug', false, $slug ) ) {
		$alt_post_name = substr ($slug, 0, 200 - ( strlen( $suffix ) + 1 ) ) . "-$suffix";
		$post_name_check = $wpdb->get_var( $wpdb->prepare($check_sql, $alt_post_name, $post_ID ) );
		$suffix++;
		$slug = $alt_post_name;
	}

?

comment:16 in reply to: ↑ 14   westi20 months ago

Replying to Vynce:

Summary: i would recommend instead inserting the hook cloaser to line 2835:

$alt_post_name = substr( $slug, 0, 200 – ( strlen( $suffix ) + 1 ) ) . “-$suffix”;

Reasoning: the way [18546] was implemented the uniqueness does not appear to take
into account the filter. for instance, assume I have a filter that
converts that – to a _.

  1. first, i create post ‘counting’
  2. then i try to create ‘counting’ and that’s not unique, so I go into the loop and get ‘counting-2′, which is unique, so i exit the loop with counting-2, which i filter to ‘counting_2′.
  3. then i try to create ‘counting’ and that’s not unique, so i go into the loop and get ‘counting-2′, which is unique, so i exit the loop with counting-2, which i filter to ‘counting_2′ — which isn’t unique.
  4. chaos

If, on the other hand, the filter were on line 2835, then I believe
it would actually unique the value that was going to be used, instead
of something else.

The idea behind the new hook is to give you full control over the slug - this means that if you want to implement a different slug format then you will need to do the uniqueness checking yourself.

This is the normal way WordPress provides for plugins to override behaviour.

It is more efficient to implement it this way than to run a filter call for every slug variant that WordPress would test.

However, I am actually considering moving this new filter to the top of the function so that the work that WordPress is doing is not wasted when the filter is used.

"Full control" -- including the option to not make them unique? that sounds fine, except that there's other stuff builtin that assumes they will be unique; letting the user shoot themselves in the foot that way seems like asking for trouble. But, if the idea is that the responsibility is on the user, that's fine; I just didn't understand that to be the case, since you do go to all the trouble to unique it.

  • Resolution set to fixed
  • Status changed from reopened to closed

See #20480 for follow-up to fix omission of sending original slug to the filter.

Note: See TracTickets for help on using tickets.