Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#33495 closed enhancement (fixed)

Shortlinks are no longer useful by default

Reported by: helen's profile helen Owned by: grvrulz's profile grvrulz
Milestone: 4.4 Priority: normal
Severity: normal Version:
Component: Permalinks Keywords: good-first-bug has-patch
Focuses: administration Cc:

Description

Shortlinks have a few limited uses, but one of the biggest ones (Twitter character limits) is no longer relevant, as all URLs are wrapped in their own shortener and thus always count as the same number of characters as any other URL.

Given that we are quite opinionated as a project about pretty, meaningful permalinks, it seems odd that we promote ugly shortlinks, and it's not a feature for the majority.

Attachments (3)

edit-form-advanced.diff (476 bytes) - added by grvrulz 9 years ago.
edit-form-advanced-v2.patch (1.2 KB) - added by grvrulz 9 years ago.
33495.diff (1.6 KB) - added by helen 9 years ago.

Download all attachments as: .zip

Change History (21)

#1 @helen
9 years ago

Technical thoughts:

The best approach to this is probably one of "hiding" as opposed to "removing". If we can be smart about detecting any changes to the shortlink and continuing to show the button/link in that case, that would be a good and user-friendly thing to do. People do use shortlinks for analytics and that type of thing still.

#2 @lchelak
9 years ago

It does seem conflicting that short links and pretty URLs should exist side by side. I think taking this button out will make the UI more lean and less confusing for newcomers who might be confused if they're unsure of the function given the similarity with social shorteners.

#3 @helen
9 years ago

  • Milestone changed from Awaiting Review to 4.4

Putting this in 4.4 to force us to think about it.

#4 @helen
9 years ago

  • Keywords good-first-bug needs-patch added

#5 follow-up: @ibenic
9 years ago

These could maybe done with 2 possible workarounds:

  • hide the shortlink button, enable it through the Settings > Permalinks or Settings > Writing
  • hide/show the shortlink button through the Screen Options

By doing that, users who really need the shortlink can easily enable it.

What do you think about this solutions?

#6 in reply to: ↑ 5 ; follow-up: @helen
9 years ago

Replying to ibenic:

Thanks for the suggestions! My strong preference here, which also aligns with our philosophy of "decisions not options", would be for there to be no core interface for toggling this on and off, and leave it to plugins to enable. What would be smart for core to do is detect if anything has hooked into places that can alter the shortlink and continue to show UI for getting the shortlink if that's the case. We do something along those lines for the welcome panel on the dashboard.

#7 in reply to: ↑ 6 @grvrulz
9 years ago

Replying to helen:

I agree with what you're saying. The button should be hidden unless a plugin hooks in it's own shortlink logic(like Jetpack does).

#8 @grvrulz
9 years ago

  • Keywords has-patch added; needs-patch removed

I just added a patch that checks if something is adding a filter to either 'pre_get_shortlink' or 'get_shortlink' before showing the button.

#9 @DrewAPicture
9 years ago

  • Owner set to grvrulz
  • Status changed from new to assigned

Setting ownership to mark the good-first-bug as "claimed".

#10 @DrewAPicture
9 years ago

  • Keywords dev-feedback added

#11 @helen
9 years ago

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

Hi @grvrulz, thanks for jumping in. A few things here:

  • Your patch fatals due to a missing parenthesis, please test your patches.
  • As it stands, your patch attempts to hide the entire permalink area, not just the button when editing a post.
  • Some people may still want the default shortlinks, which means they need a method to turn it on (via filter).
  • There are two functions hooked in by default - wp_shortlink_header() for a PHP header and wp_shortlink_wp_head() for an HTML link element. What happens if these are removed by default?

#12 @grvrulz
9 years ago

Hi @helen,

My bad, I uploaded the wrong patch. I have uploaded the correct patch now(tested on trunk). It only hides the 'Get Shortlink' button.

If someone wants to turn this on, they can use a filter like this

add_filter( 'get_shortlink', 'my_enable_shortlink' );
function my_enable_shortlink() {
  return true;
}

I'm still looking over the stuff in your last point.

Thanks

#13 @grvrulz
9 years ago

Hi @helen,

I looked over wp_shortlink_header() and wp_shortlink_wp_head(), and I think these can also be disabled in absence of a shortlink enabler. What are your thoughts on this?

This ticket was mentioned in Slack in #core by helen. View the logs.


9 years ago

#15 @helen
9 years ago

  • Keywords has-patch added; needs-patch removed

After some discussion in Slack, we're going to leave the headers for now.

@helen
9 years ago

#16 @helen
9 years ago

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

In 34556:

Shortlinks: Hide the Get Shortlink button by default.

Shortlinks had their day in the popular usage sun before all these services moved to their own shorteners and running your own custom one became a lot easier. Shortlinks are still useful in some contexts, such as analytics or when links need to be shared verbally or copied down by hand.

If any filters are hooked onto pre_get_shortlink or get_shortlink and produce a non-empty value (with an exception described below), the button will magically reappear. This allows any custom shortlinks to keep the button without hiccups.

If you're in need of the default shortlinks, the fastest way to reenable them is add_filter( 'pre_get_shortlink', '__return_false' ). Note that it must return false in order to continue on to the rest of wp_get_shortlink().

props grvrulz for the initial patch.
fixes #33495.

#17 follow-up: @helen
9 years ago

@grvrulz I went ahead and massaged the patch a bit for commit so we don't lose momentum here, and I thought I'd leave you some notes about it here. Essentially, the has_filter() checks just needed to wrap around the existing shortlink bits - no need to get the shortlink if we're not going to use it, and the HTML that was being appended will always be appended together.

#18 in reply to: ↑ 17 @grvrulz
9 years ago

Replying to helen:
Thank you.. This feels great :)

Note: See TracTickets for help on using tickets.