[Boost] Fix minify paths on sites in a subdir. #30893

thingalon · 2023-05-24T09:05:02Z

Related: pedxs5-eK-p2#comment-114

When minifying and concatenating css on a site in a subdirectory, URLs weren't being properly interpreted. URLs like /wp-includes/* are relative to WordPress' home URL, and were being treated as relative to the root path of the domain.

#30863 went some way towards fixing that, but only fixed it for JS URLs. This PR updates the fix to apply to both JS and CSS URLs.

Proposed changes:

Ensure that enqueued script and css URLs are treated as relative to WordPress home

Other information:

Have you written new tests for your changes, if applicable?
Have you checked the E2E test CI results, and verified that your changes do not break them?
Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

n/a

Does this pull request change what data or activity we track or use?

no

Testing instructions:

Use a WordPress site on a subdirectory on any domain.
Ensure that CSS and JS are properly minfied and concatenated.

github-actions · 2023-05-24T09:06:20Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Boost plugin:

Next scheduled release: June 6, 2023.
Scheduled code freeze: May 29, 2023.

github-actions · 2023-05-24T09:06:20Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Boost plugin:

Next scheduled release: June 6, 2023.
Scheduled code freeze: May 29, 2023.

dilirity

Good catch! Thanks for fixing this. Code looks great.

brandonpayton · 2023-05-25T14:47:45Z

projects/plugins/boost/app/lib/minify/functions-helpers.php

