Skip to content

Sitemaps: Support PATHINFO permalinks#4093

Merged
jeherve merged 2 commits intomasterfrom
add/pathinfo-sitemap
Jun 16, 2016
Merged

Sitemaps: Support PATHINFO permalinks#4093
jeherve merged 2 commits intomasterfrom
add/pathinfo-sitemap

Conversation

@kraftbj
Copy link
Copy Markdown
Contributor

@kraftbj kraftbj commented Jun 8, 2016

This PR adds the other possibility for WP permalinks: PATHINFO via index.php.

To test:

  1. Set your permalink structure to a custom one beginning with /index.php/
  2. Verify /index.php/sitemap.xml loads as does the stylesheet. Repeat for the news sitemap.
  3. Set permalink to a normal pretty one, repeat 2 at sitemap.xml.
  4. Set to default permalinks ?p=123, repeat 2 at /?jetpack-sitemap=true

Verify all load.

@kraftbj kraftbj added Bug When a feature is broken and / or not performing as intended [Status] Needs Review This PR is ready for review. [Feature] Sitemaps labels Jun 8, 2016
@kraftbj kraftbj added this to the 4.1 milestone Jun 8, 2016
@kraftbj kraftbj self-assigned this Jun 8, 2016
@kraftbj kraftbj force-pushed the add/pathinfo-sitemap branch from 5ac630e to eb9d33f Compare June 8, 2016 17:11
@zinigor
Copy link
Copy Markdown
Contributor

zinigor commented Jun 8, 2016

Maybe I'm doing something wrong, but I was able to get a sitemap after setting permalinks to start with /index.php/ on master.

@kraftbj
Copy link
Copy Markdown
Contributor Author

kraftbj commented Jun 8, 2016 •
edited
Loading

Remove everything from .htaccess. This would typically be done on servers without mod_rewrite and thus .htaccess rules not working.

Should have mentioned that earlier up. Sorry about that!

@zinigor
Copy link
Copy Markdown
Contributor

zinigor commented Jun 8, 2016

Ah, OK, it looks like I have no way to test it because I use nginx on my server - the path logic is different there.

@kraftbj
Copy link
Copy Markdown
Contributor Author

kraftbj commented Jun 8, 2016

The alternative way to test -- confirm in the sitemap source that the stylesheet is loading using index.php in the permalink and that the module description (formerly the module card) gives the URL to the index.php variant of the sitemap.

@zinigor zinigor added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Jun 9, 2016
@zinigor
Copy link
Copy Markdown
Contributor

zinigor commented Jun 9, 2016

OK, I can confirm that the stylesheet is loaded using the proper URL now, thanks!

@jeherve
Copy link
Copy Markdown
Member

jeherve commented Jun 16, 2016

Works well in my tests as well. Merging!

@jeherve jeherve merged commit 8677a58 into master Jun 16, 2016
@jeherve jeherve removed the [Status] Ready to Merge Go ahead, you can push that green button! label Jun 16, 2016
@jeherve jeherve deleted the add/pathinfo-sitemap branch June 16, 2016 13:23
jeherve added a commit that referenced this pull request Jun 20, 2016
@jeherve
Release Notes: add #4093
c79d456
jeherve added a commit that referenced this pull request Jun 22, 2016
@jeherve
Release Notes: add #4093
d0d096e
@nbloomf nbloomf mentioned this pull request Nov 19, 2016
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@kraftbj kraftbj

Labels

Bug When a feature is broken and / or not performing as intended [Feature] Sitemaps

Projects

None yet

Milestone

4.1

Development

Successfully merging this pull request may close these issues.

3 participants

@kraftbj @zinigor @jeherve