Opened 13 years ago
Last modified 5 years ago
#19100 new enhancement
Introduce esc_color()
Reported by: | mfields | Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Formatting | Keywords: | has-patch dev-feedback |
Focuses: | css | Cc: |
Description
Currently there is no way to escape a color in hexadecimal notation before printing it to a block of css or saving to the database. Many themes like to introduce functionality, whether it be core-supported or completely custom, to change the color of various parts of the templates. I believe that a function such as esc_color()
would promote best practices while ensuring that unintended values do not get stored as colors and thus echoed in css blocks potentially breaking display.
Attachments (4)
Change History (24)
#2
@
13 years ago
Great question! IMHO it might be a good idea to standardize on hexadecimal notation. Although it is still a rough draft, the [WordPress Coding Standards for CSS]http://codex.wordpress.org/CSS_Coding_Standards currently states:
"Use hex code for colors. Avoid RBG and uppercase, and shorten values when possible: #fff instead of #FFFFFF. "
Also, I believe that the color-picker script bundled with core use hex colors.
Personally, I've always tried to avoid using named values to define colors. The only valid reason to use decimal notation IMHO is to take advantage of the alpha channel in the RGBA color space. I do not believe that there is a currently a core-supported way of tweaking the alpha channel. If there is, It's been hiding from me all of this time :)
#3
@
13 years ago
I agree with westi - this shouldn't be included unless rgb()
, rgba()
, hsl()
and hsla()
formats are included. Just because a rough draft of some coding standards (which as we know, WP doesn't follow to the letter anyway) says only to use hex colours, doesn't mean you shouldn't support all formats, if it's intended to be used by theme developers.
I'd support inclusion of this, if it was esc_color_hex()
, with the other formats being added as individual functions too, along with a master esc_color()
that checked the string against each of the formats until one was found, that way, a theme developer *could* be more specific and only accept hex colours, *or*, could accept colours in any format.
Also, what about color keywords, or currentColor, or the deprecated (for CSS3, but still valid at CSS2) system colours?
#4
@
13 years ago
As the patch shows, it's pretty easy to escape colors if you only need to support hex notation. The value of an esc_color() function in Core would be if it supports other notations as well.
I am willing to bet money on the fact that once HSL(A) CSS color notation is better supported by browsers, it will be preferred by designers.
#5
@
13 years ago
Actually, since this would be used in Core (custom background color), I guess other notations could be added later, as the need arises.
#6
@
13 years ago
I'm not against supporting other color-spaces or notations. The reason I choose hexadecimal is because it the easiest to deal with when storing, sanitizing and escaping. Both the custom background and custom header text color UIs use hexadecimal notation and I think that this is a great place to start. It's rare that I see extensions that store color values in any other way.
I've uploaded a new patch that changes the function name a suggested by GaryJ.
I've overwritten 19100.diff which was a botched patch to begin with.
#9
@
13 years ago
I don't think we use any of the ctype_* functions anywhere else in core, and since it's one of those sets of functions that can be disabled using a flag, we should probably try to see how many hosts have it disabled.
#11
@
13 years ago
- Cc mike@… added
Surveying webhosts for ctype_ seems like overkill here.
Regex works for me. It can be a little slow but these strings should be short and I don't think this will get used too many times in a loop. To prevent super-long strings from causing slowdowns, we could also move the length-check ahead of the character check:
if ( ! in_array( strlen( $color ), array( 3, 6 ) ) ) return $default; if( function_exists( "ctype_xdigit" ) ) { if ( ! ctype_xdigit( $color ) ) return $default; } elseif ( ! preg_match( "/(([a-f0-9]){3}){1,2}$/i", $color ) ) { return $default; }
(Please check my regex syntax, I didn't try to break this enough)
#12
@
13 years ago
We can't use ctype_*. Needs to be is_numeric() followed by a preg_match.
Like westi, I like the idea of standardizing this, but I question introducing something which standardizes on hex. Non-hex values are used more and more these days. Granted, core only supports hex *currently*.
esc_color_hex() seems okay. I'm not sure I like esc_* though. sanitize_color_hex?
#13
@
13 years ago
@nacin, I don't understand the need for is_numeric. Would that be more efficient than pulling the hashes this way?
+1 on sanitize_color_hex. Just seems more like existing names.
BTW, on the efficiency, instead of if...strpos...substr, would it be faster to do trim( urldecode( $color ), '#' ) ?
#14
@
13 years ago
I speed tested several different methods & the one I went with in 19100.2.diff _might_ be the best of three alternatives. Note that it's 30-35% faster than _mfields strpos() methods when a hash exists, but is 3-8% slower when there's no character to be trimmed.
Also, my regex seems good, but I'd like if it could be tested more.
#16
@
11 years ago
- Component changed from General to Formatting
- Milestone changed from Awaiting Review to Future Release
I'm OK with the proposal here, but I feel like this should be a sanitize_* function, not an escaping one. Let's keep the esc_* namespace to security things. This isn't necessarily for security or output.
I like the idea of standardising here but what about all the other valid ways of specifying colours?
http://www.w3.org/TR/css3-color/#colorunits