+ * Note: this method uses home_url, so should only be used plugin-side when
+ * generating concatenated URLs.
+ */
+function enqueued_to_absolute_url( $url ) {


Hi @thingalon, it seems like it would be good to prefix this function name with jetpack_boost_ to reduce the possibility of naming collisions.

Good point, thank you. Fixed in e0b3f35.

🤦🏻 I missed that. I thought they were namespaced.

It's an understandable assumption. :) Namespaces seem common in Jetpack, but the team that created the bits from Page Optimize heavily favors function prefixing over dealing with namespaces.

brandonpayton

I haven't tested this directly, but these changes look reasonable to me.

* Properly take home_url into account to catch subdir paths * changelog * Prepend method with jetpack_boost_ --------- Co-authored-by: Mark George <thingalon@gmail.com>

thingalon · 2023-05-29T06:11:25Z

Cherry-picked to 1.9.0 in fb26b14

* Update readme and changelogs for Boost 1.9.0-beta * Updated package versions for automattic/jetpack-boost * [not verified] Boost/add minify to pricing (#30711) * Add minify and concat to pricing tables * changelog --------- Co-authored-by: Mark George <thingalon@gmail.com> * [not verified] Boost: Fix UI issues (#30751) * Fix showing exclude text when no value is empty * Fix bottom tooltip showing below footer on getting started page * Update tooltip position in pricing table component to be configurable * Update tooltip positions in left column on getting started page * Fix score text capitalization * Add changelogs * [not verified] Boost: Update Minify to use old Page Optimize file functions (#30829) * Fix concat initializing despite file system being unavailable * add changelog * Ignore file_* function usage for phpcs * Update changelog * Revert changes to minify setup helper * [not verified] Boost: Fix Minify concat not working when WP is running from a subdirectory on the server (#30863) * Fix issues with wp instances in subdirectory urls * Add changelog * Boost: Make Minify concat concat service independent of WordPress (#30825) * Attempt to remove WordPress dependencies from Minify service * add changelog * Fix concat initializing despite file system being unavailable * add changelog * Add missing cssmin library * Ignore file_* function usage for phpcs * Update changelog * Revert changes to minify setup helper * Attempt to remove WordPress dependencies from Minify service * Add changelog * Add missing cssmin library * Clean up old file system references * Fix issues with wp instances in subdirectory urls * Fix issues with wp instances in subdirectory urls * Add changelog * Remove comments * Improve code readability * Make linter happy * changelog --------- Co-authored-by: Mark George <thingalon@gmail.com> * [not verified] [Boost] Fix minify paths on sites in a subdir. (#30893) * Properly take home_url into account to catch subdir paths * changelog * Prepend method with jetpack_boost_ --------- Co-authored-by: Mark George <thingalon@gmail.com> * [not verified] [Boost] Remove invalid apply_filter on minified CSS (#31058) * Do not call style_loader_tag with an array of handles * changelog --------- Co-authored-by: Mark George <thingalon@gmail.com> * [not verified] [Boost] Avoid double-prefixing url() values in minified CSS (#31059) * Prevent double-prefixing subdirectories * changelog --------- Co-authored-by: Mark George <thingalon@gmail.com> * Boost/update readme (#31065) * Update screenshots and readme * changelog * Remove out-dated paragraph --------- Co-authored-by: Mark George <thingalon@gmail.com> * Update readme, changelogs * Update version numbers * fix changelog link * [not verified] Updated automattic/jetpack-boost version to 1.9.1 * [not verified] Updated package versions for automattic/jetpack-boost * [not verified] Update readme and changelog * Set stable tag to 1.9.0 * Fixup project versions * Fix changelog and project versions * Fix boost composer * Composer update * Composer update from within docker container for php 8.0 --------- Co-authored-by: thingalon <mark@thingalon.com> Co-authored-by: Mark George <thingalon@gmail.com> Co-authored-by: Peter Petrov <peter.petrov89@gmail.com>

Mark George added 2 commits May 24, 2023 18:57

Properly take home_url into account to catch subdir paths

b10a84f

changelog

c6e1a5c

thingalon added the [Status] Needs Team Review label May 24, 2023

thingalon requested review from brandonpayton, dilirity and a team May 24, 2023 09:05

thingalon self-assigned this May 24, 2023

thingalon marked this pull request as ready for review May 24, 2023 09:05

github-actions bot added [Plugin] Boost A feature to speed up the site and improve performance. [Status] In Progress labels May 24, 2023

dilirity previously approved these changes May 25, 2023

View reviewed changes

dilirity removed the [Status] In Progress label May 25, 2023

brandonpayton reviewed May 25, 2023

View reviewed changes

Prepend method with jetpack_boost_

e0b3f35

thingalon dismissed dilirity’s stale review via e0b3f35 May 26, 2023 00:02

brandonpayton reviewed May 26, 2023

View reviewed changes

dilirity approved these changes May 29, 2023

View reviewed changes

thingalon merged commit 8f638ce into trunk May 29, 2023

thingalon deleted the fix/minify-css-urls branch May 29, 2023 06:09

thingalon added the [Status] Needs Cherry-Pick label May 29, 2023

github-actions bot removed the [Status] Needs Team Review label May 29, 2023

anomiex removed the [Status] Needs Cherry-Pick label May 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Boost] Fix minify paths on sites in a subdir. #30893

[Boost] Fix minify paths on sites in a subdir. #30893

thingalon commented May 24, 2023

github-actions bot commented May 24, 2023

github-actions bot commented May 24, 2023 •

edited

Loading

dilirity left a comment

brandonpayton May 25, 2023

thingalon May 26, 2023

dilirity May 26, 2023 •

edited

Loading

brandonpayton May 26, 2023

brandonpayton left a comment

thingalon commented May 29, 2023

[Boost] Fix minify paths on sites in a subdir. #30893

[Boost] Fix minify paths on sites in a subdir. #30893

Conversation

thingalon commented May 24, 2023

Proposed changes:

Other information:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

github-actions bot commented May 24, 2023

github-actions bot commented May 24, 2023 • edited Loading

dilirity left a comment

Choose a reason for hiding this comment

brandonpayton May 25, 2023

Choose a reason for hiding this comment

thingalon May 26, 2023

Choose a reason for hiding this comment

dilirity May 26, 2023 • edited Loading

Choose a reason for hiding this comment

brandonpayton May 26, 2023

Choose a reason for hiding this comment

brandonpayton left a comment

Choose a reason for hiding this comment

thingalon commented May 29, 2023

github-actions bot commented May 24, 2023 •

edited

Loading

dilirity May 26, 2023 •

edited

Loading