Twenty Seventeen: Use of wrong function when escaping font url

[davidakennedy]

See: ​https://wordpress.slack.com/archives/core-themes/p1480445730000676

/**
 * Register custom fonts.
 */
function twentyseventeen_fonts_url() {
    $fonts_url = '';

    /**
     * Translators: If there are characters in your language that are not
     * supported by Libre Franklin, translate this to 'off'. Do not translate
     * into your own language.
     */
    $libre_franklin = _x( 'on', 'Libre Franklin font: on or off', 'twentyseventeen' );

    if ( 'off' !== $libre_franklin ) {
        $font_families = array();

        $font_families[] = 'Libre Franklin:300,300i,400,400i,600,600i,800,800i';

        $query_args = array(
            'family' => urlencode( implode( '|', $font_families ) ),
            'subset' => urlencode( 'latin,latin-ext' ),
        );

        $fonts_url = add_query_arg( $query_args, 'https://fonts.googleapis.com/css' );
    }

    return esc_url_raw( $fonts_url );
}

return esc_url_raw( $fonts_url ); should be return esc_url( $fonts_url );

That function is more appropriate for the intended output here.

[ocean90]

I think the esc_url_raw() is appropriate here because twentyseventeen_fonts_url() only returns the URL, it doesn't print it. The value is currently passed to wp_enqueue_style() which uses esc_url() internally in WP_Styles::_css_href().

[WPDevHQ]

That makes sense to me now @ocean90 - thanks.

[peterwilsoncc]

I agree with @ocean90 , 2012-'15 urlencode but do not escape the return value. As it stands, output is consistent across the default themes.