Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build home_url() previews with trailing slash #357

Merged
merged 1 commit into from
May 5, 2018
Merged

Build home_url() previews with trailing slash #357

merged 1 commit into from
May 5, 2018

Conversation

jeremyfelt
Copy link
Contributor

home_url() will only add a trailing slash to a site's home URL if a path is passed as an argument.

If no path is passed, a page preview could be something like: http://wp.wsu.dev?page_id=1

And it would work fine in most cases.

In a subdirectory multisite configuration, a page preview could be something like: http://wp.wsu.dev/site?page_id=1

This could be processed by WordPress as a lookup for a page named "site" rather than a site with that URL because no trailing slash exists.

When adding query args to home_url(), we should tell it to use a slash to avoid confusion.

See _get_page_link() for how core builds these in a way where a slash is always added.

See also #344, which I believe this fixes.

`home_url()` will only add a trailing slash to a site's home URL
if a path is passed as an argument.

If no path is passed, a page preview could be something like:

http://wp.wsu.dev?page_id=1

And it would work fine in most cases.

In a subdirectory multisite configuration, a page preview could be
something like:

http://wp.wsu.dev/site?page_id=1

This could be processed by WordPress as a lookup for a page named
"site" rather than a site with that URL because no trailing slash
exists.

When adding query args to `home_url()`, we should tell it to use
a slash to avoid confusion.

See `_get_page_link()` for how core builds these in a way where a
slash is always added.
jeremyfelt added a commit to washingtonstateuniversity/WSUWP-Build-Plugins-Public that referenced this pull request Jul 27, 2016
@cojennin
Copy link
Contributor

Awesome! Makes sense. Can you add a test to make sure this function returns the preview link correctly for both single and multisite installs? Tests should be passing again with this most recent commit.

@cojennin
Copy link
Contributor

cojennin commented Aug 8, 2016

Just following up. Think you'll have some time to write a test for this patch? Would definitely be great to see one for fix_preview_link.

edit: clarification

@jeremyfelt
Copy link
Contributor Author

Yep, I spaced on this a couple times. I'll have some time to add tests soon. :)

@cojennin
Copy link
Contributor

@jeremyfelt sounds good!

@cojennin
Copy link
Contributor

Just circling back on this. Think you'll have time to wrap it up with a small test?

@rinatkhaziev rinatkhaziev added this to the 0.8.3 milestone Apr 20, 2018
@rinatkhaziev
Copy link
Contributor

Marked for 0.8.3 release

@rinatkhaziev rinatkhaziev merged commit 3be71d6 into Automattic:master May 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants