Make WordPress Core

Opened 3 years ago

Closed 2 years 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 3 years ago.
Screenshot - Farsi
56325.diff (560 bytes) - added by sabernhardt 2 years ago.
PR 3096, without the extra tabs
56325.2.diff (803 bytes) - added by SergeyBiryukov 2 years ago.
56325.3.diff (818 bytes) - added by SergeyBiryukov 2 years ago.
Refreshed after [54290]
56325.4.diff (789 bytes) - added by SergeyBiryukov 2 years ago.
Refreshed after [54323]

Download all attachments as: .zip

Change History (18)

@zoonini
3 years ago

Screenshot - Farsi

#1 @sabernhardt
3 years 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
3 years 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
2 years ago

PR 3096, without the extra tabs

#3 @sabernhardt
2 years 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 2 years ago by sabernhardt (previous) (diff)

#4 @sabernhardt
2 years 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
2 years 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
2 years 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
2 years 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 2 years ago by Ankit K Gupta (previous) (diff)

@SergeyBiryukov
2 years ago

Refreshed after [54290]

#8 @aristath
2 years ago

Patch looks good to me :+1:

@SergeyBiryukov
2 years ago

Refreshed after [54323]

#9 @SergeyBiryukov
2 years 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
2 years ago

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

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

#11 @SergeyBiryukov
2 years 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
2 years 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
2 years 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.