#26316 closed defect (bug) (fixed)
Colour Scheme picker doesn't switch with RTL
Reported by: | dd32 | Owned by: | nacin |
---|---|---|---|
Milestone: | 3.8 | Priority: | normal |
Severity: | major | Version: | |
Component: | RTL | Keywords: | |
Focuses: | Cc: |
Description
Requires #26315 to be fixed first so that the colors RTL stylesheets are generated.
When in RTL mode, the colour scheme picker doesn't automatically switch to the new colour scheme until the next page load. Haven't tracked it down, but it appears that css_url
doesn't refer to a rtl stylesheet, although that doesn't seem like the cause of this.
Attachments (1)
Change History (11)
#3
@
11 years ago
- Severity changed from normal to major
There's enough of a mess here to classify this as fairly major. I'm going to work on this.
#6
@
11 years ago
This got complicated quickly. Integrating WP_Styles is more trouble than it is worth, at least for the moment. It is easier to think of colors stylesheets as entirely independent from WP_Styles, at which point adding support for RTL and minified CSS is as easy as checking on registration of the color schemes.
#7
@
11 years ago
- Owner set to nacin
- Resolution set to fixed
- Status changed from new to closed
In 26780:
#9
@
11 years ago
After updating to the latest revision (r26782) this AM, I had a notice display in the dashboard at the top of the WordPress News widget:
Notice: Use of undefined constant SCRIPT_DEBUG - assumed 'SCRIPT_DEBUG' in C:\xampp\htdocs\wordpress-svn\src\wp-includes\general-template.php on line 2119
That code was introduced in 26780.
It only appeared once. I added back the rest of the widgets for the dashboard screen in Screen Options, and the notice disappeared. I've logged out and back in, and the notice does not reappear, so can't reproduce the issue.
The issue here is that RTL is handled by the script loader, while color schemes are a separate beast. The actual URL used for the color scheme doesn't match what is registered, thanks to all of our fancy juggling. This occurs in two places: wp_style_loader_src(), to define a URL for the 'colors' "meta" handle; and in WP_Styles::do_item(), which deals with RTL stuff.
This bug also means that the color picker only previews minified color files, never the development versions. Not a big deal, though.
I attached what I was playing with to debug this. Ideally, wp_admin_css_color() should accept the handle of a registered style instead of a URL. That way we wouldn't be guessing when it came to .min.css versus .css (or .css versus .dev.css), whether it supported RTL, whether the RTL was in-addition-to or should replace it, etc. This was a mess when we moved from .dev.css to .min.css (especially since .css went from being minified to raw) and MP6 makes it worse.
Alternatively, we might need to expose some helper methods on WP_Styles that can generate a URL outside the context of doing an item. That's not fun either.
FWIW, we didn't previously have an RTL version of colors stylesheets, instead opting for some .rtl classes. This means we could simply ignore the in-addition-to or replace issue, and assume that no other colors stylesheets have an RTL stylesheet. Indeed, in 3.7, colors-fresh.css wasn't defined as an RTL stylesheet, so a color scheme wouldn't have had support for -rtl.css out of the box. This does make things a bit easier here.
We still can't blindly replace .css with -rtl.css because it will break 3rd-party color schemes, but I think 3.8 was breaking them anyway in RTL (in general, not in preview) and in other contexts anyway. I will test with the bbPress color scheme.