Make WordPress Core

Opened 20 months ago

Last modified 5 weeks ago

#61625 accepted defect (bug)

Styles enqueued from block.json are not correctly configured for RTL

Reported by: ryelle's profile ryelle Owned by: westonruter's profile westonruter
Milestone: 7.0 Priority: normal
Severity: normal Version:
Component: Script Loader Keywords: needs-patch
Focuses: rtl Cc:

Description

When viewing a site using an RTL language, the RTL CSS files are not loaded for custom blocks using block.json. For example, a block might say "style": "file:./style-index.css", which is then loaded as-is on LTR sites. On RTL sites, the style-index.css file is still loaded, despite style-index-rtl.css existing.

This seems to be due to the register_block_style_handle function — it does correctly add data for the RTL file, style-index-rtl.css, but it also adds a suffix value (when SCRIPT_DEBUG = false).

When the style tag is printed, WP tries to update the file's url to the RTL file, but it's searching for .min.css to replace, and that isn't the right filename. The url is not updated, so it still loads the LTR style-index.css.

This can be replicated with the default create-block block, I've also created a simple demo plugin to explain: https://github.com/ryelle/rtl-style-example

Change History (5)

This ticket was mentioned in PR #638 on WordPress/wporg-mu-plugins by @ryelle.


20 months ago
#1

  • Keywords has-patch added

CSS registered from block.json are incorrectly configured with a suffix for RTL sites, but the files don't use a suffix. This prevents the files from being replaced correctly, causing visual issues on RTL sites.

The "simple" fix here is to just wipe out the suffix on all wporg-added styles. Checking for the rtl extra data should also prevent replacing anything that isn't using the block.json registration. We don't use .min or separate minified files anywhere (IIRC), so this shouldn't have any negative impacts.

See https://core.trac.wordpress.org/ticket/61625.

Before After
Query filters https://github.com/WordPress/wporg-mu-plugins/assets/541093/241bb2c2-8cd8-4113-8224-6f333caf0935 https://github.com/WordPress/wporg-mu-plugins/assets/541093/89d507ab-667b-4271-adb8-a1bdceee6737
Local nav https://github.com/WordPress/wporg-mu-plugins/assets/541093/89f68240-ee2b-4aed-9cd3-a48c29e5451f https://github.com/WordPress/wporg-mu-plugins/assets/541093/cd2bf803-9ae5-473c-a1ac-3c689f896e59

To test

This is easiest to test on a sandbox, so that you can view a real RTL site. When applied, this should fix a lot of the issues flagged on https://github.com/WordPress/Learn/issues/2483.

#2 @ryelle
20 months ago

  • Keywords has-patch removed

Removing the has-patch tag, the above PR is not for core & shouldn't have been attached. It's just a workaround for RTL on wordpress.org.

#3 @sippis
3 months ago

  • Keywords needs-patch added

We just ran into this with @tnke and another colleague. Can confirm it's still an issue with 6.8.3 but partially fixed on 6.9-RC1-61214.

In 6.9-RC1-61214 RTL styles are inlined correctly, but style tag still suggests non RTL file as sourceURL.

register_block_style_handle sets the file path correctly for RTL, leaving src untouched. This leads to RTL file being inlined correctly as well, while src being non RTL version and making its way to sourceURL of inlined styles.

#4 @westonruter
3 months ago

  • Milestone changed from Awaiting Review to 7.0
  • Owner set to westonruter
  • Status changed from new to accepted

What I see in RC1 using @ryelle's test plugin, when a site is set to have Arabic as its language.

With add_filter( 'styles_inline_size_limit', '__return_zero' )

SCRIPT_DEBUG is true:

<link
  rel="stylesheet"
  id="ryelle-rtl-style-example-style-rtl-css"
  href="http://localhost:8000/wp-content/plugins/rtl-style-example/build/style-index-rtl.css?ver=1762973381"
  media="all"
/>

SCRIPT_DEBUG is false:

<link
  rel="stylesheet"
  id="ryelle-rtl-style-example-style-rtl-css"
  href="http://localhost:8000/wp-content/plugins/rtl-style-example/build/style-index.css?ver=0.1.0"
  media="all"
/>

Note that in the latter the stylesheet lacks the -rtl suffix in the filename.

Without add_filter( 'styles_inline_size_limit', '__return_zero' )

SCRIPT_DEBUG is true:

<style id='ryelle-rtl-style-example-style-inline-css'>
/*!***************************************************************************************************************************************************************************************************************************************!*\
  !*** css ./node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[4].use[1]!./node_modules/postcss-loader/dist/cjs.js??ruleSet[1].rules[4].use[2]!./node_modules/sass-loader/dist/cjs.js??ruleSet[1].rules[4].use[3]!./src/style.scss ***!
  \***************************************************************************************************************************************************************************************************************************************/
/**
 * The following styles get applied both on the front of your site
 * and in the editor.
 *
 * Replace them with your own styles or remove the file completely.
 */
.wp-block-ryelle-rtl-style-example {
  border-right: 10px solid #21759b;
  padding: 10px;
}

/*# sourceURL=http://localhost:8000/wp-content/plugins/rtl-style-example/build/style-index.css */
</style>

SCRIPT_DEBUG is false:

<style id='ryelle-rtl-style-example-style-inline-css'>
/*!***************************************************************************************************************************************************************************************************************************************!*\
  !*** css ./node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[4].use[1]!./node_modules/postcss-loader/dist/cjs.js??ruleSet[1].rules[4].use[2]!./node_modules/sass-loader/dist/cjs.js??ruleSet[1].rules[4].use[3]!./src/style.scss ***!
  \***************************************************************************************************************************************************************************************************************************************/
/**
 * The following styles get applied both on the front of your site
 * and in the editor.
 *
 * Replace them with your own styles or remove the file completely.
 */
.wp-block-ryelle-rtl-style-example {
  border-right: 10px solid #21759b;
  padding: 10px;
}

/*# sourceURL=http://localhost:8000/wp-content/plugins/rtl-style-example/build/style-index.css */
</style>

The inlined CSS is correct regardless of SCRIPT_DEBUG, but when the CSS is not inlined, then it is incorrect. This is reflected in the example plugin:

<?php
// Prevent the site from inlining any CSS, inlined CSS uses
// the `path` data which is set correctly.
add_filter( 'styles_inline_size_limit', '__return_zero' );

Nevertheless, the sourceURL is remains incorrect, which reflects the underlying issue for non-inlined styles.

Note that as of #63018 more CSS will get inlined since the limit has been increased from 20K to 40K.

#5 @westonruter
5 weeks ago

In 61469:

Themes: Use WP_HTML_Tag_Processor to insert the block template skip link instead of JavaScript.

  • The skip link now works when JavaScript is turned off.
  • By removing the script, the amount of JavaScript sent to the client is reduced for a very marginal performance improvement.
  • A new wp-block-template-skip-link stylesheet is registered, with minification and path data for inlining.
  • The CSS for the skip link now has an RTL version generated, although it is not yet served when the styles are inlined. See #61625.
  • The wp_enqueue_block_template_skip_link() function now exclusively enqueues the stylesheet since the script is removed.
  • For backwards-compatibility, the skip link will continue to be omitted if the_block_template_skip_link() is unhooked from the wp_footer action or wp_enqueue_block_template_skip_link() is unhooked from wp_enqueue_scripts.

Developed in https://github.com/WordPress/wordpress-develop/pull/10676

Follow-up to [56932], [51003].

Props rutviksavsani, westonruter, dmsnell, whiteshadow01, Slieptsov.
See #59505, #53176.
Fixes #64361.

Note: See TracTickets for help on using tickets.