WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 7 years ago

Last modified 7 years ago

#1485 closed enhancement (fixed)

Don't use trailingslashed IRIs if the user doesn't prefer them

Reported by: MathiasBynens Owned by: markjaquith
Milestone: 2.2 Priority: low
Severity: normal Version: 1.6
Component: Template Keywords: permalink permalinks iridesign bg|dev-feedback bg|2nd-opinion has-patch
Focuses: Cc:

Description

On wp-admin/options-permalink.php, users can set their own virtual permalink structure. They're free to choose whether to use a trailing slash for post permalinks or not, which is fine. However, WordPress is a bit inconsistent in this by default. Currently, category permalinks are _always_ generated with a trailing slash, regardless of the user's preference. The same goes for date, month, year, ... etc permalinks.
WordPress should assume the user doesn't like trailing slashes if get_settings('permalink_structure') doesn't have a trailing slash.

Attachments (1)

user-trail.diff (10.7 KB) - added by MathiasBynens 9 years ago.

Download all attachments as: .zip

Change History (21)

comment:1 MathiasBynens9 years ago

  • Component changed from Administration to Template

comment:2 westi9 years ago

Reading http://scott.yang.id.au/2005/05/permalink-redirect/ and http://photomatt.net/2005/05/30/one-true-permalink-plugin/ trailing slashes are the way to go and should probably be forced on by wordpress.

westi

MathiasBynens9 years ago

comment:3 MathiasBynens9 years ago

Westi, the fact that Matt talked about implementing Scott's plugin into the WP core actually made me do this. People who don't use trailing slashes are most likely to have some mod_rewrite rules like this:

RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule ^(.+)/$ http://mathibus.com/$1 [R=301,L]

Which means: "If the requested file is not a directory, and the request IRI ends with a trailing slash, redirect the UA to the same IRI without the trailing slash, send a 301 Moved Permanently HTTP header and stop the rewrite process."

This script would of course conflict with Scott's plugin, and a redirect loop would occur when for example a category page is visited.

Trailing slashes are definately not "the way to go"; I'm not saying they're bad or anything, I just point out that this is a user preference. Those links you mention don't claim trailing slashes should be used, all they're saying is that there should be only one permalink to each page of a WP blog.

I now added a patch which fixes this in WordPress.

comment:4 markjaquith9 years ago

  • Owner changed from anonymous to markjaquith
  • Status changed from new to assigned

Scott Yang's plugin seems to use get_permalink(), so it should enforce the permalink that WP returns.

If the user chooses a permalink structure with no trailing slash for their permalinks, I think it's a reasonable assumption that they don't want trailing slashes and we shouldn't be forcing them.

That's quite a patch you got there. I really like how you cleaned up code as you went... that's really helps keep the code consistent and easy to read.

Heh, this line (old) doesn't even make sense:

$tb_url = trailingslashit(get_permalink()) . 'trackback/';

The trailing slash is explicitly added so putting it through tralingslashit() is pointless.

The only issue I really have is a minor one. I really don't think the IRI term is ready to be used in the code. There is enough URI/URL divergence as it is, and heck, I didn't even know what an IRI was until about 2 months ago. Eventually, yeah, that might be where things go, but having three different acronyms for what is basically the same thing doesn't make sense. I say we stick with URI/URL and then move to IRI en masse, if that's where things are moving. Minor quibble though.

I'm passing this on for dev feedback, but I think it's a good idea.

comment:5 MathiasBynens9 years ago

"If the user chooses a permalink structure with no trailing slash for their permalinks, I think it's a reasonable assumption that they don't want trailing slashes and we shouldn't be forcing them."

Exactly!

I like it how you like it how I cleaned up the code a bit, too. :) The "senseless" code sample you provided makes sense though... You probably misread:

$tb_url = trailingslashit(get_permalink()) . 'trackback/';

This makes sense, because get_permalink wasn't always trailingslashed, depending on the user's permalink settings.

$tb_url = trailingslashit(get_permalink() . 'trackback/');

That would be totally useless. Oh well :)

As for the xRn thing: IRIs are a superset of URIs, and URIs are a superset of URLs. I decided to go for $iri because WordPress fully supports all kinds of IRIs -- why not show off a bit? ;) But I agree, that's just a minor thing.

comment:6 markjaquith9 years ago

Ah! You're right, I did misread it thusly. Let's wait for someone else (yoohoo, Skippy!) to weigh in on this, and then I'll clean up that patch a bit and see how the devs like it.

comment:7 markjaquith9 years ago

Okay... all silent on the 2nd opinion front. I'll try to get a cleaned up patch for 1.6 SVN made soon.

comment:8 ryan9 years ago

We could do the smart slashing in the get_*_permastruct() methods of the WP_Rewrite class. The places where we call get_*_permastruct() would assume the structures are already slashed according to user preference and would not call trailingslashit().

comment:9 MathiasBynens9 years ago

Yeah, whatever. As long as WP stops forcing trailing slashes on certain occasions, you won't hear me nagging :)

