Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#53090 closed defect (bug) (wontfix)

Admin color schemes don't work in RTL

Reported by: tomjcafferkey's profile tomjcafferkey Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch
Focuses: css, rtl Cc:

Description

Steps to replicate

  • Install [Page Optimize](https://wordpress.org/plugins/page-optimize/) plugin
  • Change your admin color scheme in /wp-admin/profile.php to a non-default scheme
  • Go to /wp-admin/options-general.php and update your Site Language to an RTL language

What I expected to happen

My admin color scheme would be the one I updated it to.

What actually happened

Admin color scheme switched back to the default color scheme.

Additional information

We found out this is because there needs to be [extra data set](https://developer.wordpress.org/reference/functions/wp_style_add_data/) for RTL when enqueuing the stylesheet:

wp_style_add_data( 'colors', 'rtl', 'replace' ) fixes this when the site language is set to RTL

Additional RTL support documentation that recommends wp_style_add_data for RTL found [here](https://codex.wordpress.org/Right-to-Left_Language_Support).

This will also help maintain consistency as admin-menu stylesheet does include this additional data for RTL support when the Site Language is set to an RTL language.

Attachments (5)

Screen Shot 2021-04-26 at 15.42.29.png (86.1 KB) - added by desrosj 4 years ago.
53090.diff (903 bytes) - added by desrosj 4 years ago.
Screen Shot 2021-04-29 at 2.11.29 PM.png (133.1 KB) - added by ryelle 4 years ago.
53090.2.diff (1.1 KB) - added by ryelle 4 years ago.
Screen Shot 2021-04-29 at 2.19.35 PM.png (40.8 KB) - added by ryelle 4 years ago.

Download all attachments as: .zip

Change History (15)

#1 @desrosj
4 years ago

  • Component changed from General to Administration
  • Focuses administration removed
  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

Hi @tomjcafferkey,

Welcome to Trac! And thanks for this very detailed error report!

I tested the steps that you supplied, and I am able to reproduce the issue when Page Optimize is installed. However, it's a bit strange because some of the colors make their way to the user, but most do not (note the brown in Screen Shot 2021-04-26 at 15.42.29.png).

However, if I do not install Page Optimize, I am unable to reproduce this issue. This means it is likely a bug in the plugin and not WordPress Core.

I've gone ahead and opened a topic in that plugin's support forums so that the right people can investigate. You can follow along here: https://wordpress.org/support/?post_type=topic&p=14367315.

I'm closing this as invalid, but only because that is Trac's default ticket resolution and this report is not an issue with WordPress Core.

#2 @tomjcafferkey
4 years ago

Hi @desrosj

Thank you for getting back to me so quickly and replicating the issue.

I'd just like to clarify that I don't believe this is an issue within the plugin, the plugin is surfacing an issue within core due to some missing information that the documentation advises should be added to core stylesheet data which the plugin is expecting.

As per the documentation [here]https://codex.wordpress.org/Right-to-Left_Language_Support it is advised that the additional RTL data should be added when the Site Language is set to an RTL site language.

Use wp_style_add_data to set the rtl property to replace for your stylesheet.

This is something core does with other stylesheets (admin-menu etc) however the colors stylesheet is missing this data which the Page Optimize plugin has surfaced during some testing.

If you've any further questions on this, or I have misunderstood the issue please don't hesitate to let me know.

#3 @tomjcafferkey
4 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

@desrosj
4 years ago

#4 @desrosj
4 years ago

  • Keywords has-patch added
  • Milestone set to 5.7.2

Thanks @tomjcafferkey. I looked into this more, and you are correct! Thanks for being patient and further explaining.

The admin color scheme stylesheets are an outlier in that they are registered with true as their source, and the actual file URL is constructed later in wp_style_loader_src() via the style_loader_src filter.

I've attached 53090.diff, which should fix the issue. However, while this fixes the issue when the plugin is active when using an RTL language, it causes the color schemes to not load when the plugin is inactive and using an RTL language.

The reason seems to have something to do with the code in wp_style_loader_src(). The URL being returned is 'http://1?ver=5.8-alpha-50427-src. The 1 comes from the value of $src passed. But I haven't figured out why this happens.

@ryelle I was also curious if not using wp_style_add_data( 'colors', 'rtl', 'replace' ) was intentional.

Moving to 5.7.2, as this would be good to get fixed in the next minor release, if ready.

#5 @tomjcafferkey
4 years ago

Thank you @desrosj for revisiting and fixing this! :)

However, while this fixes the issue when the plugin is active when using an RTL language, it causes the color schemes to not load when the plugin is inactive and using an RTL language.

This is interesting, and also something I might investigate myself if you haven't already figured it out by then. However if you do, would you mind updating here so I can learn? Thank you!

I was also curious if not using wp_style_add_data( 'colors', 'rtl', 'replace' ) was intentional.

Same goes for this.

Thanks again!

@ryelle
4 years ago

#6 follow-up: @ryelle
4 years ago

The root of the problem here is that Page Optimize hoists the color CSS file up above the rest of the CSS, but it's intended to override the other CSS. When it's loaded early, the default colors end up overriding the selected color scheme. You can see a similar issue with Page Optimize + non-RTL sites in Screen Shot 2021-04-29 at 2.11.29 PM.png The plugin should probably exclude this file from its concatenation.

I was also curious if not using wp_style_add_data( 'colors', 'rtl', 'replace' ) was intentional.

I'm looking into this as a write, and so the patch I uploaded is wrong 😅

This comes from r26780/#26316 - when on the profile page in an RTL language, click through the color scheme live previews, with 53090.diff it loads the non-RTL stylesheet. This is most obvious in the admin menu Screen Shot 2021-04-29 at 2.19.35 PM.png. It uses the URL in $_wp_admin_css_colors, so that needs to be RTL-aware.


For record-keeping, this is the logic behind 53090.2.diff - but we probably shouldn't make that change.

However, while this fixes the issue when the plugin is active when using an RTL language, it causes the color schemes to not load when the plugin is inactive and using an RTL language.

When it's automatically RTL'd with add_data, the style's handle changes to colors-rtl, and then the URL replacement doesn't happen. I updated the patch with a check for colors-rtl, so now it will work in both cases.

#7 @dd32
4 years ago

  • Milestone changed from 5.7.2 to 5.7.3

WordPress 5.7.2 has been released, moving open tickets to 5.7.3

#8 in reply to: ↑ 6 @peterwilsoncc
4 years ago

Replying to ryelle:

For record-keeping, this is the logic behind 53090.2.diff - but we probably shouldn't make that change.

Should I hold off on this for 5.7.3 in its current form?

#9 follow-up: @ryelle
4 years ago

I think this should be wontfix, actually- that the issue is in the Page Optimize plugin after all. The colors css is just a special case that doesn't handle RTL the same as other styles.

#10 in reply to: ↑ 9 @peterwilsoncc
4 years ago

  • Milestone 5.7.3 deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

Replying to ryelle:

I think this should be wontfix, actually ... The colors css is just a special case that doesn't handle RTL the same as other styles.

Thanks boss, that makes sense from both the perspective of purpose and handling the cascade.

--

@tomjcafferkey An issue has been opened on the plugin's support forum by @desrosj if you wish to follow up there. It seems color schemes have been treated as a special case for some years looking over #26316.

Note: See TracTickets for help on using tickets.