Make WordPress Core

Opened 7 months ago

Closed 7 months ago

Last modified 4 months ago

#59715 closed defect (bug) (fixed)

blocks RTL Styles doesn't load in recent versions

Reported by: maahrokh's profile maahrokh Owned by: mukesh27's profile mukesh27
Milestone: 6.4 Priority: normal
Severity: critical Version: 6.3
Component: Script Loader Keywords: has-patch has-unit-tests commit dev-reviewed
Focuses: css, rtl Cc:

Description

Hi,
in recent versions of WP, RTL versions of block styles doesn't load

Example:
http://localhost/wordpress/wp-includes/blocks/social-links/style.min.css?ver=6.3.2
http://localhost/wordpress/wp-includes/blocks/navigation/style.min.css?ver=6.3.2

But in 6.2 they load correctly in RTL mode:

http://localhost/!wordpress/wp-includes/blocks/navigation/style-rtl.min.css?ver=6.2

I think all RTL websites have visibility problem now

Change History (30)

This ticket was mentioned in Slack in #core by maahrokh. View the logs.


7 months ago

#2 @maahrokh
7 months ago

I can confirm that this problem is from 6.3
On 6.2.3 is OK

This ticket was mentioned in Slack in #core-performance by joemcgill. View the logs.


7 months ago

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


7 months ago

#5 @hellofromTonya
7 months ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 6.4
  • Version set to 6.3

Hello @maahrokh,

Welcome to WordPress Core's Trac :) Thank you for reporting this issue.

I'm pulling it into the 6.4 milestone for awareness and investigation to confirm its severity.

As I noted in Make/Core slack today, the next steps:

  • As @joemcgill noted, it may have been introduced in 6.3 by [56064].
  • Needs testing: to confirm RTL sites are indeed broken (ie to confirm severity).
  • If yes, then a high confidence fix might be a 6.4 RC3 candidate.
Last edited 7 months ago by joedolson (previous) (diff)

#6 @huzaifaalmesbah
7 months ago

RTL css not loaded in 6.3.2 it right but I didn't notice any problem on RLT site. I think we need more opinions.

#7 @rajinsharwar
7 months ago

  • Keywords needs-testing-info added

Hi @maahrokh, thanks for the ticket. Could you please add testing steps to the ticket? I tried to load the RTL version of the scripts in 6.3.2, but it does seem to load. So, would be really helpful for all if you could test information on how to properly replicate the bug.

#8 @maahrokh
7 months ago

Hi,
@huzaifaalmesbah When RTL CSS doesn't load so there are problem with Right To Left direction
For Example Menus , etc
You can find the difference of CSS and RTL CSS, so you can find what are different in RTL
So these are problems of loading

@rajinsharwar Did you use any editor blocks which loads RTL CSS?
For example these two blocks:
social-links and navigation

http://localhost/wordpress/wp-includes/blocks/social-links/style.min.css?ver=6.3.2
http://localhost/wordpress/wp-includes/blocks/navigation/style.min.css?ver=6.3.2

These styles should load in this case:
http://localhost/wordpress/wp-includes/blocks/social-links/style-rtl.min.css?ver=6.3.2
http://localhost/wordpress/wp-includes/blocks/navigation/style-rtl.min.css?ver=6.3.2

It's ok in 6.2.3

Last edited 7 months ago by maahrokh (previous) (diff)

#9 follow-up: @huzaifaalmesbah
7 months ago

@maahrokh Thanks. I found the difference.

@hellofromTonya i confirm that RTL site broken on wp 6.3.2

6.2

https://i.ibb.co/dPVW5WB/Screenshot-2023-10-28-at-8-16-59-AM.png

6.3.2

https://i.ibb.co/gm9jgT6/Screenshot-2023-10-28-at-8-15-33-AM.png

#10 in reply to: ↑ 9 @devmuhib
7 months ago

How can I reproduce it?

Replying to huzaifaalmesbah:

@maahrokh Thanks. I found the difference.

@hellofromTonya i confirm that RTL site broken on wp 6.3.2

6.2

https://i.ibb.co/dPVW5WB/Screenshot-2023-10-28-at-8-16-59-AM.png

6.3.2

https://i.ibb.co/gm9jgT6/Screenshot-2023-10-28-at-8-15-33-AM.png

#11 follow-up: @rajinsharwar
7 months ago

Yes, if @huzaifaalmesbah @maahrokh could add step-by-step testing instructions, it would have been really helpful for both developers and testers to replicate :)

I tried to add both navigating navigation block, and Social Link blocks, but both seemed to work well with the RTL version in WP 6.3.2. (Tested in a non-block theme) Is there any specific theme you are using?

#12 in reply to: ↑ 11 @maahrokh
7 months ago

Replying to rajinsharwar:

Yes, Test on a block theme!
First check if these CSS styles are loading:

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

So you are in a right way to check, if not try to add those blocks in the site
Then switch to a RTL language like Persian, Arabic or Hebrew
Then check if RTL version of the files are loading or not:

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

Yes, if @huzaifaalmesbah @maahrokh could add step-by-step testing instructions, it would have been really helpful for both developers and testers to replicate :)