comment:10 MathiasBynens9 years ago

The places where we call get_*_permastruct() would assume the structures are already slashed according to user preference and would not call trailingslashit().

I'm not too sure about that. How about trackback and feed IRIs? If we apply the default WP IRI design, we have:

http://site.com/archives/yyyy/mm/dd/post-slug/trackback/
http://site.com/archives/yyyy/mm/dd/post-slug/feed/

If we'd modify the get_*_permastruct() methods, and if the above user does prefer no trailing slashes, get_permalink() would return http://site.com/archives/yyyy/mm/dd/post-slug, and for instance the trackback address would be generated as http://site.com/archives/yyyy/mm/dd/post-slugtrackback... Right?

In my opinion, this patch is okay; if not the best way to do it.

comment:11 skippy9 years ago

I like the idea of putting trailing slash (or not) into the get_*_permastruct() method. Feed permalinks can trivially check for the presence of a trailing slash at the end of the permalink with strrpos:

if (strrpos($permalink, "/") != strlen($permalink)) { ... }

comment:12 MathiasBynens9 years ago

Indeed, or of course just do a trailingslashit(get_permalink()) when a permalink is needed for trackback or feed IRIs to ensure that it's trailingslashed...

comment:13 Jaykul9 years ago

This isn't really a "user prefference" sort of option... don't forget the reason why HTTP servers append a trailing slash to URLs for directories: if you didn't and you navigated to http://server/path/dir and
clicked on a link {a href="page.html" } you would get
http://server/path/page.html ... the trailing slash on the directory http://server/path/dir/ ensures that you'll get what you really want which is http://server/path/dir/page.html

Point being, as specified in the RFC 1630 { http://www.ietf.org/rfc/rfc1630.txt }

      Note: Trailing slashes

   If a path of the context locator ends in slash, partial URIs are
   treated differently to the URI with the same path but without a
   trailing slash. The trailing slash indicates a void segment of the
   path.

The two things are not equivalent if the page that is served up has relative paths embedded in it.

See also rfc3986, Sections 6.2.2 through 6.2.4
( http://www.ietf.org/rfc/rfc3986.txt )

This is particularly important when using relative paths to images ... since if the relative path to the image works with the trailing slash, it won't work without it, and vice-versa.


Not that it actually matters, in this context, but my personal prefference would be to append a pretend filename "/index.html" and make the mod-rewrite work such that when I write a page "AboutMe" ... if I actually created the folder "/AboutMe/" ... I could put images in it which were used in the page, and the mod-rewriting would make it look like: /AboutMe/index.html ... of course, this would have to be done such that I could intermix non-wordpress pages with the pages generated from wordpress ... my personal solution would be to change the extension (eg: have wordpress serve .html and my personal files .htm, or have wordpress serve .php and my personal files be .html, or even a custom extension like wordpress serving index.wp, so all the usual extensions would be usable by me... )

Other people wrestling with this:

Sitemap url format ...
http://www.google.com/webmasters/sitemaps/docs/en/protocol.html#faq_url_format

See Apache's "Trailing Slash Problem" entry in the rewrite guide:
http://httpd.apache.org/docs/2.0/misc/rewriteguide.html#ToC2

comment:14 MathiasBynens9 years ago

This isn't really a "user prefference" [sic] sort of option...

Yes, it is. No offense, Jaykul, but you're totally missing the point; what you just wrote is completely off-topic and hasn't got anything to do with this matter. You're talking about actual directories on a server, which of course require a trailing slash when viewed through a browser. WordPress generates a virtual permalink structure (e.g. http://domain.ext/archive/yyyy/mm/dd/post-slug/ is not a real directory on the server, so it doesn't matter whether a trailing slash is used or not. And this is where the user decides.

comment:15 markjaquith8 years ago

I'd still like to see this get in... if someone can write an updated patch for WP 1.6, I'd recommend it for commit.

comment:16 matt8 years ago

  • Milestone changed from 1.6 to 2.1
  • Priority changed from normal to low
  • Severity changed from normal to enhancement

comment:17 rgovostes8 years ago

  • Keywords has-patch added

comment:18 matt7 years ago

  • Milestone changed from 2.1 to 2.2

comment:19 markjaquith7 years ago

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

(In [4886]) Consistent use or disuse of trailing slashes in URLs according to user preference. props MathiasBynens. fixes #1485

comment:20 markjaquith7 years ago

To test, choose a permalink structure with a trailing slash, like:

/%year%/%monthnum%/%day%/%postname%/

or

/index.php/%year%/%monthnum%/%day%/%postname%/

Then look around. All WP-generated URLs should having trailing slashes.

Now switch to:

/%year%/%monthnum%/%day%/%postname%

or

/%year%/%monthnum%/%day%/%postname%

Now all WP-generated URLs should have no trailing slash (exception: home URL)

It's very possible I missed one, so assume a non-trailing-slash structure and start hunting!

Note: See TracTickets for help on using tickets.