![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
#33920 closed defect (bug) (fixed)
`wp_old_slug_redirect` ignores rewrite endpoints
| Reported by: | swissspidy | Owned by: | pento |
|---|---|---|---|
| Priority: | normal | Milestone: | 4.4 |
| Component: | Query | Version: | 2.1 |
| Severity: | normal | Keywords: | has-patch |
| Cc: | Focuses: |
Description
For the oEmbed feature plugin we're adding an embed rewrite endpoint to posts, to have URLs like http://example.com/my-post/embed/
However, when the slug changes from my-post to new-post, wp_old_slug_redirect redirects to http://example.com/new-post/, ignoring the endpoint.
The function should check for such query vars / endpoints in the process.
Additionally, a filter for the resulting URL in this function would be helpful for developers.
Attachments (6)
Change History (22)
This ticket was mentioned in Slack in #core by swissspidy. View the logs.
11 years ago
This ticket was mentioned in Slack in #core by swissspidy. View the logs.
11 years ago
#4
@
11 years ago
- Keywords needs-unit-tests added
I like it.
While it's kind of hacky, we could add unit tests using the new old_slug_redirect_url filter - hook into the filter, verify that the URL is correct, then return false to short circuit before the wp_redirect().
#5
@
11 years ago
- Keywords needs-refresh added
The new patch adds unit tests using the method you suggested.
While testing I noticed that WordPress doesn't properly redirect feed URLs, even with the patch applied.
/old-slug/feed/ doesn't redirect to /new-slug/feed/ but instead shows a feed for a non-existent post.
This is because feeds and trackbacks are pretty much hardcoded and not added via add_rewrite_endpoint.
#7
@
11 years ago
Unit tests are passing when they're run by themselves, but not when they're run as part of the full suite.
3) Tests_Rewrite_OldSlugRedirect::test_old_slug_redirect is_404 should be true. is_single, is_singular should be false. Failed asserting that false is true. /srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:492 /srv/www/wordpress-develop/tests/phpunit/tests/rewrite/oldSlugRedirect.php:62 4) Tests_Rewrite_OldSlugRedirect::test_old_slug_redirect_endpoint is_404 should be true. is_single, is_singular should be false. Failed asserting that false is true. /srv/www/wordpress-develop/tests/phpunit/includes/testcase.php:492 /srv/www/wordpress-develop/tests/phpunit/tests/rewrite/oldSlugRedirect.php:80 5) Tests_Rewrite_OldSlugRedirect::test_old_slug_redirect_feed Failed asserting that null matches expected 'http://example.org/?p=1087/feed/'. /srv/www/wordpress-develop/tests/phpunit/tests/rewrite/oldSlugRedirect.php:99
#8
@
11 years ago
Thanks.
Fixed in 33920.4.diff by setting up everything in setUp / tearDown.
#9
@
11 years ago
33920.5.diff now properly respects custom endpoints with a different query var.
#10
@
11 years ago
I haven't tested it, what happens with attachment URLs?
Either way, it'd be nice to have a unit test for it.
#11
@
11 years ago
This doesn't change behaviour for attachments, as they already seem to work properly. Perhaps because of redirect_canonical.
In WP_Query name is set to the attachment slug, so wp_old_slug_redirect has no effect.
I added a test for it in 33920.6.diff anyway, and also tested this with paginated posts.
#14
@
11 years ago
@pento: What about the unit test from 33920.6.diff?
#15
@
11 years ago
They all look like they're there to me? (Changeset view doesn't show new files in the diff, only in the file list.)
I just added a patch that adds missing endpoints to the URL.