I tried to add both navigating navigation block, and Social Link blocks, but both seemed to work well with the RTL version in WP 6.3.2. (Tested in a non-block theme) Is there any specific theme you are using?

#13 @huzaifaalmesbah
7 months ago

Reproduce Instructions:

  1. Install or upgrade to WordPress 6.3.2
  2. Activate a default block theme (e.g., Twenty Twenty-Two or Twenty Twenty-Three).
  3. Next, create some pages and subpages for navigation blocks, or open the full-site editor and access the Navigation section to add a menu and submenu items.
  4. Switch to a right-to-left (RTL) language for your WordPress site.
  5. After switching to an RTL language, check the frontend navigation menu. You may notice that it appears broken or not functioning correctly.
  6. To better understand the issue, compare this behavior with how the navigation menu worked in WordPress version 6.2.

This ticket was mentioned in Slack in #core by huzaifaalmesbah. View the logs.


7 months ago

#15 @rajinsharwar
7 months ago

  • Keywords needs-patch added; needs-testing needs-testing-info removed

Thanks for the replication steps @huzaifaalmesbah @maahrokh. This seems to be a major issue as this could break many of the RTL-based blocks. We might need to hotfix this after RC3.

This ticket was mentioned in PR #5589 on WordPress/wordpress-develop by @mukesh27.


7 months ago
#17

  • Keywords has-patch added; needs-patch removed

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

The RTL style issue emerged following the r56524 commit in 6.3.2. Following this commit, the full path for the RTL style is now being used, as exemplified by /var/www/src/wp-includes/blocks/navigation/style-rtl.css.

Within the $files variable, we maintain a list of styles that must be loaded alongside the block path, for instance, blockname/blockstyle.css.

However, an issue arises with the condition:

if ( is_rtl() && in_array( $rtl_file, $files, true ) ) {

This condition is failing, leading to the failure to load the RTL style. Instead, the normal style for the block is loaded.

#18 @mukesh27
7 months ago

  • Focuses javascript removed
  • Keywords needs-testing added
  • Owner set to mukesh27
  • Status changed from new to assigned

You're welcome, @maahrokh, and thanks for ticket.

I have managed to reproduce the issue in the current RC and 6.3.2 version and have opened a PR 5589 with a solution. Can someone please test these changes? @rajinsharwar @huzaifaalmesbah @devmuhib

I will self-assign for now and removed javascript keyword as it only affect CSS.

#19 @rajinsharwar
7 months ago

PATCH TESTED: This is now loading the RTL version when switched to an RTL language.

#20 @mukesh27
7 months ago

  • Focuses performance added

Thanks for testing. Waiting for some other tests result.

Add performance keyword.

#21 @huzaifaalmesbah
7 months ago

I test working properly. RTL loading now and frontend navigation menu appears properly. @mukesh27 thanks for adding patch.

https://i.ibb.co/prY40TZ/Screenshot-2023-10-30-at-12-54-07-PM.png
https://i.ibb.co/2SpY1hf/Screenshot-2023-10-30-at-12-54-34-PM.png

#22 @devmuhib
7 months ago

Tested the patch. It's working now.

#23 @swissspidy
7 months ago

  • Focuses ui performance removed
  • Keywords needs-unit-tests added

Unit tests would be great for this, but can also be added later since time is of the essence.

#24 @maahrokh
7 months ago

  • Focuses ui performance added
  • Keywords needs-unit-tests removed

@mukesh27 Thank you for fix,
I just need to know about the loading of RTL CSS using block.json:
I appreciate if anyone know more info about this,
I created a ticket too:
https://wordpress.org/support/topic/loading-rtl-css-on-frontend-block-development/

#25 @SergeyBiryukov
7 months ago

  • Focuses ui performance removed
  • Keywords needs-unit-tests added

#26 @mukesh27
7 months ago

  • Keywords has-unit-tests added; needs-unit-tests removed

#27 @SergeyBiryukov
7 months ago

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

In 57028:

Editor: Correctly load RTL stylesheets in register_core_block_style_handles().

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.

Follow-up to [56524].

Props mukesh27, maahrokh, hellofromTonya, joemcgill, huzaifaalmesbah, rajinsharwar, devmuhib, swissspidy.
Fixes #59715.

@SergeyBiryukov commented on PR #5589:


7 months ago
#28

Thanks for the PR! Merged in r57028.

#29 @SergeyBiryukov
7 months ago

  • Keywords commit dev-feedback added; needs-testing removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backporting to the 6.4 branch after a second committer's review.

#30 @hellofromTonya
7 months ago

  • Keywords dev-reviewed added; dev-feedback removed

Marking [57028] okay for backport to the 6.4 branch.

Follow-up note:
Though using the same strategy as the previous test, I don't understand the need for the test assertion(s) conditional logic. Not a blocker for backporting.

#31 @SergeyBiryukov
7 months ago

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

In 57031:

Editor: Correctly load RTL stylesheets in register_core_block_style_handles().

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.

Follow-up to [56524].

Reviewed by hellofromTonya.
Merges [57028] to the 6.4 branch.

Props mukesh27, maahrokh, hellofromTonya, joemcgill, huzaifaalmesbah, rajinsharwar, devmuhib, swissspidy.
Fixes #59715.

Note: See TracTickets for help on using tickets.