Fonts: Normalize font face font-family #9951
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -358,17 +358,7 @@ private function order_src( array $font_face ) { | |
| private function build_font_face_css( array $font_face ) { | ||
| $css = ''; | ||
|
|
||
| $font_face['font-family'] = $this->normalize_css_font_family( $font_face['font-family'] ); | ||
|
|
||
| foreach ( $font_face as $key => $value ) { | ||
| // Compile the "src" parameter. | ||
|
|
@@ -389,6 +379,69 @@ private function build_font_face_css( array $font_face ) { | |
| return $css; | ||
| } | ||
|
|
||
| /** | ||
| * Normalizes a font-face name for use in CSS. | ||
| * | ||
| * Add quotes to the font-face name and escape problematic characters. | ||
| * | ||
| * @see https://www.w3.org/TR/css-fonts-4/#font-family-desc | ||
| * | ||
| * @since 6.9.0 | ||
| * | ||
| * @param string $font_family The font-face name to normalize. | ||
| * @return string The normalized font-face name. | ||
| */ | ||
| protected function normalize_css_font_family( string $font_family ): string { | ||
| $font_family = trim( $font_family, " \t\r\f\n" ); | ||
|
|
||
| if ( | ||
| strlen( $font_family ) > 1 && | ||
| ( '"' === $font_family[0] && '"' === $font_family[ strlen( $font_family ) - 1 ] ) || | ||
| ( "'" === $font_family[0] && "'" === $font_family[ strlen( $font_family ) - 1 ] ) | ||
| ) { | ||
| _doing_it_wrong( | ||
| __METHOD__, | ||
| __( 'Font family should not be wrapped in quotes; they will be added automatically.' ), | ||
| '6.9.0' | ||
| ); | ||
| return $font_family; | ||
| } | ||
|
|
||
| return '"' . strtr( | ||
sirreal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| $font_family, | ||
| array( | ||
| /* | ||
| * Normalize preprocessed whitespace. | ||
| * https://www.w3.org/TR/css-syntax-3/#input-preprocessing | ||
| */ | ||
| "\r" => '\\A ', | ||
| "\f" => '\\A ', | ||
| "\r\n" => '\\A ', | ||
|
|
||
| /* | ||
| * CSS Unicode escaping for problematic characters. | ||
| * https://www.w3.org/TR/css-syntax-3/#escaping | ||
| * | ||
| * These characters are not required by CSS but may be problematic in WordPress: | ||
| * | ||
| * - "<" is replaced to prevent issues with KSES and other sanitization when | ||
| * printing CSS later. | ||
| * - "," is replaced to prevent issues where multiple font family names may be | ||
| * split, sanitized, and joined on the `,` character (regardless of quoting | ||
| * or escaping). | ||
| * | ||
| * Note that the Unicode escape sequences are used rather than backslash-escaping. | ||
| * This also helps to prevent issues with problematic characters. | ||
| */ | ||
| "\n" => '\\A ', | ||
adamziel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| '\\' => '\\5C ', | ||
| ',' => '\\2C ', | ||
|
Contributor
There was a problem hiding this comment. Is this one necessary? Not that it's wrong, but we're producing a quoted value anyway. It shouldn't derail any syntax parsing and, within a string, it's seen as a comma either way.
Member
Author
There was a problem hiding this comment. When printing CSS, the font system checks for wordpress-develop/src/wp-includes/fonts/class-wp-font-utils.php Lines 67 to 76 in 4d43703 It seemed preferable to escape
Contributor
There was a problem hiding this comment. Ah so it's a workaround for the deficiencies of another part of the system. Could we go the other way around and parse the value instead of exploding by
Member
Author
There was a problem hiding this comment. Core doesn't have a CSS parser, does it? This Unicode escaping is at worst harmless and at best better than requiring full blown CSS parsing (even if that would be the most correct thing to do later).
Contributor
There was a problem hiding this comment. It doesn't today, but it could. CSS Processor is just a tokenizer, not a full-blown parser. Or you might want to reuse just the part of it that parses strings, just to avoid the blanket |
||
| '"' => '\\22 ', | ||
| '<' => '\\3C ', | ||
sirreal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ) | ||
| ) . '"'; | ||
| } | ||
|
|
||
| /** | ||
| * Compiles the `src` into valid CSS. | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% satisfied with this. A broken family name could be used, like
""", which would result in broken CSS output. The existing implementation has the same issue. This is likely good enough for anything but the most malicious font name.The font family name
"""could be safely used with this implementation as'"""'or preferably something like"\22\22\22".One improvement here is that the string must start and end with a matching quote character
"…"or'…'in order to be treated as a quoted string. This allows fonts to contain those characters without issue and they'll be normalized properly.It would be nice if the system knew that plain strings were provided and all quoting and normalization of the font family name were handled by the system. I'm not sure that's possible while maintaining backwards compatibility.