Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 22 months ago

#54243 closed defect (bug) (fixed)

Inlined block styles that involve external assets (images/fonts) and relative URL's not working as expected

Reported by: cdyerkes's profile cdyerkes Owned by: gziolo's profile gziolo
Milestone: 5.9 Priority: normal
Severity: normal Version: 5.9
Component: Editor Keywords: has-patch has-testing-info
Focuses: javascript, css Cc:

Description

When using the tool npx @wordpress/create-block to create a custom block that references assets like images or fonts within scss files used in the build process (that generate style-index.css referenced in block.json that is rendered on the front and backend), the assets are inlined on the front-end with relative URL's which results in a 404 not found error for the referenced asset, rather than a direct url to the file using the plugin's filepath.

The problem still occurs for me using default WordPress themes and no other plugins aside from the custom block created following the @wordpress/create-block official tutorial https://developer.wordpress.org/block-editor/handbook/tutorials/create-block/.

Steps to reproduce:

  1. In a local dev environment (like Local), create a block: npx @wordpress/create-block wp-test-block in the plugins directory.
  2. Copy any external asset you want (like an image) into the block plugin's src directory.
  3. In the file located here src/style.scss add some lines to the prebuilt block CSS class that references the image or other asset you copied into the src directory like this (and save it):
.wp-block-wp-test-block {
    background-color: #21759b;
    color: #fff;
    padding: 2px;
    background-image: url(image-asset.png);
    background-repeat: repeat;
}
  1. Run npm run build on the block folder to run the build process and then activate the plugin in your local environment within the WP Dashboard.
  2. Go to a page in your local site and add your new block and update the page.
  3. View the page on the front-end and open your dev tools. You will see an error in the console marking the image asset as 404 not found.
  4. Inspect the page's HTML code in the inspector and you will see the block's CSS was inlined in the page like so:
<style id='wp-block-wp-test-block-style-inline-css'>
.wp-block-wp-test-block{background-color:#21759b;background-image:url(images/image-asset.f0104819.png);background-repeat:repeat;color:#fff;padding:2px}
</style>

The background-image: url(image-asset.png); has been inlined using a relative URL that would be equivalent to https://localhost.site/sample-page/images/image-asset.f0104819.png.

What I would expect to happen is for the image asset to use the plugin URL automatically in the build process so the 404 error does not happen. It should have been:

<style id='wp-block-wp-test-block-style-inline-css'>
.wp-block-wp-test-block{background-color:#21759b;background-image:url(/wp-content/plugins/wp-test-block/build/images/image-asset.f0104819.png);background-repeat:repeat;color:#fff;padding:2px}
</style>

Or allow us to load a stylesheet (like a <link rel="stylesheet" href="https://domain.com/wp-content/plugins/wp-test-block/build/style.css"> with the proper plugin filepaths via block.json used in the front-end rather than have to use a PHP function to enqueue block assets.

This is my first trac bug submission, so apologies if something isn't clear or correct.

Attachments (1)

wp-bug-screenshot.png (1.2 MB) - added by cdyerkes 3 years ago.
Shows the errors on my local dev site I have been using.

Download all attachments as: .zip

Change History (18)

@cdyerkes
3 years ago

Shows the errors on my local dev site I have been using.

#1 @gziolo
3 years ago

I can confirm the issue. @aristath, how should we tackle this issue? Any takes on that?

This ticket was mentioned in PR #1752 on WordPress/wordpress-develop by aristath.


3 years ago
#2

  • Keywords has-patch added

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

#3 @aristath
3 years ago

Added a patch that fixes the issue for relative URLs. The patch also has a PR in Gutenberg to backport the changes there as well: https://github.com/WordPress/gutenberg/pull/35538

Last edited 3 years ago by aristath (previous) (diff)

#4 @SergeyBiryukov
3 years ago

  • Milestone changed from Awaiting Review to 5.8.2

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


3 years ago

#7 @Boniu91
3 years ago

  • Keywords needs-testing has-testing-info added

#8 @desrosj
3 years ago

  • Milestone changed from 5.8.2 to 5.9

I'd like to see this approach get tested and confirmed in the plugin before being merged into Core. It's also preferable to avoid introducing a new function in a minor when possible.

Since 5.8.2 RC is due out in about an hour, I'm going to punt this one to 5.9.

gziolo commented on PR #1752:


3 years ago
#9

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:
https://i0.wp.com/user-images.githubusercontent.com/699132/140378683-d4a5069c-5841-469b-9bd7-9a331e72b585.png

They get correctly adjusted when inlined on frontend:
https://i0.wp.com/user-images.githubusercontent.com/699132/140378787-c28904ba-5aee-42f8-a8d1-c66a0a982b2e.png

#10 @gziolo
3 years ago

  • Keywords needs-testing removed
  • Version changed from 5.8.1 to trunk

@desrosj, I tested it against trunk. Do you think it's fine to land it for WP 5.9 first and decide about 5.8.x later?

The related bug fix in the Gutenberg plugin is also ready to go: https://github.com/WordPress/gutenberg/pull/35538.

Last edited 3 years ago by gziolo (previous) (diff)

#11 @gziolo
3 years ago

  • Owner set to gziolo
  • Resolution set to fixed
  • Status changed from new to closed

In 52036:

Fix relative URLs in inlined block styles that involve external assets

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

Fixes #54243.
Props aristath, cdyerkes, hellofromtonya.

#13 @ocean90
3 years ago

In 52754:

Script Loader: Prevent normalizing data URIs in _wp_normalize_relative_css_links().

This prevents making data URIs as in mask-image:url('data:image/svg+xml;utf8,<svg... relative to the WordPress installation.

Props staatic.
See #54243.
Fixes #55177.

#14 @hellofromTonya
3 years ago

In 52762:

Script Loader: Prevent normalizing data URIs in _wp_normalize_relative_css_links().

This prevents making data URIs as in mask-image:url('data:image/svg+xml;utf8,<svg... relative to the WordPress installation.

Props staatic.
Merges [52754] to the 5.9 branch.
See #54243.
Fixes #55177.

#15 @westonruter
23 months ago

Performance follow-up: #58069

#16 @westonruter
22 months ago

In 55658:

Script Loader: Optimize performance of _wp_normalize_relative_css_links() by more than 2x.

  • Replace preg_match_all() and its secondary str_replace() call with preg_replace_callback().
  • Fix case where paths beginning with http and https (but not http: and https:) were erroneously not counted as relative.
  • Improve code style and readability by consolidating conditions and returning once.
  • Use str_starts_with() consistently instead of strpos().

Follow-up to [52036], [52695], and [52754].

Fixes #58069.
See #54243.

#17 @audrasjb
22 months ago

In 55736:

Script Loader: Optimize performance of _wp_normalize_relative_css_links() by more than 2x.

  • Replace preg_match_all() and its secondary str_replace() call with preg_replace_callback().
  • Fix case where paths beginning with http and https (but not http: and https:) were erroneously not counted as relative.
  • Improve code style and readability by consolidating conditions and returning once.
  • Use str_starts_with() consistently instead of strpos().

Follow-up to [52036], [52695], and [52754].

Props westonruter, adamsilverstein, azaozz.
Merges [55658] and [55669] to the 6.2 branch.
Fixes #58069.
See #54243.

Note: See TracTickets for help on using tickets.