Support for subdirectory multisites #28
Conversation
To allow us to work with the css URL directly if needed
Incluse util function to test if two url matches a specified site url
To support siteurls with subdirectories
|
Thanks for working on this @mjangda ! When testing the patch in VIP Quickstart, I have noticed that the For instance : I'm still digging into this and will look into possible solutions, but wanted to flag that here in order to share the findings. |
Some scripts, eg /wp-includes/css/admin-bar.min.css, are being enqueued w/o the protocol and host, and as relative, they should be evaluated as internal ones. This commit adds check agains `'host'` in parsed tested URL in order to figure out whether the URL is or is not relative and does not let the `WPCOM_Concat_Utils::is_internal_url` method retuning `false` for those URLs.
tests/test-concat-utils.php
Outdated
| 'subdir_site__internal_absolute_url' => array( | ||
| '/wp-content/plugins/plugin/style.css', | ||
| 'https://example.com/mysite', | ||
| false, |
There was a problem hiding this comment.
@mjangda why we should be evaluating /wp-content/plugins/plugin/style.css as external in subdirectory site case?
There was a problem hiding this comment.
I think the use case I was thinking of was WordPress being installed a subdirectory. Probably just a mistake though :)
On subdirectory multisite install, the style's path begins with blog's path, eg: /mysite/, and this is breaking the `realpath` check which is being performed. This commit checks whether the current instance is a subdirectory multisite, and whether the path is set to something other than `Ã'/'. If so, it's being trimmed from the path before performing the realpath check.
And drop PHP5.3
The `get_blog_details` function only exists in multisite installation, which leads to PHP fatal errors on single site. This commit is addressing the issue by moving the `get_blog_details` to inside `is_multisite` conditional
…ct realpath on both, standard installations and subbir MS installations The body of the method is a copy of the code which has been added in previous commits. Since we need the same code on multiple places, it's better if we abstract it this way.
…OM_Concat_Utils::realpath method call
When the `$js_url` war replaced by `$js_url_parsed`, I forgot to create the `$js_url` variable, which is being used later in the code. This commit fixes JS concatenation on standard as well as on MS subdir installs.
This commit is taking advantage of WPCOM_Concat_Utils::realpath method for figuring out the last modified date
/wp-content/plugins/plugin/style.css on subdirectory install should be evaluated as internal URL. Discussion: #28 (comment)
|
Note: the tests are failing "just" because of PHPCS, which is not working properly. |
|
@mjangda I'm done with this PR. I have successfully tested the updates on 1) a single site installation 2) MS domain setup 3) MS directory setup. I have used following scenario for my tests:
The diff was clean (JS and CSS wise) in all cases mentioned above. Could you, please, have a look at the changes I have done and eventually proceed with merging this PR? |
|
Haven't tested it yet, but the code looks solid. Will run through on my test site and follow-up. |
Since the original `style_loader_tag` filters against a _single_ handle, this can cause problems when the concatenated file contains multiple handles worth of CSS. This produced a bug where the `Jetpack::concat_remove_style_loader_tag()` filter would remove an _entire_ concatenated file's worth of CSS just because it contained one handle. The new filter will allow the concatenated tag to still be adjusted if necessary, but it's not at the mercy of one handle removing all of the file.
|
@mjangda I have nerfed the PHPCS checks for good and tests are now passing. How are the changes looking on your test site? |
If this is a subdirectory site, we need to strip off the subdir from the URL before trying to determine its realpath. This is because in a multisite install, the subdir is virtual and therefore not needed in the path(For `example.com/subdir/file.css`, `file.css` is actually served from `/`). In a single-site subdir install, the subdir is included in the ABSPATH and therefore ends up duplicated (ABSPATH: /var/www/wp | URL: example.com/wp/file.css).
mjangda
changed the title
|
Tested across a bunch of different configurations and looks pretty good. The only issue I spotted is that concat was broken for single site subdirectory installs (i.e. site installed and served from Pushed a fix for this 8396984 and re-tested and seems to be working well. If you agree, I can look at merging/deploying sometime today (Friday). |
|
@mjangda the patch looks good and it tests out during my testing. Ship it! |
|
💥 |
Properly handle subdirectory sites on a multisite install. The original code for this plugin was written for WordPress.com which only supports domains and has many related assumptions that fail when running in a subdirectory (either as part of a multisite or on its own).