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

Fix relative URLs for inlined stylesheets #1752

Closed
wants to merge 11 commits into from

Conversation

aristath
Copy link
Member

When styles get inlined, relative URLs break.
The problem is that URLs inside CSS files are relative to the stylesheet's path, and when styles get inlined that relation is lost.
This PR fixes the issue by finding relative URLs which then get modified to be relative to the site's root.

Trac ticket: https://core.trac.wordpress.org/ticket/54243


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@gziolo
Copy link
Member

gziolo commented Nov 4, 2021

It works like a charm. I tested with the block tutorial from the Block Editor Handbook by running:

npx @wordpress/create-block gutenpride --template @wordpress/create-block-tutorial-template

Styles from the plugin:
Screen Shot 2021-11-04 at 17 22 27

They get correctly adjusted when inlined on frontend:
Screen Shot 2021-11-04 at 17 22 03

Co-authored-by: Tonya Mork <tonya.mork@automattic.com>
@gziolo
Copy link
Member

gziolo commented Nov 8, 2021

Changes landed with https://core.trac.wordpress.org/changeset/52036.

* @return string The CSS with URLs made relative to the WordPress installation.
*/
function _wp_normalize_relative_css_links( $css, $stylesheet_url ) {
$has_src_results = preg_match_all( '#url\s*\(\s*[\'"]?\s*([^\'"\)]+)#', $css, $src_results );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious: is there any reason why preg_replace_callback() wasn't used here? If it was, then it could eliminate the last str_replace() in the foreach loop. I've just opened a ticket and patch which shows the performance of this function can be improved >2x: https://core.trac.wordpress.org/ticket/58069

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR to make this change: #4297

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no particular reason... I think at the time, it just didn't cross my mind to use preg_replace_callback() 👍

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