-
Notifications
You must be signed in to change notification settings - Fork 798
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
Fix "View Full Site" and "View Mobile Site" for WordPress Subdirectory Installations #12414
Conversation
Thank you for the great PR description! When this PR is ready for review, please apply the Scheduled Jetpack release: June 4, 2019. |
This matches the formatting of surrounding code.
@BoleynSu Do you want to give this a try, see if it works for you? |
I am sorry I do not have a subdirectory installation of wordpress, so I
cannot test it. If it is possible, I can test it later when I get
some time. However, I am not sure when it will be.
…On Tue, May 21, 2019, 01:17 Jeremy Herve ***@***.***> wrote:
@BoleynSu <https://github.com/BoleynSu> Do you want to give this a try,
see if it works for you?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12414?email_source=notifications&email_token=AAXVSKEZSVJ5UW55T4QIURLPWLMJHA5CNFSM4HN43SZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVZP5CQ#issuecomment-494075530>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAXVSKA6Z2VBAXBKKWKOS7TPWLMJHANCNFSM4HN43SZQ>
.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution.
This works well. I would only recommend escaping as late possible, so you could move esc_url
and use it right as the anchor is being built / the link being outputted. This is a small detail right now, but could be important in the future if other things are added to the functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. 👍
* Kick off the changelog * Add 7.3.1 * Update date and post link * changelog: add #12219 * changelog: add #12170 * changelog: add #12184 * Changelog: add #12268 * Changelog: add #12081 * Changelog: add #12323 * Changelog: add #12204 * Changelog: add #12269 * Changelog: add #12332 * changelog: add #12339 * changelog: add #12209 * Changelog: add #12319 * Changelog: add #12357 * Changelog: add #12124 * Changelog: add #12373 * Changelog: add #12252 * Changelog: add #12383 * Changelog: add #12372 * changelog: add #12337 * Changelog: add #12290 * Changelog: add #12301 * Changelog: add #12061 * Testing list: add instructions for #12061 * Changelog: add #12393 * Update minimum supported version See #12287 * Changelog: add #12406 * Testing list: add #12406 * Changelog: add #12277 * Changelog: add #12412 * Changelog: add #11318 * Changelog: add #12328 * Changelog: add #12425 * Changelog: add #12380 * Changelog: add #12428 * Changelog: add #12414 * Changelog: add #12395 * Changelog & Testing list: add #12416, #12417, #12418, and #12348 * changelog: add #12379 * Changelog: add #12341 * changelog: add #12444 * Changelog: add #12434 * Changelog: add #12454 * Changelog: add #12460 * Changelog: add #12463 * Changelog: add #12457 * Changelog / testing list: add #10333 * Changelog: add #12467 Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>
When WordPress is installed to a subdirectory, the "View Full Site" and "View Mobile Site" links do not work properly. Incidentally, the problem is masked when on a blog post page, but present on the home page.
For example, suppose a blog is hosted at
https://somesite.com/blog
, with this URL set for bothWordPress Address
andSite Address
in the Wordpress settings (i.e., the blog is intended to be accessed at the subdirectory). Under this scenario, the "View Full Site" link expands tohttps://somesite.com/blog/blog/?ak_action=reject_mobile
, which has an extrablog/
. It should expand tohttps://somesite.com/blog/?ak_action=reject_mobile
. The double occurrence ofblog
is due to the omitted$url
argument toadd_query_arg
combined with the call tohome_url
. The call toadd_query_arg
returns/blog/?ak_action=reject_mobile
and that is passed tohome_url
which returnshttps://somesite.com/blog/blog/?ak_action=reject_mobile
.The same thing happens for the "View Mobile Site" link.
The problem occurs on the blog's homepage, and results in a page not found. The same issue occurs on blog posts, where
blog/
is added to the URL twice, but it appears that Wordpress is robust to such malformed URLs.Changes proposed in this Pull Request:
Is this a new feature or does it add/remove features to an existing part of Jetpack?
Testing instructions:
Proposed changelog entry for your changes: