Make WordPress Core

Opened 10 months ago

Closed 8 months ago

#56325 closed defect (bug) (fixed)

Some blocks' RTL stylesheets not loading when RTL language is set

Reported by: zoonini's profile zoonini Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.1 Priority: normal
Severity: major Version: 6.0
Component: Editor Keywords: has-patch needs-testing needs-unit-tests
Focuses: css, rtl Cc:

Description (last modified by sabernhardt)

When setting an RTL language under Settings > General, some RTL stylesheets are not loaded, meaning that some blocks are not displayed correctly.

To replicate

  • Add navigation block to the header if there isn't one already
  • Visit Settings > General
  • Choose an RTL language
  • View site on the front end

I tested with:

  • Languages: Hebrew and Farsi
  • WordPress 6.0.1
  • No active plugins

Result

Some RTL stylesheets are not loaded; instead, the LTR stylesheets are loaded.

Examples of stylesheets being loaded:

/wp-includes/blocks/social-links/style.min.css?ver=6.0.1
/wp-includes/blocks/navigation/style.min.css?ver=6.0.1
/wp-includes/blocks/cover/style.min.css?ver=6.0.1

Expected

When selecting an RTL language, all appropriate RTL stylesheets should be loaded.

Examples of stylesheets that should be loaded:

/wp-includes/blocks/social-links/style-rtl.min.css?ver=6.0.1
/wp-includes/blocks/navigation/style-rtl.min.css?ver=6.0.1
/wp-includes/blocks/cover/style-rtl.min.css?ver=6.0.1

Note: These are three examples provided by the original reporter; there may also be other stylesheets not loading for other blocks.

Screenshots

Example of how the navigation menu looks with language set to Farsi is attached.

Report

https://wordpress.org/support/topic/rtl-problem-bug/

Attachments (5)

farsi.jpg (591.3 KB) - added by zoonini 10 months ago.
Screenshot - Farsi
56325.diff (560 bytes) - added by sabernhardt 10 months ago.
PR 3096, without the extra tabs
56325.2.diff (803 bytes) - added by SergeyBiryukov 9 months ago.
56325.3.diff (818 bytes) - added by SergeyBiryukov 8 months ago.
Refreshed after [54290]
56325.4.diff (789 bytes) - added by SergeyBiryukov 8 months ago.
Refreshed after [54323]

Download all attachments as: .zip

Change History (18)

@zoonini
10 months ago

Screenshot - Farsi

#1 @sabernhardt
10 months ago

  • Component changed from Bundled Theme to Editor
  • Description modified (diff)
  • Keywords close added

Thanks for the report!

These are editor stylesheets, and the same occurs with the TT1 Blocks theme, so the report belongs on GitHub. Gutenberg issue 41649 seems similar, and I'll comment there to ask whether it's related.

#2 @maahrokh
10 months ago

  • Severity changed from normal to major

Please attention to this bug, Because all RTL web sites have problem in visibility of components
Thank you

@sabernhardt
10 months ago

PR 3096, without the extra tabs

#3 @sabernhardt
10 months ago

  • Keywords has-patch needs-testing added; close removed
  • Milestone changed from Awaiting Review to 6.0.2
  • Summary changed from Twenty Twenty-Two: Some RTL stylesheets not loading when RTL language is set to Some RTL stylesheets not loading when RTL language is set

Farhad Sakhaei's PR 3096 modifies wp-includes/blocks.php, so we might be able to fix this apart from the plugin. (props farhad0)

The editor fetches the minified RTL stylesheets when I apply the patch's change to my WordPress 6.0.1 installation with Gutenberg deactivated and SCRIPT_DEBUG set to false.

Last edited 10 months ago by sabernhardt (previous) (diff)

#4 @sabernhardt
10 months ago

  • Summary changed from Some RTL stylesheets not loading when RTL language is set to Some blocks' RTL stylesheets not loading when RTL language is set

#5 @SergeyBiryukov
9 months ago

  • Keywords needs-unit-tests added

Thanks for the patch!

If my understanding is correct, these lines can be added to an existing conditional a few lines below, see 56325.2.diff.

This would need a unit test for register_block_style_handle() and could use some manual testing, so I'm not quite comfortable committing this just yet, but there is still some time before 6.0.2 comes next Tuesday.

#6 @SergeyBiryukov
9 months ago

  • Milestone changed from 6.0.2 to 6.1

Looks like this did not get any further traction yet. With 6.0.2 coming today, moving to 6.1 for now.

#7 @Ankit K Gupta
8 months ago

Test Report

Env

Steps to test

  1. Add Navigation Block to the header
  2. Switch site to RTL language (Persian)
  3. Open site in front-end and view source.
  4. Check the source of CSS
  5. Apply the patch file, refresh page and check the CSS source.
  6. RTL CSS should load now.

Test result

  • Performed some Manual Testing. RTL CSS is loading in RTL sites for navigation block after the patch.


Actual Results

  • ✅ Issue resolved with patch.

Before Fix -

<link rel='stylesheet' id='wp-block-navigation-css'  href='https://core.local/src/wp-includes/blocks/navigation/style.min.css?ver=6.1-beta1-54282-src' media='all' />

After Fix -

<link rel='stylesheet' id='wp-block-navigation-rtl-css'  href='https://core.local/src/wp-includes/blocks/navigation/style-rtl.min.css?ver=6.1-beta1-54282-src' media='all' />

Thanks!

Last edited 8 months ago by Ankit K Gupta (previous) (diff)

@SergeyBiryukov
8 months ago

Refreshed after [54290]

#8 @aristath
8 months ago

Patch looks good to me :+1:

@SergeyBiryukov
8 months ago

Refreshed after [54323]

#9 @SergeyBiryukov
8 months ago

In 54330:

Editor: Correctly load RTL stylesheets in register_block_style_handle().

When setting an RTL language under Settings → General, some RTL stylesheets were not loaded, with LTR stylesheets being loaded instead, meaning that some blocks were not displayed correctly.

This commit ensures that all appropriate RTL stylesheets are loaded when selecting an RTL language.

Additionally, this commit improves performance by only running a file_exists() check for an RTL stylesheet if is_rtl() returns true, i.e. an RTL locale is selected.

Follow-up to [49982], [50836].

Props zoonini, sabernhardt, maahrokh, ankit-k-gupta, aristath, poena, SergeyBiryukov.
See #56325.

#10 @SergeyBiryukov
8 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

Keeping the ticket open for now to add some unit tests.

#11 @SergeyBiryukov
8 months ago

In 54331:

Tests: Update block registration tests to account for RTL stylesheet loading changes.

The RTL data should only be added in register_block_style_handle() if an RTL locale is selected, so these test expectations do not appear to be correct.

Follow-up to [49982], [53091], [54330].

See #56325.

#12 follow-up: @audrasjb
8 months ago

@SergeyBiryukov is it still in your todolist?

If not, we could probably close this as fixed and open a new ticket to handle the missing unit test. What do you think?

#13 in reply to: ↑ 12 @SergeyBiryukov
8 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

Replying to audrasjb:

we could probably close this as fixed and open a new ticket to handle the missing unit test. What do you think?

Sounds good :) Follow-up: #56797.

Note: See TracTickets for help on using tickets.