WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 2 years ago

Last modified 22 months ago

#14111 closed enhancement (fixed)

wp_unique_post_slug() doesn't have a filter

Reported by: Viper007Bond Owned by: westi
Milestone: 3.3 Priority: lowest
Severity: trivial Version:
Component: Permalinks Keywords: has-patch needs-testing westi-likes
Focuses: Cc:

Description

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

Attachments (2)

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

Download all attachments as: .zip

Change History (21)

comment:1 follow-up: Viper007Bond4 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 Viper007Bond4 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. :)

comment:3 nacin3 years ago

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

Future pending patch. Which makes sense to me.

ptahdunbar3 years ago

ptahdunbar3 years ago

comment:4 ptahdunbar3 years ago

  • 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: Viper007Bond3 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 ptahdunbar3 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.

comment:7 in reply to: ↑ description mikeschinkel3 years ago

  • 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.

comment:8 imadha3 years ago

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.

comment:9 Viper007Bond3 years ago

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

comment:10 westi3 years ago

  • 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.

comment:11 Viper007Bond3 years ago

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

Regardless, you can never have enough filters! :)

comment:12 westi3 years ago

  • Milestone changed from Future Release to 3.3

comment:13 westi3 years ago

  • 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: Vynce3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

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.

.

Vynce

.

Last edited 3 years ago by Vynce (previous) (diff)

comment:15 Vynce3 years ago

  • 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 westi3 years 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.

comment:17 Vynce3 years ago

"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.

comment:18 ryan2 years ago

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

comment:19 coffee2code22 months ago

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

Note: See TracTickets for help on using tickets.