[wp-trac] [WordPress Trac] #19292: Not found errors due to sanitization in sanitize_title_with_dashes
WordPress Trac
wp-trac at lists.automattic.com
Tue Nov 22 06:27:24 UTC 2011
#19292: Not found errors due to sanitization in sanitize_title_with_dashes
--------------------------+------------------
Reporter: xknown | Owner:
Type: defect (bug) | Status: new
Priority: normal | Milestone: 3.3
Component: General | Version:
Severity: blocker | Resolution:
Keywords: has-patch |
--------------------------+------------------
Comment (by nacin):
Discussed this further with dd32.
The second (and worse) blocker here is as follows: Take a page (or any
'''hierarchical''' post type) that was created in 3.0~3.2 that has a
special character in the slug. (You can rig this to work without doing svn
switches by hacking sanitize_title_with_dashes() to prevent the code
inside the `'save' == $context` conditional from firing, then remove the
hack.)
At this point, you will be able to view the page under the bad URL. Now go
into the page and save it. The page now has a good, clean URL. But the bad
URL no longer works.
To fix this, one of the following needs to occur: 1) a hack to prevent
sanitization from happening to hierarchical post types that are being
updated, or 2) we need to implement _wp_old_slug that works for pages.
(2) Is actually not terrible to do as long as we don't care about parent
changes (the focus of #4328), and focus squarely on slug changes. We can
then introduce _wp_old_path to properly handle actual path changes in 3.4.
On the other hand, I don't know what the heck would happen to a child
page's original link when a parent page is changed. Yeah, so that won't
really work.
(1) is doable, but kind of messy. Let me explain:
At first, I recommended to dd32 that sanitize_title() could be hacked to
allow an integer for $context, and in that case, $context could then be
assumed to be a post ID, and then $context would be set to 'save' only if
the post type was not hierarchical.
On one hand, this is backwards compatible. ($context was introduced
specifically for 'save' versus 'query' in 3.1.) On the other hand, this is
really low level and totally lame.
'''The better way to do this''' — still hacky, but I'm warming up to it —
is to modify every call to sanitize_title() that affects $post_name
generation. This happens in only a few instances — three, and all in
wp_insert_post(). (Also, these would need to be modified anyway to pass
post IDs, as proposed and rejected above.)
The modification is simple: If you're running and update AND it's a
hierarchical post type, then don't run the sanitization on save context.
This means that our feature will work for all new content, and old content
that has _wp_old_slug. [attachment:19292.diff] reflects this.
dd32 cooked up a slightly different patch that results in it only working
for new content (or old content that is sanitized identically to new
content), rather than falling back onto canonical redirections via
_wp_old_slug. While conservative, our original intent of the rolling
upgrade was that it would simply work, so I think I would rather go with
the route I've described.
--
Ticket URL: <http://core.trac.wordpress.org/ticket/19292#comment:5>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software
More information about the wp-trac
mailing list