wp_get_shortlink not working on pages

[mattsay]

wp_get_shortlink doesn't return anything on pages.

reason:

2196   if ( isset($post->post_type) && 'post' == $post->post_type )
2197         $shortlink = home_url('?p=' . $post->ID);

[Otto42]

Doesn't work on custom post types either.

[sillybean]

Generates p= shortlinks for all content types, not just posts.

[sillybean]

Updated patch removes some cruft and makes sure shortlinks are generated only for public post types.

[layotte]

Updated patch to remove the permalink_structure check. If the user does not have permalinks turned on, this function returns a blank string as the shortlink. It should be permalink agnostic... so plugins/themes can use it without worrying about permalink checks.

Thoughts?

[sillybean]

The latest patch is great.

[Otto42]

+1 to latest patch.

[mikecorkum]

+1 to patch

[enej]

+1 to this patch.

[ryan]

See also #18632

[ryan]

Refresh with unit tests

[nacin]

Replying to layotte:

Updated patch to remove the permalink_structure check. If the user does not have permalinks turned on, this function returns a blank string as the shortlink. It should be permalink agnostic... so plugins/themes can use it without worrying about permalink checks.

The reason why it returns a blank when the permalinks are off is because it isn't a valid shortlink — it's the same length as the existing link.

I see the argument that plugins/themes should be able to use it without checking the permalink structure (though really, if it returns nothing, then just call get_permalink() as a fallback — this already must occur since it was so limited to post type) — but the idea was to not show a "Get Shortlink" button if we weren't actually showing something shorter.

[Otto42]

Yeah, that's a pretty awful return value there. If the normal link is as short as it gets, then it should return the normal link. It's short too. Relying on plugins/themes to check for blank and call get_permalink is a bit silly.

I can see the use case for wanting to not show the button, however in that case then this function is badly named. The name "wp_get_shortlink" implies that it will return a shortlink. It doesn't imply that it will return something different based on external factors. If you want to check for that sort of thing, then that should be in something like "has_shortlink" or something.

[ryan]

Don't return shortlinks when cruft-free links aren't enabled

[ryan]

.2.diff is without the change of return value.

[ryan]

I'm leaning toward changing the behavior. Predictable is good.

[sillybean]

Otto beat me to it. Having the function return a blank is certainly not what most developers would expect. If there's no shorter link, it should just return the original link.

[nacin]

Replying to ryan:

I'm leaning toward changing the behavior. Predictable is good.

I'm fine with that but the button should be hidden in those cases.

[sillybean]

Do you mean the "Get Shortlink" button on the Edit Post screen? In order to hide that when no shorter link is available, wouldn't we have to check for any shortlink-related plugins?

[Otto42]

A shortlink plugin would hook into this and presumably return a different shortlink, so that works either way.

[sillybean]

Hide "Get Shortlink" button if no shorter link is available

[sillybean]

Something like this​, then? (14760-button.diff.)

[nacin]

As a plugin could return nothing from that filter (which is a valid return value, and will still be used for non-public post types), the check should probably be $shortlink && $shortlink !== get_permalink( $post->ID ).

[sillybean]

Updated the button patch. (I'm not integrating Ryan's because I'm having trouble checking out the unit tests.)

[ryan]

In 25030:

wp_get_shortlink() improvements.

• Return shortlinks for pages and public CPTs.
• Return shortlinks even when cruft-free links are not enabled.
• Unit tests

Props sillybean, layotte, cais
fixes #18632
see #14760

[ryan]

Since the permalink for page IDs used ?page_id= instead of ?p=, we'll need to add a case to the buttom patch.

[ryan]

Replying to ryan:

Since the permalink for page IDs uses ?page_id= instead of ?p=, we'll need to add a case to the buttom patch.

Unless we think that qualifies as a shortlink. Technically, it is shorter, but meh.

[sillybean]

Actually, I don't think we need a case. If there's no permalink structure set, we fail the '' != get_option('permalink_structure') test and wp_get_shortlink returns ?page_id=. The button doesn't show. If there is a permalink structure, the button shows. We get a ?p= shortlink instead of ?page_id=, but since it just redirects, I don't see that as a problem.

[ryan]

With permalinks turned off, I see the "Get Shortlink" button when editing a page. The permalink is page_id=2 and the shortlink is p=2.

[sillybean]

Add page_id= check to Get Permalink button conditional

[ryan]

In 25122:

Don't show the "Get Shortlink" button for pages with a ?page_id=x permalink.

Props sillybean
fixes #14760