Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#38987 closed defect (bug) (invalid)

Twenty Seventeen: Use of wrong function when escaping font url

Reported by: davidakennedy's profile davidakennedy Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.7
Component: Bundled Theme Keywords: good-first-bug close
Focuses: Cc:



 * 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, '' );

    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.

Change History (3)

#1 @ocean90
7 years ago

  • Keywords close added

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().

#2 @WPDevHQ
7 years ago

That makes sense to me now @ocean90 - thanks.

#3 @peterwilsoncc
7 years ago

  • Milestone 4.7 deleted
  • Resolution set to invalid
  • Status changed from new to closed

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

Note: See TracTickets for help on using tickets.