Subscriptions: register filesystem path for inline-style support#47988
Open
dd32 wants to merge 1 commit intoAutomattic:trunkfrom
Open
Subscriptions: register filesystem path for inline-style support#47988dd32 wants to merge 1 commit intoAutomattic:trunkfrom
dd32 wants to merge 1 commit intoAutomattic:trunkfrom
Conversation
PR Automattic#44546 added `wp_style_add_data( 'jetpack-subscriptions', 'path', $path )` where `$path` is the URL returned by `Assets::get_file_url_for_environment()`. But `wp_maybe_inline_styles()` requires a filesystem path so it can read the file with `file_get_contents()`. Passing a URL triggers a `_doing_it_wrong` notice from core for the `jetpack-subscriptions` stylesheet. Mirror the pattern used by the Likes module (Automattic#46750): resolve the absolute filesystem path via `JETPACK__PLUGIN_DIR` and the `jetpack_should_use_minified_assets` filter.
github-actions
bot
added
[Feature] Subscriptions
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
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 🤖 Jetpack plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
github-actions
bot
added
the
[Status] Needs Author Reply
jeherve
added
Bug
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a regression introduced in #44546 (Jetpack 14.9).
Summary
Jetpack_Subscriptions_Widget::enqueue_style()registers thepathstyle data using the URL returned byAssets::get_file_url_for_environment(). However,wp_maybe_inline_styles()requires a filesystem path so it canfile_get_contents()the stylesheet. Passing a URL triggers a_doing_it_wrongnotice from core:Assets::get_file_url_for_environment()always returns a URL (it ends inplugins_url(...)), so it should not be used as the value for thepathstyle data.Proposed changes
JETPACK__PLUGIN_DIRto compute an absolute filesystem path for the stylesheet, gated by thejetpack_should_use_minified_assetsfilter.modules/likes.php.wp_style_add_data( ..., 'path', ... )changes.See also #44532 (the original performance report that #44546 was responding to).
Testing instructions
WP_DEBUGenabled._doing_it_wrongnotice forjetpack-subscriptionsappears in the debug log / on screen.wp_maybe_inline_styles().SCRIPT_DEBUGenabled (which causesjetpack_should_use_minified_assetsto return false) and confirm the unminifiedmodules/subscriptions/subscriptions.csspath resolves correctly.Does this pull request change what data or activity we track or use?
No.
🤖 Generated with Claude Code (Opus 4.6).